https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94567

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jeffrey A. Law from comment #5)
> I've pondered just killing that pattern, but I'm pretty sure there'll be
> notable regressions.  There was a clear regression we fixed in gcc-6 due to
> not handling QImode operands in that pattern.
> 
> What I'm playing with is looking at pos + len and if it hits the sign bit in
> the operand's mode, then widening the mode.  Something like this:
> 
> +  /* If the mask is going to have the sign bit set in the mode
> +     we want to do the comparison, then we must widen the target
> +     mode.  Otherwise the flags will be incorrect when we split
> +     this into a (compare (and (op0) (mask))) and a subsequent
> +     test like LE will get the wrong result.  */
> +  if (mode < E_DImode
> +      && pos + len == GET_MODE_PRECISION (mode))
> +    {
> +      mode = GET_MODE_WIDER_MODE (mode).require ();
> +      val = gen_lowpart (mode, val);
> +    }
> +
> 
> Which I think is roughly what you were suggesting.  Mine does it with a
> SUBREG, so it matches existing patterns...  A ZERO_EXTEND may well require
> new patterns.

I see.  A subreg should work as well indeed.  Note I think simply using
the original mode combine used is safer than using some random other(?)
then we know the comparison semantics are unchanged.  Of course your
suggested change above will cause less changes at this point.

There's also

   && ix86_match_ccmode (insn,
                         /* *testdi_1 requires CCZmode if the mask has bit
                            31 set and all bits above it clear.  */
                         GET_MODE (operands[2]) == DImode
                         && INTVAL (operands[3]) + INTVAL (operands[4]) == 32
                         ? CCZmode : CCNOmode)"

which is likely where the original correctness issue lies - it fails
to check for the similar cases and HImode (and SImode and QImode).

Reply via email to