Hi!
Mostlt nitpicking left:
On Mon, Dec 19, 2022 at 10:06:45PM +0800, Jiufu Guo wrote:
> When checking eq/ne with a constant which has only 16bits, it can be
> optimized to check the rotated data. By this, the constant building
> is optimized.
>
> As the example in PR103743:
> For "in == 0x8000000000000000LL", this patch generates:
> rotldi 3,3,1 ; cmpldi 0,3,1
> instead of:
> li 9,-1 ; rldicr 9,9,0,0 ; cmpd 0,3,9
Excellent :-)
> * config/rs6000/rs6000-protos.h (can_be_rotated_to_lowbits): New.
> (can_be_rotated_to_positive_16bits): New.
> (can_be_rotated_to_negative_15bits): New.
> * config/rs6000/rs6000.cc (can_be_rotated_to_lowbits): New definition.
> (can_be_rotated_to_positive_16bits): New definition.
> (can_be_rotated_to_negative_15bits): New definition.
> * config/rs6000/rs6000.md (*rotate_on_cmpdi): New define_insn_and_split.
Good names. Great function comments as well.
> +/* Check if C (as 64bit integer) can be rotated to a constant which constains
> + nonzero bits at LOWBITS only.
"at the LOWBITS low bits only". Well it probably is clear what is
meant :-)
> + Return true if C can be rotated to such constant. And *ROT is written to
> + the number by which C is rotated.
> + Return false otherwise. */
"If so, *ROT is written" etc.
> +(define_code_iterator eqne [eq ne])
You should say in the changelog that "eqne" was moved.
"(eqne): Move earlier." is plenty of course.
> +(define_insn_and_split "*rotate_on_cmpdi"
> + rtx note = find_reg_note (curr_insn, REG_BR_PROB, 0);
Move this much later please, to just before it is used.
> + /* keep the probability info for the prediction of the branch insn. */
"Keep", sentences start with a capital.
> +}
> +)
These go on one line, as just
})
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr103743.c
> @@ -0,0 +1,52 @@
> +/* { dg-options "-O2" } */
> +/* { dg-do compile { target has_arch_ppc64 } } */
> +
> +/* { dg-final { scan-assembler-times {\mcmpldi\M} 10 } } */
> +/* { dg-final { scan-assembler-times {\mcmpdi\M} 4 } } */
> +/* { dg-final { scan-assembler-times {\mrotldi\M} 14 } } */
> +
With so much going on in just one function, I am a bit worried that this
testcase will easily fail in the future. We will see.
Okay for trunk with those i's dotted. Thank you!
Segher
