Avoid optimizing primitive #33 in vm.c (for VC++)
Jaco van der Merwe
jaco.lists at telkomsa.net
Mon Aug 9 05:35:09 PDT 2004
Hi
Looking at the disassembly below it certainly looks like a global
optimization bug in VC++. I did some experimentation and found that the
following implementation of _primitive33() works without having to disable
any optimizations:
void _primitive33(struct Interpreter * interpreter, ObjectPointer *
arguments, unsigned long int argumentsSize, struct OopArray * optionals)
{
signed long int y;
signed long int x;
x = ObjectPointer_asSmallInt( arguments[0] );
y = ObjectPointer_asSmallInt( arguments[1] );
PSInterpreter_stackPush_(interpreter, x < y ? CurrentMemory->TrueObject :
CurrentMemory->FalseObject);
}
Comparing this to the orignal version below
void _primitive33(struct Interpreter * interpreter, ObjectPointer *
arguments, unsigned long int argumentsSize, struct OopArray * optionals)
{
ObjectPointer y;
ObjectPointer x;
x = arguments[0];
y = arguments[1];
PSInterpreter_stackPush_(interpreter, ObjectPointer_asSmallInt(x) <
ObjectPointer_asSmallInt(y)?CurrentMemory->TrueObject:CurrentMemory->FalseOb
ject);
}
it seems that the bug is caused by the interaction between the comparison
operator (<) with the inlining of the ObjectPointer_asSmallInt() function
calls. Taking out the latter into local variables somehow removes this
interaction and makes the optimization bug disappear.
I'm not familiar with the pidgin Slate for generating this primitive, so
maybe somebody familiar with this can update the pidgin code to generate the
function in the above form.
Reading the history of the Microsoft bug reports on global optimisation
bugs, it seems that they have been having problems with the interaction
between the shift operators and the comparison operators. By the way, these
bug reports are all for older versions of teh VC++ compilers, i.e., versions
5 and 6. One would expect that these bugs should have been fixed in the 7.0
and 7.1 versions, but maybe there are still some uhandled corner cases
lurking in their code somewhere.
Just to be on the safe side I would modify the form of ALL the pidgin code
that generates similar forms, i.e. inlined function calls (containing shift
operators) called around comparison operators, to first store the results of
the function calls in local variables and then use the comparison operator
after that on the local variables.
Regards
Jaco
----- Original Message -----
From: "Lendvai Attila" <Attila.Lendvai at netvisor.hu>
To: <slate at tunes.org>
Sent: Monday, August 09, 2004 12:08 PM
Subject: RE: Avoid optimizing primitive #33 in vm.c (for VC++)
hi!
Please find the disassembly below of the optimized/non-optimized
version. I've tried to surround the signed cast with an extra (), but
the same result
static INLINE signed long int ObjectPointer_asSmallInt(ObjectPointer
oop)
{
return ((signed long int) oop) >> 1;
}
in the optimized version the rotation is done with an "shr" opcode,
which clears the MSB. in the non-optimized version, "sar" is used, which
behaves as expected...
there are other >> 1 operations all around in vm.c, but still turning
off global optimizations with the #pragma results in a working vm.
I'm downloading VC++ 2005 beta now and see what's in there?
- 101
MSDN jokes, the search itself:
http://search.microsoft.com/search/results.aspx?view=msdn&st=a&na=81&qu=
&qp=global+optimizations&qa=&qn=&c=10&s=1
some intresting pieces:
http://support.microsoft.com/default.aspx?scid=kb;en-us;250887
http://support.microsoft.com/default.aspx?scid=kb;en-us;113427
NON-OPTIMIZED:
void _primitive33(struct Interpreter * interpreter, ObjectPointer *
arguments, unsigned long int argumentsSize, struct OopArray * optionals)
{
00408B50 push ebp
00408B51 mov ebp,esp
00408B53 sub esp,18h
ObjectPointer x;
ObjectPointer y;
x = arguments[0];
00408B56 mov eax,dword ptr [arguments]
00408B59 mov ecx,dword ptr [eax]
00408B5B mov dword ptr [x],ecx
y = arguments[1];
00408B5E mov edx,dword ptr [arguments]
00408B61 mov eax,dword ptr [edx+4]
00408B64 mov dword ptr [y],eax
PSInterpreter_stackPush_(interpreter, ObjectPointer_asSmallInt(x) <
ObjectPointer_asSmallInt(y)?CurrentMemory->TrueObject:CurrentMemory->Fal
seObject);
00408B67 mov ecx,dword ptr [x]
00408B6A sar ecx,1
00408B6C mov edx,dword ptr [y]
00408B6F sar edx,1
00408B71 cmp ecx,edx
00408B73 jge _primitive33+32h (408B82h)
00408B75 mov eax,dword ptr [_CurrentMemory (411D60h)]
00408B7A mov ecx,dword ptr [eax+24h]
00408B7D mov dword ptr [ebp-18h],ecx
00408B80 jmp _primitive33+3Eh (408B8Eh)
00408B82 mov edx,dword ptr [_CurrentMemory (411D60h)]
00408B88 mov eax,dword ptr [edx+28h]
00408B8B mov dword ptr [ebp-18h],eax
00408B8E mov ecx,dword ptr [ebp-18h]
00408B91 push ecx
00408B92 mov edx,dword ptr [interpreter]
00408B95 push edx
00408B96 call PSInterpreter_stackPush_ (406860h)
00408B9B add esp,8
}
00408B9E mov esp,ebp
00408BA0 pop ebp
00408BA1 ret
OPTIMIZED:
void _primitive33(struct Interpreter * interpreter, ObjectPointer *
arguments, unsigned long int argumentsSize, struct OopArray * optionals)
{
ObjectPointer x;
ObjectPointer y;
x = arguments[0];
y = arguments[1];
PSInterpreter_stackPush_(interpreter, ObjectPointer_asSmallInt(x) <
ObjectPointer_asSmallInt(y)?CurrentMemory->TrueObject:CurrentMemory->Fal
seObject);
004064E0 mov eax,dword ptr [esp+8]
004064E4 mov ecx,dword ptr [eax+4]
004064E7 mov edx,dword ptr [eax]
004064E9 shr ecx,1
004064EB shr edx,1
004064ED cmp edx,ecx
004064EF push esi
004064F0 push edi
004064F1 jge _primitive33+1Dh (4064FDh)
004064F3 mov eax,dword ptr [_CurrentMemory (40FD60h)]
004064F8 mov edi,dword ptr [eax+24h]
004064FB jmp _primitive33+26h (406506h)
004064FD mov ecx,dword ptr [_CurrentMemory (40FD60h)]
00406503 mov edi,dword ptr [ecx+28h]
00406506 mov esi,dword ptr [esp+0Ch]
0040650A mov edx,dword ptr [esi+24h]
0040650D cmp edx,dword ptr [esi+28h]
00406510 jne _primitive33+3Bh (40651Bh)
00406512 push esi
00406513 call PSInterpreter_growStack (404D60h)
00406518 add esp,4
0040651B mov eax,dword ptr [esi+24h]
0040651E mov ecx,dword ptr [esi+0Ch]
00406521 mov dword ptr [ecx+eax*4+0Ch],edi
00406525 add dword ptr [esi+24h],1
00406529 pop edi
0040652A pop esi
}
0040652B ret
:: -----Original Message-----
:: From: slate-bounces at tunes.org
:: [mailto:slate-bounces at tunes.org] On Behalf Of Lee Salzman
:: Sent: Monday, August 09, 2004 9:01 AM
:: To: Paul Dufresne
:: Cc: slate at tunes.org
:: Subject: Re: Avoid optimizing primitive #33 in vm.c (for VC++)
::
::
:: The primitive causing the problem is SmallInteger <
:: SmallInteger, not the Float one. Not sure why VC would do
:: this, though, other than that it
:: may just be a silly VC bug. Anyone has any ideas?
::
:: Lee
::
:: On Mon, Aug 09, 2004 at 06:11:45AM +0000, Paul Dufresne wrote:
:: > I said: "I think primitive 13 is this one:"
:: > but really meant primitive 33.
:: >
:: > Also, somehow I find it strange myself that the
:: > primitive causing problem be < for Floats,
:: > so it would be nice for someone to confirm.
:: >
:: > Finally, building bootstrap image have been done without a hitch.
:: > (Just hope the expression 'without a hitch' is Ok in english).
:: >
:: > --Paul
:: >
::
::
More information about the Slate
mailing list