RFR: 8259822: [PPC64] Support the prefixed instruction format added in POWER10 [v2]

Corey Ashford github.com+51754783+coreyashford at openjdk.java.net
Wed Jan 27 17:50:42 UTC 2021


On Tue, 26 Jan 2021 19:14:45 GMT, Kazunori Ogata <ogatak at openjdk.org> wrote:

>> src/hotspot/cpu/ppc/ppc.ad line 5925:
>> 
>>> 5923: instruct loadConL34(iRegLdst dst, immL34 src) %{
>>> 5924:   match(Set dst src);
>>> 5925:   ins_cost(DEFAULT_COST+1);
>> 
>> There's no predicate for >= POWER10.  I can see how this works because of the immL34 operand having its own predicate, but in later instructs, e.g. addL_reg_imm34 even though the operand is immI32, you still add the explicit predicate.
>> 
>> I'd rather there be an explicit POWER10 predicate in this instruct.
>
> If predicate is added, adlc fails with an error message: "Syntax Error: :ADLC does not support instruction chain rules with predicates"  I think addL_reg_imm34 allows predicate because it is not called from other rules.  Is it better to leave some comments?  (BTW, immI32 is only for POWER10 or higher.  POWER9 version uses immI16 or immI16hi.)

Hmm, I'm confused.  I don't see any other reference to loadConL34 in ppc.ad.

>> src/hotspot/cpu/ppc/assembler_ppc.inline.hpp line 209:
>> 
>>> 207: inline void Assembler::psubi(Register d, Register a, long si34) { Assembler::paddi( d, a, -si34, false); }
>>> 208: 
>>> 209: inline void Assembler::pli_or_li(Register d, long si34) {
>> 
>> Defined but not used.
>
> Same reason as the next comment (paddi_or_addi), but necessity of this one may be weaker.

Ok.  See my other comments about unused/untestable code.

>> src/hotspot/cpu/ppc/assembler_ppc.inline.hpp line 157:
>> 
>>> 155: }
>>> 156: 
>>> 157: inline void Assembler::paddi_or_addi(Register d, Register a, long si34) {
>> 
>> defined but not used.
>
> It is intended to show an example of an efficient set of macros/functions for a prefixed instruction. Since a non-prefixed (32-bit) instruction is more efficient if the argument fits within the operand fields because of smaller I-cache usage and no need for the padding nop, it is convenient to have a macro/function that select better format. I intended to show an example of recommended set of useful macros/functions when adding support for a prefixed instruction. I'm planning to use this function in my next patch. Is it better to move this to the next patch?

It would be my preference to post code up only when it's actually used.  When you get around to using it, there's a possibility you will want to modify it a bit, add a comment, etc.   So that can be done all in one commit, instead of split across two.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2095


More information about the hotspot-compiler-dev mailing list