[PATCH] rs6000: inefficient 64-bit constant generation for consecutive 1-bits
Peter Bergner
bergner@linux.ibm.com
Tue Sep 15 15:48:37 GMT 2020
> rs6000_is_valid_shift_mask handles this already (but it requires you to
> pass in the shift needed). rs6000_is_valid_mask will handle it.
> rs6000_is_valid_and_mask does not get a shift count parameter, so cannot
> use rldic currently.
After talking with you off line, I changed to using rs6000_is_valid_mask.
The did mean I had to change num_insns_constant_gpr to take a mode param
so it could be passed down to rs6000_is_valid_mask.
> Please improve something there instead?
>
>> - HOST_WIDE_INT ud1, ud2, ud3, ud4;
>> + HOST_WIDE_INT ud1, ud2, ud3, ud4, value = c;
>
> Do not put declarations for uninitialised and initialised variables on
> one line, please.
The new patch doesn't even touch this function anymore.
>> +(define_insn "rldic"
>> + [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
>> + (unspec:DI [(match_operand:DI 1 "gpc_reg_operand" "r")
>> + (match_operand:DI 2 "u6bit_cint_operand" "n")
>> + (match_operand:DI 3 "u6bit_cint_operand" "n")]
>> + UNSPEC_RLDIC))]
>> + "TARGET_POWERPC64"
>> + "rldic %0,%1,%2,%3")
...and this is gone too. I've replaced it with a generic splitter
that matches an already existing define_insn (rotl<mode>3_mask).
>> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,8,8" } } */
>> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,24,8" } } */
>> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,40,8" } } */
>> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,40,48" } } */
>> +/* { dg-final { scan-assembler "rldic r?\[0-9\]+,r?\[0-9\]+,40,23" } } */
>
> Please use {} quotes, and \m\M. \d can be helpful, too.
That was how I wrote it initially, but for some reason, it wouldn't match
at all. Do I need extra \'s for my regexs when using {}?
\d is any digit? Yeah, that would be better. Gotta find a manpage or ???
that describes what regex patterns are allowed.
This all said, Alan's rtx_costs patch touches this same area and he talked
about removing a similar splitter, so I think I will wait for his code to
be committed and then rework this on top of his changes.
Peter
More information about the Gcc-patches
mailing list