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

David T. Lewis lewis at mail.msen.com
Sat Apr 6 13:21:36 UTC 2013


I am fairly sure that Eliot fixed this and that I picked up his fix for
VMM trunk. In any case, the generated code with VMMaker-dtl.314 is


        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)));
        }

Maybe you are using a trunk VMM that needs an update? I try to merge
Eliot's changes incrementally, and sometimes I make a mistake along
the way.

Dave

On Fri, Apr 05, 2013 at 11:37:06PM -0700, Eliot Miranda wrote:
>  
> 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



More information about the Vm-dev mailing list