[PATCH] Fix the x86 *_doubleword_mask* patterns (PR target/85582)

[Jakub Jelinek]

On Wed, May 02, 2018 at 09:50:07AM +0200, Uros Bizjak wrote:
> > 2018-05-02  Jakub Jelinek  <jakub@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