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

[Vladimir Kozlov]

Thank you for explanation. I will push it and add contributor.

Vladimir

On 11/10/15 11:54 PM, Volker Simonis wrote:
>
>
> On Wed, Nov 11, 2015 at 7:13 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto: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 <mailto: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
>
>