On Thu, Apr 15, 2021 at 07:11:11PM +0100, Richard Sandiford wrote:
> Jakub Jelinek <ja...@redhat.com> writes:
> > --- gcc/config/aarch64/aarch64.md.jj 2021-04-15 10:45:02.798853095
> > +0200
> > +++ gcc/config/aarch64/aarch64.md 2021-04-15 13:28:04.734754364 +0200
> > @@ -3572,6 +3572,18 @@ (define_insn "*neg_<shift>_si2_uxtw"
> > [(set_attr "autodetect_type" "alu_shift_<shift>_op2")]
> > )
> >
> > +(define_insn "*neg_asr_si2_extr"
> > + [(set (match_operand:SI 0 "register_operand" "r")
> > + (neg:SI (match_operator 4 "subreg_lowpart_operator"
>
> Very minor, but it might be better to have the :SI on the match_operator
> too, like in the pattern below.
Fixed, thanks for catching that (and the "r" -> "=r"; I've
actually tested a patch that didn't have any constraints on the first
define_insn because I started with a define_split that didn't work,
and it happened to work during testing, only noticed I've missed them
afterwards.
> > @@ -5382,6 +5394,22 @@ (define_insn "*extrsi5_insn_uxtw_alt"
> > "extr\\t%w0, %w1, %w2, %4"
> > [(set_attr "type" "rotate_imm")]
> > )
> > +
> > +(define_insn "*extrsi5_insn_di"
> > + [(set (match_operand:SI 0 "register_operand" "=r")
> > + (ior:SI (ashift:SI (match_operand:SI 1 "register_operand" "r")
> > + (match_operand 3 "const_int_operand" "n"))
> > + (match_operator:SI 6 "subreg_lowpart_operator"
> > + [(zero_extract:DI
> > + (match_operand:DI 2 "register_operand" "r")
> > + (match_operand 5 "const_int_operand" "n")
> > + (match_operand 4 "const_int_operand" "n"))])))]
> > + "UINTVAL (operands[3]) < 32
> > + && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
> > + && UINTVAL (operands[4]) + UINTVAL (operands[5]) - 32 <= 64"
>
> Could you explain this condition? With operand 5 being the size
> and operand 4 being the position, I was expecting something like:
The first two conditions are like those on *extr<mode>5_insn
except that GET_MODE_BITSIZE (<MODE>mode) is hardcoded as 32.
Like in *extr<mode>5_insn, we have two shift counts, because
aarch64 is !BITS_BIG_ENDIAN the last operand of zero_extract
is a shift count too.
The
UINTVAL (operands[4]) + UINTVAL (operands[5]) - 32 <= 64
(should have been <= 32 actually) meant to test
IN_RANGE (UINTVAL (operands[4]) + UINTVAL (operands[5]), 32, 64)
because I mistakenly thought
or maybe just
UINTVAL (operands[4]) + UINTVAL (operands[5]) >= 32
was meant to test that the zero_extract extracts at least as many bits
that it doesn't mask any bits above that. But the subreg:SI
actually applies to the zero_extract result and therefore
operands[2] >> operands[4] rather than before that, so you're right
it needs to be == 32 rather than >= 32.
So, either it can be
"UINTVAL (operands[3]) < 32
&& UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
&& UINTVAL (operands[4]) + UINTVAL (operands[5]) == 32"
or what you wrote:
> "UINTVAL (operands[3]) < 32
> && UINTVAL (operands[3]) == UINTVAL (operands[5])
> && UINTVAL (operands[4]) + UINTVAL (operands[5]) == 32"
or it could be:
"UINTVAL (operands[3]) < 32
&& UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
&& INTVAL (operands[3]) == INTVAL (operands[5])"
Jakub
