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

[Nicolas Cellier]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20130405/ee4d8c16/attachment.htm