[Vm-dev] CCodeGeneration bug or gcc compiler bug?

[Eliot Miranda]

Hi Nicolas,

    I think this is bad parenthesization.  I think it's fixed in the Cog
branch (at least I remember fixing it).  Try it in Cog.  Its late but I'll
try and find the fix some time soon.

On Fri, Apr 5, 2013 at 1:52 PM, Nicolas Cellier <
nicolas.cellier.aka.nice at gmail.com> wrote:

>
> Let me think aloud...
>
> THE CONTEXT:
>
> backport of LargeIntegersPlugin v2.0 (32 bit native word order) to
> Interpreter VM
>
> THE BUG:
>
> The Vm crashes with
> http://smalltalkhub.com/#!/~nice/NiceVMExperiments/versions/VMMaker-nice.315
>
> THE OFFENDING METHOD:
>
> InterpreterPrimitives>>magnitude64BitIntegerFor: magnitude neg: isNegative
>     "Return a Large Integer object for the given integer magnitude and
> sign"
>     | newLargeInteger largeClass lowWord highWord sz isSmall smallVal
> highWord2 |
>     <var: 'magnitude' type: #usqLong>
>     <var: 'lowWord' type: #usqInt>
>     <var: 'highWord' type: #usqInt>
>     <var: 'highWord2' type: #usqInt>
>
>     isSmall := isNegative
>                 ifTrue: [magnitude <= 16r40000000]
>                 ifFalse: [magnitude < 16r40000000].
>     isSmall ifTrue:
>         [smallVal := self cCoerceSimple: magnitude to: #sqInt.
>         isNegative    ifTrue: [smallVal := 0 - smallVal].
>         ^objectMemory integerObjectOf: smallVal].
>     largeClass := isNegative
>         ifTrue:[self classLargeNegativeInteger]
>         ifFalse:[self classLargePositiveInteger].
>     lowWord := magnitude bitAnd: 16rFFFFFFFF.
>     highWord := magnitude >> 32.
>     highWord = 0
>         ifTrue: [sz := 4]
>         ifFalse:
>             [sz := 5.
>             (highWord2 := highWord >> 8) = 0 ifFalse: [sz := sz + 1].
>             (highWord2 := highWord2 >> 8) = 0 ifFalse: [sz := sz + 1].
>             (highWord2 := highWord2 >> 8) = 0 ifFalse: [sz := sz + 1].].
>     newLargeInteger := objectMemory instantiateClass: largeClass
> indexableSize:  sz.
>     objectMemory storeLong32: 0 ofObject: newLargeInteger withValue:
> lowWord.
>     sz > 4 ifTrue: [objectMemory storeLong32: 1 ofObject: newLargeInteger
> withValue: highWord].
>     ^newLargeInteger
>
> THE CORRECTED METHOD:
>
> just replace the assignment
>
>     isNegative
>         ifTrue:[    largeClass := self classLargeNegativeInteger]
>         ifFalse:[    largeClass := self classLargePositiveInteger].
>
> THE OFFENDING GENERATED CODE:
>
> in src/vm/interp.c
>
> sqInt magnitude64BitIntegerForneg(usqLong magnitude, sqInt isNegative) {
> register struct foo * foo = &fum;
>     usqInt highWord;
>     usqInt highWord2;
>     sqInt isSmall;
>     sqInt largeClass;
>     usqInt lowWord;
>     sqInt newLargeInteger;
>     sqInt smallVal;
>     sqInt sz;
>     sqInt oop;
>     sqInt oop1;
>
>     isSmall = (isNegative
>         ? magnitude <= 1073741824
>         : magnitude < 1073741824);
>     if (isSmall) {
>         smallVal = ((sqInt) magnitude);
>         if (isNegative) {
>             smallVal = 0 - smallVal;
>         }
>         return ((smallVal << 1) | 1);
>     }
>     largeClass = (isNegative
>         ? /* begin fetchPointer:ofObject: */(oop =
> foo->specialObjectsOop),longAt((oop + (BASE_HEADER_SIZE)) +
> (ClassLargeNegativeInteger << (SHIFT_FOR_WORD)))
>         : /* begin fetchPointer:ofObject: */(oop1 =
> foo->specialObjectsOop),longAt((oop1 + (BASE_HEADER_SIZE)) +
> (ClassLargePositiveInteger << (SHIFT_FOR_WORD))));
>     lowWord = magnitude & 4294967295U;
>     highWord = magnitude >> 32;
>     if (highWord == 0) {
>         sz = 4;
>     } else {
>         sz = 5;
>         if (!(((highWord2 = ((usqInt) highWord) >> 8)) == 0)) {
>             sz += 1;
>         }
>         if (!(((highWord2 = ((usqInt) highWord2) >> 8)) == 0)) {
>             sz += 1;
>         }
>         if (!(((highWord2 = ((usqInt) highWord2) >> 8)) == 0)) {
>             sz += 1;
>         }
>     }
>     newLargeInteger = instantiateClassindexableSize(largeClass, sz);
>     long32Atput((newLargeInteger + (BASE_HEADER_SIZE)) + (0 << 2),
> lowWord);
>     if (sz > 4) {
>         long32Atput((newLargeInteger + (BASE_HEADER_SIZE)) + (1 << 2),
> highWord);
>     }
>     return newLargeInteger;
> }
>
> THE CORRECTED GENERATED CODE:
>
> Just change the ()?: assignment:
>
>     largeClass = (isNegative
>         ? /* begin fetchPointer:ofObject: */(oop =
> foo->specialObjectsOop),longAt((oop + (BASE_HEADER_SIZE)) +
> (ClassLargeNegativeInteger << (SHIFT_FOR_WORD)))
>         : /* begin fetchPointer:ofObject: */(oop1 =
> foo->specialObjectsOop),longAt((oop1 + (BASE_HEADER_SIZE)) +
> (ClassLargePositiveInteger << (SHIFT_FOR_WORD))));
>
> with an explicit if:
>
>     if (isNegative) {
>         /* begin fetchPointer:ofObject: */
>         oop = foo->specialObjectsOop;
>         largeClass = longAt((oop + (BASE_HEADER_SIZE)) +
> (ClassLargeNegativeInteger << (SHIFT_FOR_WORD)));
>     } else {
>         /* begin fetchPointer:ofObject: */
>         oop1 = foo->specialObjectsOop;
>         largeClass = longAt((oop1 + (BASE_HEADER_SIZE)) +
> (ClassLargePositiveInteger << (SHIFT_FOR_WORD)));
>     }
>
> THE OFFENDING METHOD JUST WORKS IN COG BUT GENERATED CODE IS DIFFERENT:
>
>     largeClass = (isNegative
>         ? longAt((GIV(specialObjectsOop) + BaseHeaderSize) +
> (ClassLargeNegativeInteger << ShiftForWord))
>         : longAt((GIV(specialObjectsOop) + BaseHeaderSize) +
> (ClassLargePositiveInteger << ShiftForWord)));
>
> THE QUESTION:
>
> I just don't understand what is incorrect in the (t)?a,b:c,d; construct...
> Could it be that it is interpreted:
>
>     ((t)?a,b:c),d;
>
> Ah if so, maybe that would explain the warning about oop1 being
> potentially used before assigned! (a bulb just enlighten)
>
> In this case we have a bug in the generator (probably a known one since we
> rearely ever use x := test ifTrue: [] ifFalse: [] in Slang)...
>
> Nicolas
>
>
>
>
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20130405/95b7045d/attachment.htm