[Vm-dev] [commit] r2463 - CogVM source as per VMMaker.oscog-eem.105. Fix signed32BitValueOf for most neg-

[David T. Lewis]

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 at 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