Re: [PATCH] Fix builtin-arith-overflow-p-1[23].c on i686

[Uros Bizjak <ubizjak at gmail dot com>]

On Thu, Jun 16, 2016 at 12:39 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Jun 16, 2016 at 11:51:12AM +0200, Jakub Jelinek wrote:
>> Here is what I've committed to the trunk and 6.2 after bootstrap/regtest on
>> x86_64-linux and i686-linux.
>> For 5/4.9, this doesn't apply cleanly, as http://gcc.gnu.org/r222592
>> aka https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01930.html
>> has not been backported.  Shall I backport that too, or just not apply to
>> 5/4.9?  I guess I should, because:
>> int v;
>> __attribute__ ((noinline, noclone)) void bar (void) { v++; }
>> __attribute__ ((noinline, noclone)) void foo (unsigned int x) { signed int y = ((-__INT_MAX__ - 1) / 2), r; if (__builtin_mul_overflow (x, y, &r)) bar (); }
>> int main () { foo (2); if (v) __builtin_abort (); return 0; }
>> is miscompiled with -m32 -O2 in 5.x (though, 4.9 doesn't support
>> __builtin_*_overflow, so maybe it is not an issue there).
>
> Ok, I went ahead and committed following to 5.x branch
> and the testcase also to trunk and 6.2.
> What is your preference for 4.9?  The testcase isn't applicable.

Let's also patch 4.9. We know what kind of pattern makes problems, and
it is possible for 4.9 to generate problematic sequence (involving
cc-setting arithmetic insn) even without new patterns.

Patch should be safe, since it adds another condition.

Thanks,
Uros.

> 2016-06-16  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/71554
>         * config/i386/i386.md (setcc + movzbl peephole2): Use reg_set_p.
>         (setcc + and peephole2): Likewise.
>
>         Backported from mainline
>         2015-04-29  Uros Bizjak  <ubizjak@gmail.com>
>
>         * config/i386/i386.md (setcc+movzbl peephole2): Check also clobbered
>         reg.
>         (setcc+andl peephole2): Ditto.
>
> 2016-06-16  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/71554
>         * gcc.c-torture/execute/pr71554.c: New test.
>
> --- gcc/config/i386/i386.md     (revision 237518)
> +++ gcc/config/i386/i386.md     (working copy)
> @@ -11645,7 +11645,8 @@ (define_peephole2
>         (zero_extend (match_dup 1)))]
>    "(peep2_reg_dead_p (3, operands[1])
>      || operands_match_p (operands[1], operands[3]))
> -   && ! reg_overlap_mentioned_p (operands[3], operands[0])"
> +   && ! reg_overlap_mentioned_p (operands[3], operands[0])
> +   && ! reg_set_p (operands[3], operands[4])"
>    [(parallel [(set (match_dup 5) (match_dup 0))
>               (match_dup 4)])
>     (set (strict_low_part (match_dup 6))
> @@ -11688,7 +11689,8 @@ (define_peephole2
>               (clobber (reg:CC FLAGS_REG))])]
>    "(peep2_reg_dead_p (3, operands[1])
>      || operands_match_p (operands[1], operands[3]))
> -   && ! reg_overlap_mentioned_p (operands[3], operands[0])"
> +   && ! reg_overlap_mentioned_p (operands[3], operands[0])
> +   && ! reg_set_p (operands[3], operands[4])"
>    [(parallel [(set (match_dup 5) (match_dup 0))
>               (match_dup 4)])
>     (set (strict_low_part (match_dup 6))
> --- gcc/testsuite/gcc.c-torture/execute/pr71554.c       (revision 0)
> +++ gcc/testsuite/gcc.c-torture/execute/pr71554.c       (working copy)
> @@ -0,0 +1,28 @@
> +/* PR target/71554 */
> +
> +int v;
> +
> +__attribute__ ((noinline, noclone)) void
> +bar (void)
> +{
> +  v++;
> +}
> +
> +__attribute__ ((noinline, noclone))
> +void
> +foo (unsigned int x)
> +{
> +  signed int y = ((-__INT_MAX__ - 1) / 2);
> +  signed int r;
> +  if (__builtin_mul_overflow (x, y, &r))
> +    bar ();
> +}
> +
> +int
> +main ()
> +{
> +  foo (2);
> +  if (v)
> +    __builtin_abort ();
> +  return 0;
> +}
>
>
>         Jakub