RFR(L): 8203197: C2: consider all paths in loop body for loop predication

Roland Westrelin rwestrel at redhat.com
Fri Jun 1 14:58:30 UTC 2018


Hi Vladimir,

Thanks for reviewing this.

> I think you should make new methodData layout as separate change. I want 
> it to be pushed and tested separately to make sure it does not cause any 
> issues.
> I am fine with your approach. Few comments:
>
> +#ifndef _LP64
> +    speculative_trap_padding,
> +#endif
>
> why you need it?

MethodData::bci_to_extra_data() makes assumption about the size of a
SpeculativeTrapData. See assert there. If it doesn't hold I think that
code is broken.

> In c1_LIRAssembler_aarch64.cpp you replaced LogBytesPerWord with 0 
> except last case. Why keep it in last case?

That must be wrong. I'll double check that.

> No changes for SPARC, PPC64, etc?

Sparc should be ok from code inspection but I can't test it so I will
need someone else to. I intended to ask for help for PPC64 etc. but I
wanted to first get that review started and see if there were
objections.

> Did you take into account the dominating (loop exit) check when you move 
> a following check as profiling predicate? There could be dependencies 
> between them (klass check, for example, before field load and then NULL 
> check you want to move).

I don't understand what you are referring to with loop exit.
There can be dependencies between predicates as you say but they are
kept in the order they are moved out of loop so dependent predicates are
always executed after the ones they depend on.

> Why Shenandoah's barriers checks do not convert into implicit NULL checks?

It does. But still we want the barriers hoisted out of loop whenever
possible. If the null check stays in the loop then it keeps the barrier
in the loop as well.

Roland.


More information about the hotspot-compiler-dev mailing list