RFR(S): 8142314: Bug in C1 ControlFlowOptimizer::delete_unnecessary_jumps with bytecode profiling

[Volker Simonis]

On Wed, Nov 11, 2015 at 7:13 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com
> wrote:

> Hi Volker,
>
> I did not get next statement.
> CMP instruction should produce flags which consumed by cmove and branch
> instructions.
> Can you explain what is different there for Itanium?
>

On all currently supported platforms, the CMP instruction is 'generic' in
the sense that it doesn't take any argument on how to compare (e.g.
'less-than', 'equal', etc). Instead it changes a flag register which can be
later on quired for all the different compare variants (e.g. 'jmp_equal',
'jump_less_than', etc).

On Itanium, the CMP instruction directly checks a single condition and sets
a predicate register (which is a 1-bit register) depending on the result of
the compare. Therefore, the conditions of the compare instructions and the
depending jump instructions (which actually use modified predicate
register) have to be aligned on Itanium


>
> > On most platforms this does no harm as the cmp of the instruction
> > doesn't depend on the condition. However, on Itanium the the cmp
> > depends on the condition. Therefore the correct code should be:
>
> Changes looks fine to me.
>
>
Thanks!

Could you please also sponsor this change?

Regards,
Volker



> Thanks,
> Vladimir
>
>
> On 11/9/15 6:11 AM, Volker Simonis wrote:
>
>> Hi,
>>
>> can I please have a review and a sponsor for the following change
>> contributed by gunter.haug at sap.com:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2015/8142314/
>> https://bugs.openjdk.java.net/browse/JDK-8142314
>>
>> The function ControlFlowOptimizer::delete_unnecessary_jumps reorders
>> basic block to minimize the number of branches required.
>> For example, it transforms
>>
>> B1 ...
>>    cmp [NE] [R1|I] [int:0|I]
>>    branch [NE] [B1]
>>    branch [AL] [B2]
>> B1 ...
>> B2 ...
>>
>> to
>>
>> B1 ...
>>    cmp [EQ] [R1|I] [int:0|I]
>>    branch [EQ] [B2]
>> B1 ...
>> B2 ...
>>
>> Bytecode profiling generates a pattern like this:
>>
>> B1 ...
>>    cmp [NE] [R1|I] [int:0|I]
>>    cmov [NE] [B1_counter] [B2_counter] [R2]
>>    ...
>>    branch [NE] [B1]
>>    branch [AL] [B2]
>> B1 ...
>> B2 ...
>>
>> ControlFlowOptimizer::delete_unnecessary_jumps doesn't take care of
>> the cmov. So the code above will transform to
>>
>> B1 ...
>>    cmp [EQ] [R1|I] [int:0|I]
>>    cmov [NE] [B1_counter] [B2_counter] [R2]
>>    ...
>>    branch [EQ] [B2]
>> B1 ...
>> B2 ...
>>
>> On most platforms this does no harm as the cmp of the instruction
>> doesn't depend on the condition. However, on Itanium the the cmp
>> depends on the condition. Therefore the correct code should be:
>>
>> B1 ...
>>    cmp [EQ] [R1|I] [int:0|I]
>>    cmov [EQ] [B2_counter] [B1_counter] [R2]
>>    ...
>>    branch [EQ] [B2]
>> B1 ...
>> B2 ...
>>
>>
>> Thank you and best regards,
>> Volker
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151111/e0ca8a89/attachment-0001.html>