You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "George Timoshenko (JIRA)" <ji...@apache.org> on 2008/03/13 05:37:46 UTC
[jira] Created: (HARMONY-5602) [drlvm][jit][opt][performance]
nullcheck for a field of boolean type
[drlvm][jit][opt][performance] nullcheck for a field of boolean type
--------------------------------------------------------------------
Key: HARMONY-5602
URL: https://issues.apache.org/jira/browse/HARMONY-5602
Project: Harmony
Issue Type: Improvement
Components: DRLVM
Reporter: George Timoshenko
Priority: Minor
The problem is reproducible at least for java_lang_ThreadGroup.remove(Ljava_lang_Thread;)V method.
It contains the following byte codes:
BYTECODE: GETFIELD bc-offset=7 (java/lang/ThreadGroup::destroyed)
BYTECODE: IFEQ bc-offset=10
HIR for these two bytecodes is:
I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
I29:ldci4 #0 -) t14:int32
I30:if ceq.i4 t13, t14 goto L4
As you can see bool value is extended to 32-bit integer type type. (The reason is: 3.3.4 of JVM spec.http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22909)
But in the case there are no any other usage of t13. So the comparison can be performed without loading the field from memory to a register.
(there is no copy propagation because of MOVZX not just a MOV for loading from memory)
The same problem may persist for all integer fields of less than 32 bits size.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-5602) [drlvm][jit][opt][performance]
nullcheck for a field of boolean type
Posted by "Lijuan Xiao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5602?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lijuan Xiao updated HARMONY-5602:
---------------------------------
Attachment: movx.patch
This is a patch that removes movzx/movsx and replace the cmp oprand with memory oprand. Thank George for your detailed explanation.
> [drlvm][jit][opt][performance] nullcheck for a field of boolean type
> --------------------------------------------------------------------
>
> Key: HARMONY-5602
> URL: https://issues.apache.org/jira/browse/HARMONY-5602
> Project: Harmony
> Issue Type: Improvement
> Components: DRLVM
> Reporter: George Timoshenko
> Assignee: Mikhail Fursov
> Priority: Minor
> Attachments: movx.patch
>
>
> The problem is reproducible at least for java_lang_ThreadGroup.remove(Ljava_lang_Thread;)V method.
> It contains the following byte codes:
> BYTECODE: GETFIELD bc-offset=7 (java/lang/ThreadGroup::destroyed)
> BYTECODE: IFEQ bc-offset=10
> HIR for these two bytecodes is:
> I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
> I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
> I29:ldci4 #0 -) t14:int32
> I30:if ceq.i4 t13, t14 goto L4
> As you can see bool value is extended to 32-bit integer type type. (The reason is: 3.3.4 of JVM spec.http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22909)
> But in the case there are no any other usage of t13. So the comparison can be performed without loading the field from memory to a register.
> (there is no copy propagation because of MOVZX not just a MOV for loading from memory)
> The same problem may persist for all integer fields of less than 32 bits size.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5602) [drlvm][jit][opt][performance]
nullcheck for a field of boolean type
Posted by "George Timoshenko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12578599#action_12578599 ]
George Timoshenko commented on HARMONY-5602:
--------------------------------------------
Current state does not provide an ability to choose: use a register for t13 or not.
It is always loaded onto some reg.
> [drlvm][jit][opt][performance] nullcheck for a field of boolean type
> --------------------------------------------------------------------
>
> Key: HARMONY-5602
> URL: https://issues.apache.org/jira/browse/HARMONY-5602
> Project: Harmony
> Issue Type: Improvement
> Components: DRLVM
> Reporter: George Timoshenko
> Assignee: Mikhail Fursov
> Priority: Minor
>
> The problem is reproducible at least for java_lang_ThreadGroup.remove(Ljava_lang_Thread;)V method.
> It contains the following byte codes:
> BYTECODE: GETFIELD bc-offset=7 (java/lang/ThreadGroup::destroyed)
> BYTECODE: IFEQ bc-offset=10
> HIR for these two bytecodes is:
> I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
> I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
> I29:ldci4 #0 -) t14:int32
> I30:if ceq.i4 t13, t14 goto L4
> As you can see bool value is extended to 32-bit integer type type. (The reason is: 3.3.4 of JVM spec.http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22909)
> But in the case there are no any other usage of t13. So the comparison can be performed without loading the field from memory to a register.
> (there is no copy propagation because of MOVZX not just a MOV for loading from memory)
> The same problem may persist for all integer fields of less than 32 bits size.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (HARMONY-5602) [drlvm][jit][opt][performance]
nullcheck for a field of boolean type
Posted by "Mikhail Fursov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5602?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mikhail Fursov reassigned HARMONY-5602:
---------------------------------------
Assignee: Mikhail Fursov
> [drlvm][jit][opt][performance] nullcheck for a field of boolean type
> --------------------------------------------------------------------
>
> Key: HARMONY-5602
> URL: https://issues.apache.org/jira/browse/HARMONY-5602
> Project: Harmony
> Issue Type: Improvement
> Components: DRLVM
> Reporter: George Timoshenko
> Assignee: Mikhail Fursov
> Priority: Minor
>
> The problem is reproducible at least for java_lang_ThreadGroup.remove(Ljava_lang_Thread;)V method.
> It contains the following byte codes:
> BYTECODE: GETFIELD bc-offset=7 (java/lang/ThreadGroup::destroyed)
> BYTECODE: IFEQ bc-offset=10
> HIR for these two bytecodes is:
> I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
> I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
> I29:ldci4 #0 -) t14:int32
> I30:if ceq.i4 t13, t14 goto L4
> As you can see bool value is extended to 32-bit integer type type. (The reason is: 3.3.4 of JVM spec.http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22909)
> But in the case there are no any other usage of t13. So the comparison can be performed without loading the field from memory to a register.
> (there is no copy propagation because of MOVZX not just a MOV for loading from memory)
> The same problem may persist for all integer fields of less than 32 bits size.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5602) [drlvm][jit][opt][performance]
nullcheck for a field of boolean type
Posted by "Mikhail Fursov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12578328#action_12578328 ]
Mikhail Fursov commented on HARMONY-5602:
-----------------------------------------
Why not using a register if it's free?
Is some other value pushed to stack to get the register here?
> [drlvm][jit][opt][performance] nullcheck for a field of boolean type
> --------------------------------------------------------------------
>
> Key: HARMONY-5602
> URL: https://issues.apache.org/jira/browse/HARMONY-5602
> Project: Harmony
> Issue Type: Improvement
> Components: DRLVM
> Reporter: George Timoshenko
> Assignee: Mikhail Fursov
> Priority: Minor
>
> The problem is reproducible at least for java_lang_ThreadGroup.remove(Ljava_lang_Thread;)V method.
> It contains the following byte codes:
> BYTECODE: GETFIELD bc-offset=7 (java/lang/ThreadGroup::destroyed)
> BYTECODE: IFEQ bc-offset=10
> HIR for these two bytecodes is:
> I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
> I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
> I29:ldci4 #0 -) t14:int32
> I30:if ceq.i4 t13, t14 goto L4
> As you can see bool value is extended to 32-bit integer type type. (The reason is: 3.3.4 of JVM spec.http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22909)
> But in the case there are no any other usage of t13. So the comparison can be performed without loading the field from memory to a register.
> (there is no copy propagation because of MOVZX not just a MOV for loading from memory)
> The same problem may persist for all integer fields of less than 32 bits size.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-5602) [drlvm][jit][opt][performance]
nullcheck for a field of boolean type
Posted by "Lijuan Xiao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5602?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lijuan Xiao updated HARMONY-5602:
---------------------------------
Attachment: movx_formated.patch
The updated patch removes unused variable declaration and is better formatted. It is built successfully with r707991 and smoke test. Yes, the optimization works at the first run of peephole that is before the regalloc/spillgen pass. I just do performance test with specjbb2005. In fact, the score after the optimization is slightly lower (with -Xms256m -Xmx256m -Xem:server).
> [drlvm][jit][opt][performance] nullcheck for a field of boolean type
> --------------------------------------------------------------------
>
> Key: HARMONY-5602
> URL: https://issues.apache.org/jira/browse/HARMONY-5602
> Project: Harmony
> Issue Type: Improvement
> Components: DRLVM
> Reporter: George Timoshenko
> Assignee: Mikhail Fursov
> Priority: Minor
> Attachments: movx.patch, movx_formated.patch
>
>
> The problem is reproducible at least for java_lang_ThreadGroup.remove(Ljava_lang_Thread;)V method.
> It contains the following byte codes:
> BYTECODE: GETFIELD bc-offset=7 (java/lang/ThreadGroup::destroyed)
> BYTECODE: IFEQ bc-offset=10
> HIR for these two bytecodes is:
> I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
> I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
> I29:ldci4 #0 -) t14:int32
> I30:if ceq.i4 t13, t14 goto L4
> As you can see bool value is extended to 32-bit integer type type. (The reason is: 3.3.4 of JVM spec.http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22909)
> But in the case there are no any other usage of t13. So the comparison can be performed without loading the field from memory to a register.
> (there is no copy propagation because of MOVZX not just a MOV for loading from memory)
> The same problem may persist for all integer fields of less than 32 bits size.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5602) [drlvm][jit][opt][performance]
nullcheck for a field of boolean type
Posted by "George Timoshenko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12625622#action_12625622 ]
George Timoshenko commented on HARMONY-5602:
--------------------------------------------
Hi,
the idea implemented more or less correctly.
I do not see any obvious lack.
Have you measure the performance effect?
Do I understand correctly, that the optimization works at the first run of peephole that is before the regalloc/spillgen pass?
> [drlvm][jit][opt][performance] nullcheck for a field of boolean type
> --------------------------------------------------------------------
>
> Key: HARMONY-5602
> URL: https://issues.apache.org/jira/browse/HARMONY-5602
> Project: Harmony
> Issue Type: Improvement
> Components: DRLVM
> Reporter: George Timoshenko
> Assignee: Mikhail Fursov
> Priority: Minor
> Attachments: movx.patch
>
>
> The problem is reproducible at least for java_lang_ThreadGroup.remove(Ljava_lang_Thread;)V method.
> It contains the following byte codes:
> BYTECODE: GETFIELD bc-offset=7 (java/lang/ThreadGroup::destroyed)
> BYTECODE: IFEQ bc-offset=10
> HIR for these two bytecodes is:
> I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
> I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
> I29:ldci4 #0 -) t14:int32
> I30:if ceq.i4 t13, t14 goto L4
> As you can see bool value is extended to 32-bit integer type type. (The reason is: 3.3.4 of JVM spec.http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22909)
> But in the case there are no any other usage of t13. So the comparison can be performed without loading the field from memory to a register.
> (there is no copy propagation because of MOVZX not just a MOV for loading from memory)
> The same problem may persist for all integer fields of less than 32 bits size.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-5602) [drlvm][jit][opt][performance]
nullcheck for a field of boolean type
Posted by "George Timoshenko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12618969#action_12618969 ]
George Timoshenko commented on HARMONY-5602:
--------------------------------------------
I got a question regarding the issue. Here is my answer.
>I think that you mean to save a register for t13, and finally generate IA-32 instruction like CMP [memory address], src2, right?
Yes, you are absolutely right.
>However, I am still confused with what HIR to generate for the two lines of bytecode. Would you please explain your thoughts more on that?
You mean: you do not know what to generate in HIR instead of the lines I have qouted?
I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
I29:ldci4 #0 -) t14:int32
I30:if ceq.i4 t13, t14 goto L4
The point is these lines are correct. They are made by translator. Translator does not know if t13 will be used other than just that comparison or not.
when it generates GETFIELD.
My idea is:
find some place in HLO path where use-def information is available (memopt for example) or create a new HLO optimization which calculates use-defs for itself.
Then we need to identify all OP_LdInd instructions with mem_item_size < dst_size and the only usage of dst which mast be a comparison (or any other instruction that allows memory operand).
The significant condition: this only usage must be able to reduce its size to the mem_item_size.
Finally we can reduce:
replace LdInd dst operand (t13 in our example) with the one of mem_item_size size. Do the same for use instruction.
And then make sure that the propagation finally happens and codegen does not generate a MOV from memory.
Actually the optimization can be done in the codegenerator.
(Find MOVZX with dst that is used only once in the comparison with immediate which fits into mem_item_size.
Then remove the MOVZX and replace the original CMP with the smaller one using MOVZX's src (memory) operand )
I am not sure where the optimization will be more suitable.
> [drlvm][jit][opt][performance] nullcheck for a field of boolean type
> --------------------------------------------------------------------
>
> Key: HARMONY-5602
> URL: https://issues.apache.org/jira/browse/HARMONY-5602
> Project: Harmony
> Issue Type: Improvement
> Components: DRLVM
> Reporter: George Timoshenko
> Assignee: Mikhail Fursov
> Priority: Minor
>
> The problem is reproducible at least for java_lang_ThreadGroup.remove(Ljava_lang_Thread;)V method.
> It contains the following byte codes:
> BYTECODE: GETFIELD bc-offset=7 (java/lang/ThreadGroup::destroyed)
> BYTECODE: IFEQ bc-offset=10
> HIR for these two bytecodes is:
> I27:ldflda [t1.java/lang/ThreadGroup::destroyed] -) t12:ref:bool
> I28:ldind.unc:b [t12] ((t10,t11)) -) t13:int32
> I29:ldci4 #0 -) t14:int32
> I30:if ceq.i4 t13, t14 goto L4
> As you can see bool value is extended to 32-bit integer type type. (The reason is: 3.3.4 of JVM spec.http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22909)
> But in the case there are no any other usage of t13. So the comparison can be performed without loading the field from memory to a register.
> (there is no copy propagation because of MOVZX not just a MOV for loading from memory)
> The same problem may persist for all integer fields of less than 32 bits size.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.