function argument declared volatile double broken

[jonesd]

Some code I'm using declares a function argument with the volatile type attribute, for reasons according to the comments:

Code: Select all

/*
** Do one step of the Kahan-Babushka-Neumaier summation.
**
** https://en.wikipedia.org/wiki/Kahan_summation_algorithm
**
** Variables are marked "volatile" to defeat c89 x86 floating point
** optimizations can mess up this algorithm.
*/

I've discovered that on X86 this code is broken but appears to work if the volatile attribute is removed.
I've attached a reproducer program. I'm using VSI C x86-64 V7.5-009.

[jonesd]

Looking at the generated code in the debugger, it looks like the caller puts the argument to kbn_step in %xmm0 while the called procedure thinks that scalar argument should be read from %rsi when it's declared volatile.

[jreagan]

We'll look at it

[jonesd]

We'll look at it

Thanks.

The code that exposed this bug was added to SQLite 11 months ago. At the time, there was discussion on their forum whether to use pragmas or the volatile keyword to suppress optimization and they went with the 'simpler' solution. I confirmed that left on its own the VSI compiler will optimize away the add/subtract pair of the same value that is the heart of the algorithm.

As it is easy to override built-in SQL functions, I have a workaround for the time being.

[jonesd]

The 7.6 version of the C compiler recently added to the portal doesn't appear to exhibit this problem with volatile double arguments. The release notes make no mention of it being fixed.

[jreagan]

yeah, we fixed a few generic issues in the GEM-to-LLVM converter dealing with volatiles and such. It might have been fixed due to a broken Fortran progran which then "fixes" it elsewhere. That is the challenge of using a common piece of code and writing multiple compiler release notes.

[arne_v]

Release notes for Foobar compiler version X should really contain both:
* changes to Foobar from version X-1 to version X
* changes to backend from version used by Foobar X-1 to version used by Foobar X

[jreagan]

I hear you. We never did that with GEM however. There were Pascal releases where the "fixed" list was only 6 bugs, but if I looked at the GEM CMS library there might be 1000 changes. Some might have been very specific to a language (or feature), some might be latent support for future features, some might have been fixes to repair a regression from the week before. The effort required to track "substantive/possibly visible bugfixes" was too much even more a larger team. It just gets turned into "Various optmization bugfixes with volatile variables" or "with COMMON blocks" or "with parameter passing" or "with DEBUG generation".

I've tried (sometimes REALLY tried; sometimes barely tried) to keep track of those substantive changes.

For instance, come the day when we move from LLVM 10 to LLVM 'latest' (currently at 18.1.8, but will be larger by the time we catch up), I will not try to research all the changes/enhancements/fixes in the LLVM code base. You are welcome to head over to llvm.org and dig out the old release notes (they have them for every release - sometimes detailed; sometimes vague, depending on the author).

I'll try to track better AND standardize the release notes between compilers. They are little more alike than before, but not 100%. Heck, I was just happy to get all the /VERSION output strings to match!

[arne_v]

I understand that there is a lot, but the trick must be to automate.

First I think you as a policy should only list changes in VSI code. For upstream open source just note the version change and refer to upstream web site.

I don't know how you manage release notes but I am thinking like:

For backend stuff have those writing release note fragments for a change set specify which languages they know will be impacted: ALL, Fortran, Pascal, C ..., Basic, NONE.

If a Pascal release include Pascal change sets m to n and backend change sets o to p. then build release notes as:

release note = first part written manual
for each Pascal change set {
release notes append release note fragment from change set
}
for each backend change set {
if ALL or Pascal {
release notes append release note fragment from change set
}
}
release note append last part written manual