| Message ID | 20230531092811.1269150-1-christophe.lyon@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Add pattern for bswap + rotate [PR 110039] | expand |
Christophe Lyon <christophe.lyon@linaro.org> writes: > After commit g:d8545fb2c71683f407bfd96706103297d4d6e27b, we missed a > pattern to match the new GIMPLE form. > > With this patch, gcc.target/aarch64/rev16_2.c passes again. > > 2023-05-31 Christophe Lyon <christophe.lyon@linaro.org> > > PR target/110039 > gcc/ > * config/aarch64/aarch64.md (aarch64_rev16<mode>2_alt3): New > pattern. > --- > gcc/config/aarch64/aarch64.md | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index 8b8951d7b14..663353791fd 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -6267,6 +6267,16 @@ > [(set_attr "type" "rev")] > ) > > +;; Similar pattern to mache (rotate (bswap) 16) > +(define_insn "aarch64_rev16<mode>2_alt3" > + [(set (match_operand:GPI 0 "register_operand" "=r") > + (rotate:GPI (bswap:GPI (match_operand:GPI 1 "register_operand" "r")) > + (const_int 16)))] > + "" > + "rev16\\t%<w>0, %<w>1" > + [(set_attr "type" "rev")] > +) > + Doesn't this have to be :SI only? The rtl expression and the instruction are different for :DI. Thanks, Richard > ;; zero_extend version of above > (define_insn "*bswapsi2_uxtw" > [(set (match_operand:DI 0 "register_operand" "=r")
On Wed, 31 May 2023 at 11:49, Richard Sandiford <richard.sandiford@arm.com> wrote: > Christophe Lyon <christophe.lyon@linaro.org> writes: > > After commit g:d8545fb2c71683f407bfd96706103297d4d6e27b, we missed a > > pattern to match the new GIMPLE form. > > > > With this patch, gcc.target/aarch64/rev16_2.c passes again. > > > > 2023-05-31 Christophe Lyon <christophe.lyon@linaro.org> > > > > PR target/110039 > > gcc/ > > * config/aarch64/aarch64.md (aarch64_rev16<mode>2_alt3): New > > pattern. > > --- > > gcc/config/aarch64/aarch64.md | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/gcc/config/aarch64/aarch64.md > b/gcc/config/aarch64/aarch64.md > > index 8b8951d7b14..663353791fd 100644 > > --- a/gcc/config/aarch64/aarch64.md > > +++ b/gcc/config/aarch64/aarch64.md > > @@ -6267,6 +6267,16 @@ > > [(set_attr "type" "rev")] > > ) > > > > +;; Similar pattern to mache (rotate (bswap) 16) > > +(define_insn "aarch64_rev16<mode>2_alt3" > > + [(set (match_operand:GPI 0 "register_operand" "=r") > > + (rotate:GPI (bswap:GPI (match_operand:GPI 1 "register_operand" > "r")) > > + (const_int 16)))] > > + "" > > + "rev16\\t%<w>0, %<w>1" > > + [(set_attr "type" "rev")] > > +) > > + > > Doesn't this have to be :SI only? The rtl expression and the > instruction are different for :DI. > > Do you mean the other two examples in the testcase? ( __rev16_64_alt, __rev16_64) They currently use aarch64_rev16di2_alt1 and aarch64_rev16di2_alt2 respectively. So yeah for this case it seems :SI would be right. Thanks, Christophe Thanks, > Richard > > > ;; zero_extend version of above > > (define_insn "*bswapsi2_uxtw" > > [(set (match_operand:DI 0 "register_operand" "=r") >
Christophe Lyon <christophe.lyon@linaro.org> writes: > On Wed, 31 May 2023 at 11:49, Richard Sandiford <richard.sandiford@arm.com> > wrote: > >> Christophe Lyon <christophe.lyon@linaro.org> writes: >> > After commit g:d8545fb2c71683f407bfd96706103297d4d6e27b, we missed a >> > pattern to match the new GIMPLE form. >> > >> > With this patch, gcc.target/aarch64/rev16_2.c passes again. >> > >> > 2023-05-31 Christophe Lyon <christophe.lyon@linaro.org> >> > >> > PR target/110039 >> > gcc/ >> > * config/aarch64/aarch64.md (aarch64_rev16<mode>2_alt3): New >> > pattern. >> > --- >> > gcc/config/aarch64/aarch64.md | 10 ++++++++++ >> > 1 file changed, 10 insertions(+) >> > >> > diff --git a/gcc/config/aarch64/aarch64.md >> b/gcc/config/aarch64/aarch64.md >> > index 8b8951d7b14..663353791fd 100644 >> > --- a/gcc/config/aarch64/aarch64.md >> > +++ b/gcc/config/aarch64/aarch64.md >> > @@ -6267,6 +6267,16 @@ >> > [(set_attr "type" "rev")] >> > ) >> > >> > +;; Similar pattern to mache (rotate (bswap) 16) >> > +(define_insn "aarch64_rev16<mode>2_alt3" >> > + [(set (match_operand:GPI 0 "register_operand" "=r") >> > + (rotate:GPI (bswap:GPI (match_operand:GPI 1 "register_operand" >> "r")) >> > + (const_int 16)))] >> > + "" >> > + "rev16\\t%<w>0, %<w>1" >> > + [(set_attr "type" "rev")] >> > +) >> > + >> >> Doesn't this have to be :SI only? The rtl expression and the >> instruction are different for :DI. >> > Do you mean the other two examples in the testcase? > ( __rev16_64_alt, __rev16_64) > They currently use aarch64_rev16di2_alt1 and aarch64_rev16di2_alt2 > respectively. I meant more that the new pattern would generate wrong code if someone wrote a 64-bit bswap followed by a 64-bit rotate left. Richard
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 8b8951d7b14..663353791fd 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -6267,6 +6267,16 @@ [(set_attr "type" "rev")] ) +;; Similar pattern to mache (rotate (bswap) 16) +(define_insn "aarch64_rev16<mode>2_alt3" + [(set (match_operand:GPI 0 "register_operand" "=r") + (rotate:GPI (bswap:GPI (match_operand:GPI 1 "register_operand" "r")) + (const_int 16)))] + "" + "rev16\\t%<w>0, %<w>1" + [(set_attr "type" "rev")] +) + ;; zero_extend version of above (define_insn "*bswapsi2_uxtw" [(set (match_operand:DI 0 "register_operand" "=r")