On Wed, May 02, 2018 at 09:50:07AM +0200, Uros Bizjak wrote:
> > 2018-05-02 Jakub Jelinek <ja...@redhat.com>
> >
> > PR target/85582
> > * config/i386/i386.md (*ashl<dwi>3_doubleword_mask,
> > *ashl<dwi>3_doubleword_mask_1, *<shift_insn><dwi>3_doubleword_mask,
> > *<shift_insn><dwi>3_doubleword_mask_1): If and[sq]i3 is needed,
> > don't
> > clobber operands[2], instead use a new pseudo. Formatting fixes.
> >
> > * gcc.c-torture/execute/pr85582-1.c: New test.
> > * gcc.c-torture/execute/pr85582-2.c: New test.
>
> O.
Thanks, committed. BTW, thinking about these some more,
isn't INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1
incorrect? Say for <MODE_SIZE> being 4 this is INTVAL (operands[3]) <= 31
so it will accept & 0 to & 31 (that is correct), or e.g. & -2 (that is
incorrect).
Isn't the right guarding condition that
(INTVAL (operands[3]) & (<MODE_SIZE> * BITS_PER_UNIT)) == 0
i.e. that we have guarantee the shift count doesn't have the topmost
relevant bit set and we can ignore bits above it (UB)?
And for the decision if we should use a masking or not perhaps
if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
!= (<MODE_SIZE> * BITS_PER_UNIT) - 1)
, again ignoring bits we don't care about.
> > --- gcc/config/i386/i386.md.jj 2018-05-01 12:18:15.566832552 +0200
> > +++ gcc/config/i386/i386.md 2018-05-01 13:09:36.635164943 +0200
> > @@ -10366,7 +10366,7 @@ (define_insn_and_split "*ashl<dwi>3_doub
> > (match_operand:SI 2 "register_operand" "c")
> > (match_operand:SI 3 "const_int_operand")) 0)))
> > (clobber (reg:CC FLAGS_REG))]
> > - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1
> > + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1
> > && can_create_pseudo_p ()"
> > "#"
> > "&& 1"
> > @@ -10385,8 +10385,12 @@ (define_insn_and_split "*ashl<dwi>3_doub
> >
> > operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT);
> >
> > - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1)
> > - emit_insn (gen_andsi3 (operands[2], operands[2], operands[3]));
> > + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1)
> > + {
> > + rtx tem = gen_reg_rtx (SImode);
> > + emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
> > + operands[2] = tem;
> > + }
> >
> > operands[2] = gen_lowpart (QImode, operands[2]);
> >
Jakub
