[PATCH] rs6000: Use rldimi for vec init instead of shift + ior

[Segher Boessenkool]

On Thu, Feb 18, 2021 at 11:58:59AM -0600, will schmidt wrote:
> On Wed, 2021-02-03 at 14:37 +0800, Kewen.Lin via Gcc-patches wrote:
> > This patch merges the previously approved one[1] and its relied patch
> 
> I don't see the review for [1] in the archives.  

I said:
  Can you make a different testcase perhaps?  This patch looks fine
  otherwise.

> > made by Segher here[2], it's to make unsigned int vector init go with
> > rldimi to merge two integers instead of shift and ior.
> > 
> > Segher's patch in [2] is required to make the test case pass,
> > otherwise the costing for new pseudo-to-pseudo copies and the folding
> > with nonzero_bits in combine will make the rl*imi pattern become
> > compact and split into ior and shift unexpectedly.
> > 
> > The commit log of Segher's patch describes it in more details:
> > 
> > "An rl*imi is usually written as an IOR of an ASHIFT or similar, and an
> > AND of a register with a constant mask.  In some cases combine knows
> > that that AND doesn't do anything (because all zero bits in that mask
> > correspond to bits known to be already zero), and then no pattern
> > matches.  This patch adds a define_split for such cases.  It uses
> > nonzero_bits in the condition of the splitter, but does not need it
> > afterwards for the instruction to be recognised.  This is necessary
> > because later passes can see fewer nonzero_bits.
> > 
> > Because it is a splitter, combine will only use it when starting with
> > three insns (or more), even though the result is just one.  This isn't
> > a huge problem in practice, but some possible combinations still won't
> > happen."
> > 
> > Bootstrapped/regtested on powerpc64le-linux-gnu P9 and
> > powerpc64-linux-gnu P8, also SPEC2017 build/run passed on P9.
> > 
> > Is it ok for trunk?
> > 
> > BR,
> > Kewen
> > 
> > [1] https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562407.html
> > [2] https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563526.html
> > 
> > 
> > gcc/ChangeLog:
> > 
> > 2020-02-03  Segher Boessenkool  <segher@kernel.crashing.org>
> > 	    Kewen Lin  <linkw@gcc.gnu.org>
> > 
> > 	* config/rs6000/rs6000.md (*rotl<mode>3_insert_3): Renamed to...
> > 	(rotl<mode>3_insert_3): ...this.
> 
> ok
> 
> > 	(plus_ior_xor): New code_iterator.
> > 	(define_split for GPR rl*imi): New splitter.
> 
> As described above, these two changes appear to identical to what was
> posted by Segher in [1].

But with testcase :-)

> +/* { dg-final { scan-assembler-not "or" } } */
> 
> As is, it looks OK to me.  Per other reviews i've gotten, you may
> get a request to wrap the "or" with \m \M .

Right, because it is very easy to end up with "or" as a substring of
something else.

> Some existing cases with
> scan-assembler-not have a leading whitespace/tab qualifier too. 
> i.e.
> /* { dg-final { scan-assembler-not "\[ \t\]or "      } } */

Yeah, but the \m in {\mor\M} will do the same already.

Thanks,


Segher