[commit] r2463 - CogVM source as per VMMaker.oscog-eem.105. Fix signed32BitValueOf for most neg-
Commit message blocked due to size, forwarding trimmed version:
Author: eliot Date: 2011-07-18 17:35:51 -0700 (Mon, 18 Jul 2011) New Revision: 2463
Added: branches/Cog/macbuild/CoreMTVM.xcodeproj/ branches/Cog/macbuild/CoreMTVM.xcodeproj/project.pbxproj branches/Cog/src/vm/cointerpmt.c branches/Cog/src/vm/cointerpmt.h branches/Cog/src/vm/gcc3x-cointerpmt.c Modified: branches/Cog/macbuild/makevm branches/Cog/macbuild/mvm branches/Cog/nscogsrc/plugins/AsynchFilePlugin/AsynchFilePlugin.c branches/Cog/nscogsrc/plugins/B2DPlugin/B2DPlugin.c branches/Cog/nscogsrc/plugins/BMPReadWriterPlugin/BMPReadWriterPlugin.c branches/Cog/nscogsrc/plugins/BitBltPlugin/BitBltPlugin.c branches/Cog/nscogsrc/plugins/DSAPrims/DSAPrims.c branches/Cog/nscogsrc/plugins/DropPlugin/DropPlugin.c branches/Cog/nscogsrc/plugins/FileCopyPlugin/FileCopyPlugin.c branches/Cog/nscogsrc/plugins/FilePlugin/FilePlugin.c branches/Cog/nscogsrc/plugins/FloatArrayPlugin/FloatArrayPlugin.c branches/Cog/nscogsrc/plugins/FloatMathPlugin/FloatMathPlugin.c branches/Cog/nscogsrc/plugins/JPEGReadWriter2Plugin/JPEGReadWriter2Plugin.c branches/Cog/nscogsrc/plugins/JPEGReaderPlugin/JPEGReaderPlugin.c branches/Cog/nscogsrc/plugins/LargeIntegers/LargeIntegers.c branches/Cog/nscogsrc/plugins/Matrix2x3Plugin/Matrix2x3Plugin.c branches/Cog/nscogsrc/plugins/MiscPrimitivePlugin/MiscPrimitivePlugin.c branches/Cog/nscogsrc/plugins/RePlugin/RePlugin.c branches/Cog/nscogsrc/plugins/SecurityPlugin/SecurityPlugin.c branches/Cog/nscogsrc/plugins/SocketPlugin/SocketPlugin.c branches/Cog/nscogsrc/plugins/SoundPlugin/SoundPlugin.c branches/Cog/nscogsrc/plugins/UUIDPlugin/UUIDPlugin.c branches/Cog/nscogsrc/plugins/VMProfileMacSupportPlugin/VMProfileMacSupportPlugin.c branches/Cog/nscogsrc/plugins/Win32OSProcessPlugin/Win32OSProcessPlugin.c branches/Cog/nscogsrc/plugins/ZipPlugin/ZipPlugin.c branches/Cog/nscogsrc/vm/cointerp.c branches/Cog/nscogsrc/vm/cointerp.h branches/Cog/nscogsrc/vm/gcc3x-cointerp.c branches/Cog/nscogsrc/vm/interp.h branches/Cog/nscogsrc/vm/vmCallback.h branches/Cog/platforms/win32/vm/sqWin32Window.c branches/Cog/scripts/findUnofficialFiles branches/Cog/src/vm/cointerp.c branches/Cog/src/vm/cointerp.h branches/Cog/src/vm/gcc3x-cointerp.c branches/Cog/src/vm/interp.h branches/Cog/src/vm/vmCallback.h branches/Cog/stacksrc/vm/gcc3x-interp.c branches/Cog/stacksrc/vm/interp.c branches/Cog/stacksrc/vm/interp.h branches/Cog/stacksrc/vm/vmCallback.h Log: CogVM source as per VMMaker.oscog-eem.105. Fix signed32BitValueOf for most neg- ative value C compiler mis-optimization. Speed up primitiveFail using ! trick. Add multi-threaded sources to tree (won't build yet due to issue in ia32abicc.c) Upgrade nscogsrc/plugins to official versions.
Added: branches/Cog/macbuild/CoreMTVM.xcodeproj/project.pbxproj =================================================================== --- branches/Cog/macbuild/CoreMTVM.xcodeproj/project.pbxproj (rev 0) +++ branches/Cog/macbuild/CoreMTVM.xcodeproj/project.pbxproj 2011-07-19
<snip>
On Thu, Jul 21, 2011 at 07:08:18AM -0400, David T. Lewis wrote:
Commit message blocked due to size, forwarding trimmed version:
Author: eliot Date: 2011-07-18 17:35:51 -0700 (Mon, 18 Jul 2011) New Revision: 2463
<snip>
Log: CogVM source as per VMMaker.oscog-eem.105. Fix signed32BitValueOf for most neg- ative value C compiler mis-optimization. Speed up primitiveFail using ! trick. Add multi-threaded sources to tree (won't build yet due to issue in ia32abicc.c) Upgrade nscogsrc/plugins to official versions.
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
TIA, Dave
If it would help here the change (in #signed32BitValueOf:)
Before:
value < 0 ifTrue: [self assert: (self sizeof: value) == 4. (negative and: [self cCode: [value - 1 > 0] inSmalltalk: [value = -16r80000000]]) ifTrue: "Don't fail for -2147483648 /-16r80000000" [^value]. After: value < 0 ifTrue: [self assert: (self sizeof: value) == 4. "Don't fail for -16r80000000/-2147483648 Alas the simple (negative and: [value - 1 > 0]) fails with contemporary gcc and icc versions with optimization and sometimes without. The shift works on all, touch wood." (negative and: [0 = (self cCode: [value << 1] inSmalltalk: [value << 1 bitAnd: (1 << 32) - 1])]) ifTrue: [^value].
On 21 July 2011 13:30, David T. Lewis lewis@mail.msen.com wrote:
On Thu, Jul 21, 2011 at 07:08:18AM -0400, David T. Lewis wrote:
Commit message blocked due to size, forwarding trimmed version:
Author: eliot Date: 2011-07-18 17:35:51 -0700 (Mon, 18 Jul 2011) New Revision: 2463
<snip>
Log: CogVM source as per VMMaker.oscog-eem.105. Fix signed32BitValueOf for most neg- ative value C compiler mis-optimization. Speed up primitiveFail using ! trick. Add multi-threaded sources to tree (won't build yet due to issue in ia32abicc.c) Upgrade nscogsrc/plugins to official versions.
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
TIA, Dave
On Thu, Jul 21, 2011 at 4:30 AM, David T. Lewis lewis@mail.msen.com wrote:
On Thu, Jul 21, 2011 at 07:08:18AM -0400, David T. Lewis wrote:
Commit message blocked due to size, forwarding trimmed version:
Author: eliot Date: 2011-07-18 17:35:51 -0700 (Mon, 18 Jul 2011) New Revision: 2463
<snip>
Log: CogVM source as per VMMaker.oscog-eem.105. Fix signed32BitValueOf for
most neg-
ative value C compiler mis-optimization. Speed up primitiveFail using !
trick.
Add multi-threaded sources to tree (won't build yet due to issue in
ia32abicc.c)
Upgrade nscogsrc/plugins to official versions.
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
The unit test is in the lien tests and is the attempt to assign max neg int (-2^31) through signedLongAt:put:. The problem is due to a pervasive C compiler bug with optimization. Recall that max neg int is exceptional in that it is the only value in a 2's complement representation that can't be negated since max neg int = -max pos int - 1. Here's the bug, and it occurred in signed64BitValueOf: and signedMachineIntegerValueOf: as well; I slipped in not fixing signed32BitValueOf:
<var: #value type: #int> ... (negative and: [self cCode: [value - 1 > 0] inSmalltalk: [value = -16r80000000]]) ifTrue: "Don't fail for -2147483648 /-16r80000000"
Since value is int (32 bits on relevant systems) then if value = 16r80000000, value - 1 will wrap around to 16r7fffffff, bit all other negative values will remain negative. Hence value - 1 > 0 /should/ cheaply identify max neg int. But both gcc in many versions, and icc, the intel compiler, under optimization assume that value - 1 > 0 is always false, and hence cause the primitive to fail for max neg int.
Instead, the solution I've adopted is to use a different sequence using shifts that is not mis-optimized, at least by gcc and icc:
(negative and: [0 = (self cCode: [value << 1] inSmalltalk: [value << 1 bitAnd: (1 << 32) - 1])]) ifTrue:
i.e. 16r8000000 is the only negative value (0 is positive) that when shifted left within a 32-bit type equals zero since the sign bit is shifted out.
And to those C compilers, grrr.... ;)
TIA, Dave
Thanks a lot Eliot, and Igor also!
Dave
On Thu, Jul 21, 2011 at 10:35:00AM -0700, Eliot Miranda wrote:
On Thu, Jul 21, 2011 at 4:30 AM, David T. Lewis lewis@mail.msen.com wrote:
On Thu, Jul 21, 2011 at 07:08:18AM -0400, David T. Lewis wrote:
Commit message blocked due to size, forwarding trimmed version:
Author: eliot Date: 2011-07-18 17:35:51 -0700 (Mon, 18 Jul 2011) New Revision: 2463
<snip>
Log: CogVM source as per VMMaker.oscog-eem.105. Fix signed32BitValueOf for
most neg-
ative value C compiler mis-optimization. Speed up primitiveFail using !
trick.
Add multi-threaded sources to tree (won't build yet due to issue in
ia32abicc.c)
Upgrade nscogsrc/plugins to official versions.
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
The unit test is in the lien tests and is the attempt to assign max neg int (-2^31) through signedLongAt:put:. The problem is due to a pervasive C compiler bug with optimization. Recall that max neg int is exceptional in that it is the only value in a 2's complement representation that can't be negated since max neg int = -max pos int - 1. Here's the bug, and it occurred in signed64BitValueOf: and signedMachineIntegerValueOf: as well; I slipped in not fixing signed32BitValueOf:
<var: #value type: #int> ... (negative and: [self cCode: [value - 1 > 0] inSmalltalk: [value = -16r80000000]]) ifTrue: "Don't fail for -2147483648 /-16r80000000"
Since value is int (32 bits on relevant systems) then if value = 16r80000000, value - 1 will wrap around to 16r7fffffff, bit all other negative values will remain negative. Hence value - 1 > 0 /should/ cheaply identify max neg int. But both gcc in many versions, and icc, the intel compiler, under optimization assume that value - 1 > 0 is always false, and hence cause the primitive to fail for max neg int.
Instead, the solution I've adopted is to use a different sequence using shifts that is not mis-optimized, at least by gcc and icc:
(negative and: [0 = (self cCode: [value << 1] inSmalltalk: [value << 1 bitAnd: (1 << 32) - 1])]) ifTrue:
i.e. 16r8000000 is the only negative value (0 is positive) that when shifted left within a 32-bit type equals zero since the sign bit is shifted out.
And to those C compilers, grrr.... ;)
TIA, Dave
-- best, Eliot
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
The unit test is in the lien tests and is the attempt to assign max neg int (-2^31) through signedLongAt:put:. The problem is due to a pervasive C compiler bug with optimization. Recall that max neg int is exceptional in that it is the only value in a 2's complement representation that can't be negated since max neg int = -max pos int - 1. Here's the bug, and it occurred in signed64BitValueOf: and signedMachineIntegerValueOf: as well; I slipped in not fixing signed32BitValueOf:
<var: #value type: #int> ... (negative and: [self cCode: [value - 1 > 0] inSmalltalk: [value = -16r80000000]]) ifTrue: "Don't fail for -2147483648 /-16r80000000"
Since value is int (32 bits on relevant systems) then if value = 16r80000000, value - 1 will wrap around to 16r7fffffff, bit all other negative values will remain negative. Hence value - 1 > 0 /should/ cheaply identify max neg int. But both gcc in many versions, and icc, the intel compiler, under optimization assume that value - 1 > 0 is always false, and hence cause the primitive to fail for max neg int.
Instead, the solution I've adopted is to use a different sequence using shifts that is not mis-optimized, at least by gcc and icc:
(negative and: [0 = (self cCode: [value << 1] inSmalltalk: [value << 1 bitAnd: (1 << 32) - 1])]) ifTrue:i.e. 16r8000000 is the only negative value (0 is positive) that when shifted left within a 32-bit type equals zero since the sign bit is shifted out.
And to those C compilers, grrr.... ;)
indeed!
Hi Eliot:
On 21/07/11 19:35, Eliot Miranda wrote:
On Thu, Jul 21, 2011 at 4:30 AM, David T. Lewis <lewis@mail.msen.com mailto:lewis@mail.msen.com> wrote:
Hi Eliot, Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.The unit test is in the lien tests and is the attempt to assign max neg int (-2^31) through signedLongAt:put:. The problem is due to a pervasive C compiler bug with optimization.
Maybe I misread this blog post: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html (*Signed integer overflow:)* but from that I would conclude that it is not a compiler bug, but undefined behavior in C instead. Thus, GCC and ICC are just doing there job in terms of what they are allowed to do by the spec.
Best regards Stefan
On Thu, Jul 21, 2011 at 3:17 PM, Stefan Marr squeak@stefan-marr.de wrote:
Hi Eliot:
On 21/07/11 19:35, Eliot Miranda wrote:
On Thu, Jul 21, 2011 at 4:30 AM, David T. Lewis <lewis@mail.msen.commailto: lewis@mail.msen.com> wrote:
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>**signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
The unit test is in the lien tests and is the attempt to assign max neg int (-2^31) through signedLongAt:put:. The problem is due to a pervasive C compiler bug with optimization.
Maybe I misread this blog post: http://blog.llvm.org/2011/05/** what-every-c-programmer-**should-know.htmlhttp://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html(*Signed integer overflow:)* but from that I would conclude that it is not a compiler bug, but undefined behavior in C instead. Thus, GCC and ICC are just doing there job in terms of what they are allowed to do by the spec.
Thanks, Stefan. I hadn't known this (or had forgotten):
*Signed integer overflow:* If arithmetic on an 'int' type (for example) overflows, the result is undefined. One example is that "INT_MAX+1" is not guaranteed to be INT_MIN. This behavior enables certain classes of optimizations that are important for some code. For example, knowing that INT_MAX+1 is undefined allows optimizing "X+1 > X" to "true". Knowing the multiplication "cannot" overflow (because doing so would be undefined) allows optimizing "X*2/2" to "X". While these may seem trivial, these sorts of things are commonly exposed by inlining and macro expansion. A more important optimization that this allows is for "<=" loops like this:
for (i = 0; i <= N; ++i) { ... }
In this loop, the compiler can assume that the loop will iterate exactly N+1 times if "i" is undefined on overflow, which allows a broad range of loop optimizations to kick in. On the other hand, if the variable is defined to wrap around on overflow, then the compiler must assume that the loop is possibly infinite (which happens if N is INT_MAX) - which then disables these important loop optimizations. This particularly affects 64-bit platforms since so much code uses "int" as induction variables.
Makes evil sense :)
Best regards Stefan
On 22 July 2011 01:24, Eliot Miranda eliot.miranda@gmail.com wrote:
On Thu, Jul 21, 2011 at 3:17 PM, Stefan Marr squeak@stefan-marr.de wrote:
Hi Eliot:
On 21/07/11 19:35, Eliot Miranda wrote:
On Thu, Jul 21, 2011 at 4:30 AM, David T. Lewis <lewis@mail.msen.com mailto:lewis@mail.msen.com> wrote:
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
The unit test is in the lien tests and is the attempt to assign max neg int (-2^31) through signedLongAt:put:. The problem is due to a pervasive C compiler bug with optimization.
Maybe I misread this blog post: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html (*Signed integer overflow:)* but from that I would conclude that it is not a compiler bug, but undefined behavior in C instead. Thus, GCC and ICC are just doing there job in terms of what they are allowed to do by the spec.
Thanks, Stefan. I hadn't known this (or had forgotten): Signed integer overflow: If arithmetic on an 'int' type (for example) overflows, the result is undefined. One example is that "INT_MAX+1" is not guaranteed to be INT_MIN. This behavior enables certain classes of optimizations that are important for some code. For example, knowing that INT_MAX+1 is undefined allows optimizing "X+1 > X" to "true". Knowing the multiplication "cannot" overflow (because doing so would be undefined) allows optimizing "X*2/2" to "X". While these may seem trivial, these sorts of things are commonly exposed by inlining and macro expansion. A more important optimization that this allows is for "<=" loops like this:
for (i = 0; i <= N; ++i) { ... }
In this loop, the compiler can assume that the loop will iterate exactly N+1 times if "i" is undefined on overflow, which allows a broad range of loop optimizations to kick in. On the other hand, if the variable is defined to wrap around on overflow, then the compiler must assume that the loop is possibly infinite (which happens if N is INT_MAX) - which then disables these important loop optimizations. This particularly affects 64-bit platforms since so much code uses "int" as induction variables. Makes evil sense :)
Ah yes.. integer types in C and implicit conversion between signed/unsigned/different bit size.. Yeah.. a huge field for bugs and frustration.
Best regards Stefan
-- best, Eliot
On Thu, Jul 21, 2011 at 03:24:08PM -0700, Eliot Miranda wrote:
On Thu, Jul 21, 2011 at 3:17 PM, Stefan Marr squeak@stefan-marr.de wrote:
Hi Eliot:
On 21/07/11 19:35, Eliot Miranda wrote:
On Thu, Jul 21, 2011 at 4:30 AM, David T. Lewis <lewis@mail.msen.commailto: lewis@mail.msen.com> wrote:
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>**signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
The unit test is in the lien tests and is the attempt to assign max neg int (-2^31) through signedLongAt:put:. The problem is due to a pervasive C compiler bug with optimization.
Maybe I misread this blog post: http://blog.llvm.org/2011/05/** what-every-c-programmer-**should-know.htmlhttp://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html(*Signed integer overflow:)* but from that I would conclude that it is not a compiler bug, but undefined behavior in C instead. Thus, GCC and ICC are just doing there job in terms of what they are allowed to do by the spec.
Thanks, Stefan. I hadn't known this (or had forgotten):
*Signed integer overflow:* If arithmetic on an 'int' type (for example) overflows, the result is undefined. One example is that "INT_MAX+1" is not guaranteed to be INT_MIN. This behavior enables certain classes of optimizations that are important for some code. For example, knowing that INT_MAX+1 is undefined allows optimizing "X+1 > X" to "true". Knowing the multiplication "cannot" overflow (because doing so would be undefined) allows optimizing "X*2/2" to "X". While these may seem trivial, these sorts of things are commonly exposed by inlining and macro expansion. A more important optimization that this allows is for "<=" loops like this:
for (i = 0; i <= N; ++i) { ... }
In this loop, the compiler can assume that the loop will iterate exactly N+1 times if "i" is undefined on overflow, which allows a broad range of loop optimizations to kick in. On the other hand, if the variable is defined to wrap around on overflow, then the compiler must assume that the loop is possibly infinite (which happens if N is INT_MAX) - which then disables these important loop optimizations. This particularly affects 64-bit platforms since so much code uses "int" as induction variables.
Makes evil sense :)
The issue does not exist in VMMaker trunk, which uses a simpler and presumably faster implementation. There is no need for the "value - 1" logic, hence no need for a workaround in the special case of the max negative value. The relevant methods are also tested on 32/64 host and image combinations.
Here is the logic from #signed32BitValueOf: with the special case handling removed entirely:
"Fail if value exceeds range of a 32-bit two's-complement signed integer." negative ifTrue:[value := 0 - value. value >= 0 ifTrue: [^ self primitiveFail]] ifFalse:[value < 0 ifTrue:[^ self primitiveFail]].
I did not run the alien tests, so maybe I'm missing something. In any case, background is at http://bugs.squeak.org/view.php?id=6987.
HTH, Dave
Hi David,
On Mon, Jul 25, 2011 at 6:27 AM, David T. Lewis lewis@mail.msen.com wrote:
On Thu, Jul 21, 2011 at 03:24:08PM -0700, Eliot Miranda wrote:
On Thu, Jul 21, 2011 at 3:17 PM, Stefan Marr squeak@stefan-marr.de
wrote:
Hi Eliot:
On 21/07/11 19:35, Eliot Miranda wrote:
On Thu, Jul 21, 2011 at 4:30 AM, David T. Lewis <lewis@mail.msen.com
<mailto:
lewis@mail.msen.com>> wrote:
Hi Eliot,
Can you say what the issue was with signed32BitValueOf? I can see the changes in InterpreterPrimitives>>**signed32BitValueOf: but I'm not clear on whether this is something that affects Alien, or if it is something that has been causing problems more generally but went unnoticed. Also, I'd like to document this with a unit test, so if you can suggest a code snippet that would be great.
The unit test is in the lien tests and is the attempt to assign max
neg
int (-2^31) through signedLongAt:put:. The problem is due to a
pervasive C
compiler bug with optimization.
Maybe I misread this blog post: http://blog.llvm.org/2011/05/** what-every-c-programmer-**should-know.html<
http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html%3E(*Si... integer overflow:)*
but from that I would conclude that it is not a compiler bug, but
undefined
behavior in C instead. Thus, GCC and ICC are just doing there job in
terms
of what they are allowed to do by the spec.
Thanks, Stefan. I hadn't known this (or had forgotten):
*Signed integer overflow:* If arithmetic on an 'int' type (for example) overflows, the result is undefined. One example is that "INT_MAX+1" is
not
guaranteed to be INT_MIN. This behavior enables certain classes of optimizations that are important for some code. For example, knowing that INT_MAX+1 is undefined allows optimizing "X+1 > X" to "true". Knowing the multiplication "cannot" overflow (because doing so would be undefined) allows optimizing "X*2/2" to "X". While these may seem trivial, these
sorts
of things are commonly exposed by inlining and macro expansion. A more important optimization that this allows is for "<=" loops like this:
for (i = 0; i <= N; ++i) { ... }
In this loop, the compiler can assume that the loop will iterate exactly
N+1
times if "i" is undefined on overflow, which allows a broad range of loop optimizations to kick in. On the other hand, if the variable is defined
to
wrap around on overflow, then the compiler must assume that the loop is possibly infinite (which happens if N is INT_MAX) - which then disables these important loop optimizations. This particularly affects 64-bit platforms since so much code uses "int" as induction variables.
Makes evil sense :)
The issue does not exist in VMMaker trunk, which uses a simpler and presumably faster implementation.
Alas, no, it still exists. The expression 0 - value is still undefined for overflow (i.e. undefined for max neg int) and hence, depending on compiler, the following statement value >= 0 may be assumed to be always true. This is the case for the intel compiler that we used at Qwaq/Teleplace, and is the reason I made the change in the first place. So yes, one does need the shift expression to be able to rely on different compilers generating correct code in this edge case.
There is no need for the "value - 1" logic, hence no need for a workaround in the special case of the max negative value. The relevant methods are also tested on 32/64 host and image combinations.
Here is the logic from #signed32BitValueOf: with the special case handling removed entirely:
"Fail if value exceeds range of a 32-bit two's-complement signed integer." negative ifTrue:[value := 0 - value. value >= 0 ifTrue: [^ self primitiveFail]] ifFalse:[value < 0 ifTrue:[^ self primitiveFail]].
I did not run the alien tests, so maybe I'm missing something. In any case, background is at http://bugs.squeak.org/view.php?id=6987.
HTH, Dave
On Mon, Jul 25, 2011 at 02:01:31PM -0700, Eliot Miranda wrote:
Hi David,
The issue does not exist in VMMaker trunk, which uses a simpler and presumably faster implementation.
Alas, no, it still exists. The expression 0 - value is still undefined for overflow (i.e. undefined for max neg int) and hence, depending on compiler, the following statement value >= 0 may be assumed to be always true. This is the case for the intel compiler that we used at Qwaq/Teleplace, and is the reason I made the change in the first place. So yes, one does need the shift expression to be able to rely on different compilers generating correct code in this edge case.
Thanks Eliot.
Dave
On Mon, Jul 25, 2011 at 02:01:31PM -0700, Eliot Miranda wrote:
On Mon, Jul 25, 2011 at 6:27 AM, David T. Lewis lewis@mail.msen.com wrote:
The issue does not exist in VMMaker trunk, which uses a simpler and presumably faster implementation.
Alas, no, it still exists. The expression 0 - value is still undefined for overflow (i.e. undefined for max neg int) and hence, depending on compiler, the following statement value >= 0 may be assumed to be always true. This is the case for the intel compiler that we used at Qwaq/Teleplace, and is the reason I made the change in the first place. So yes, one does need the shift expression to be able to rely on different compilers generating correct code in this edge case.
On further thought, according to the blog that Stefan referenced:
"It is worth noting that unsigned overflow is guaranteed to be defined as 2's complement (wrapping) overflow, so you can always use them."
http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html
If this is the case, then coercing to unsigned for the subtraction should prevent the problem of undefined results for value -16r80000000, thus:
"Fail if value exceeds range of a 32-bit twos-complement signed integer." negative ifTrue:["perform subtraction using unsigned int to prevent undefined result for optimizing C compilers in the case of value = -16r80000000" value := 0 - (self cCoerce: value to: 'unsigned int'). value >= 0 ifTrue: [^ self primitiveFail]] ifFalse:[value < 0 ifTrue:[^ self primitiveFail]].
I do not have access to a compiler that exhibits the problem, so I cannot verify. But I would expect this to work on all compilers, while avoiding an extra check for value -16r80000000.
Dave
vm-dev@lists.squeakfoundation.org
-
David T. Lewis -
Eliot Miranda -
Igor Stasenko -
Stefan Marr -
stephane ducasse