On Wed, 2021-02-03 at 14:37 +0800, Kewen.Lin via Gcc-patches wrote: > Hi, >
Hi, > This patch merges the previously approved one[1] and its relied patch I don't see the review for [1] in the archives. > 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 <seg...@kernel.crashing.org> > Kewen Lin <li...@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]. ( > * config/rs6000/vsx.md (vsx_init_v4si): Use gen_rotldi3_insert_3 > for integer merging. > > gcc/testsuite/ChangeLog: > > * gcc.target/powerpc/vec-init-10.c: New test. +/* { 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 . Some existing cases with scan-assembler-not have a leading whitespace/tab qualifier too. i.e. /* { dg-final { scan-assembler-not "\[ \t\]or " } } */ Thanks, -Will > > ----- >
