[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