[jira] Commented: (HARMONY-6016) [drlvm][jit] An undetected interference in Graph Coloring register allocation

["Xiaoming Gu (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HARMONY-6016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12648073#action_12648073 ] 

Xiaoming Gu commented on HARMONY-6016:
--------------------------------------

With more investigation, I guess the problem comes from "stack" pass in Ia32StackLayout.cpp which generates push & pop instructions for callee-saved registers. In debugging, the callee-save registers includes EBX, EBP, ESI and EDI.

If we use bin-packing, EBX is used explicitly in DataInputStream.readByte(). Then stack pass will save and restore EBX at prolog and epilog of that method. If we use graph-coloring, EBX is used implicitly (only BH is used) in the method. Then stack pass thinks no need to do push and pop for EBX, which causes an exception finally.

> [drlvm][jit] An undetected interference in Graph Coloring register allocation
> -----------------------------------------------------------------------------
>
>                 Key: HARMONY-6016
>                 URL: https://issues.apache.org/jira/browse/HARMONY-6016
>             Project: Harmony
>          Issue Type: Bug
>          Components: DRLVM
>         Environment: IA32 Windows
>            Reporter: Xiaoming Gu
>
> When early_prop turned off in server_static mode, there is a NullPointerException during running startup.helloworld. The bug comes from the last BB of the callee - DataInputStream.readByte(). Following are the generated binary code for this bug:
>   with early_prop
>     	02D67855 I15: MOVSX t26(EAX):I_32,t12(EAX):I_32 
>   without early_prop
> 	02D67865 I33: MOV t25(BH):I_8,t12(EAX):I_32 
> 	02D67867 I15: MOVSX t26(EAX):I_32,t25(BH):I_8 
> BH is defined if no early_prop. But in the caller - ICUBinary.readHeader() - EBX is actually defined before the above part of code and assumed to keep unmodifed and be used later. See details:
> 02466A22  mov         edx,edi 
> 02466A24  mov         dword ptr [esp+9Ch],eax 
> 02466A2B  mov         edi,ebx 
> 02466A2D  mov         ebx,edx    // ebx is defined
> 02466A2F  sub         esp,8 
> 02466A35  push        ebx  
> 02466A36  call        dword ptr [esi]    // call to readByte()
> 02466A38  mov         edx,dword ptr [esp+9Ch] 
> 02466A3F  mov         dword ptr [esp+98h],esi 
> 02466A46  mov         esi,ebp 
> 02466A48  mov         ecx,ebx    // ebx is used
> 02466A4A  mov         dword ptr [esp+94h],eax 
> 02466A51  mov         ebx,edx 
> 02466A53  mov         ebp,edi 
> 02466A55  mov         edi,ecx 
> 02466A57  mov         edx,dword ptr [esp+98h] 
> 02466A5E  sub         esp,8 
> 02466A64  push        edi    // the value of ebx is pushed
> 02466A65  call        dword ptr [edx]    // call to readByte() and exception thrown out finally
> So some part of EBX (BH) is changed uncarefully and an exception is thrown out finally. With early_prop the bug is gone because there is no changes about EBX.
> I verified my guess by making some changes in Ia32RegAlloc0.cpp to only use bin packing register allocation and found no bug when running startup.helloworld. My basic assumptions are early_prop is NOT indispensable and each optimization pass should be independent as much as possible.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.