[PATCH] rs6000: Use rldimi for vec init instead of shift + ior
Segher Boessenkool
segher@kernel.crashing.org
Thu Feb 18 18:28:45 GMT 2021
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
More information about the Gcc-patches
mailing list