On Sun, Nov 23, 2014 at 7:47 PM, Zhenqiang Chen <zhenqiang.c...@arm.com> wrote:
>
>> -----Original Message-----
>> From: Richard Henderson [mailto:r...@redhat.com]
>> Sent: Friday, November 21, 2014 2:27 AM
>> To: Zhenqiang Chen; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH, ifcvt] Fix PR63917
>>
>> On 11/20/2014 10:48 AM, Zhenqiang Chen wrote:
>> > +/* Check X clobber CC reg or not. */
>> > +
>> > +static bool
>> > +clobber_cc_p (rtx x)
>> > +{
>> > + RTX_CODE code = GET_CODE (x);
>> > + int i;
>> > +
>> > + if (code == CLOBBER
>> > + && REG_P (XEXP (x, 0))
>> > + && (GET_MODE_CLASS (GET_MODE (XEXP (x, 0))) == MODE_CC))
>> > + return TRUE;
>> > + else if (code == PARALLEL)
>> > + for (i = 0; i < XVECLEN (x, 0); i++)
>> > + if (clobber_cc_p (XVECEXP (x, 0, i)))
>> > + return TRUE;
>> > + return FALSE;
>> > +}
>>
>> Why would you need something like this when modified_between_p or one
>> of its kin ought to do the job?
>
> Thanks for the comments. Patch is updated to use set_of.
>
> And it is also enhanced to make sure that the new generated insns can not
> clobber CC.
>
> Bootstrap and no make check regression on X86-64 and IA32.
>
> OK for trunk?
>
> Thanks!
> -Zhenqiang
>
> ChangeLog:
> 2014-11-24 Zhenqiang Chen <zhenqiang.c...@arm.com>
>
> PR rtl-optimization/63917
> * ifcvt.c (cc_in_cond): New function.
> (end_ifcvt_sequence): Make sure new generated insns do not clobber
> CC.
> (noce_process_if_block, check_cond_move_block): Check CC references.
>
> testsuite/ChangeLog:
> 2014-11-24 Zhenqiang Chen <zhenqiang.c...@arm.com>
>
> * gcc.dg/pr63917.c: New test.
>
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index 21f08c2..1acd0ff 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -1016,6 +1016,18 @@ noce_emit_move_insn (rtx x, rtx y)
> 0, 0, outmode, y);
> }
>
> +/* Return the CC reg if it is used in COND. */
> +
> +static rtx
> +cc_in_cond (rtx cond)
> +{
> + if ((HAVE_cbranchcc4) && cond
> + && (GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == MODE_CC))
> + return XEXP (cond, 0);
> +
> + return NULL_RTX;
> +}
> +
> /* Return sequence of instructions generated by if conversion. This
> function calls end_sequence() to end the current stream, ensures
> that are instructions are unshared, recognizable non-jump insns.
> @@ -1026,6 +1038,7 @@ end_ifcvt_sequence (struct noce_if_info *if_info)
> {
> rtx_insn *insn;
> rtx_insn *seq = get_insns ();
> + rtx cc = cc_in_cond (if_info->cond);
>
> set_used_flags (if_info->x);
> set_used_flags (if_info->cond);
> @@ -1040,7 +1053,9 @@ end_ifcvt_sequence (struct noce_if_info *if_info)
> allows proper placement of required clobbers. */
> for (insn = seq; insn; insn = NEXT_INSN (insn))
> if (JUMP_P (insn)
> - || recog_memoized (insn) == -1)
> + || recog_memoized (insn) == -1
> + /* Make sure new generated code does not clobber CC. */
> + || (cc && set_of (cc, insn)))
> return NULL;
>
> return seq;
> @@ -2544,6 +2559,7 @@ noce_process_if_block (struct noce_if_info *if_info)
> rtx_insn *insn_a, *insn_b;
> rtx set_a, set_b;
> rtx orig_x, x, a, b;
> + rtx cc;
>
> /* We're looking for patterns of the form
>
> @@ -2655,6 +2671,13 @@ noce_process_if_block (struct noce_if_info *if_info)
> if_info->a = a;
> if_info->b = b;
>
> + /* Skip it if the instruction to be moved might clobber CC. */
> + cc = cc_in_cond (cond);
> + if (cc)
> + if (set_of (cc, insn_a)
> + || (insn_b && set_of (XEXP (cond, 0), insn_b)))
> + return FALSE;
> +
> /* Try optimizations in some approximation of a useful order. */
> /* ??? Should first look to see if X is live incoming at all. If it
> isn't, we don't need anything but an unconditional set. */
> @@ -2811,6 +2834,7 @@ check_cond_move_block (basic_block bb,
> rtx cond)
> {
> rtx_insn *insn;
> + rtx cc = cc_in_cond (cond);
>
> /* We can only handle simple jumps at the end of the basic block.
> It is almost impossible to update the CFG otherwise. */
> @@ -2868,6 +2892,10 @@ check_cond_move_block (basic_block bb,
> && modified_between_p (src, insn, NEXT_INSN (BB_END (bb))))
> return FALSE;
>
> + /* Skip it if the instruction to be moved might clobber CC. */
> + if (cc && set_of (cc, insn))
> + return FALSE;
> +
> vals->put (dest, src);
>
> regs->safe_push (dest);
> diff --git a/gcc/testsuite/gcc.dg/pr63917.c b/gcc/testsuite/gcc.dg/pr63917.c
> new file mode 100644
> index 0000000..422b15d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr63917.c
It should be pr64007.c.
> @@ -0,0 +1,45 @@
> +/* { dg-options " -O3 " } */
You need
/* { dg-do run } */
since it is a run-time test.
> +
> +int d, i;
> +
> +struct S
> +{
> + int f0;
> +} *b, c, e, h, **g = &b;
> +
> +static struct S *f = &e;
> +
> +int
> +fn1 (int p)
> +{
> + int a = 0;
> + return a || p < 0 || p >= 2 || 1 >> p;
> +}
> +
> +int
> +main ()
> +{
> + int k = 1, l, *m = &c.f0;
> +
> + for (;;)
> + {
> + l = fn1 (i);
> + *m = k && i;
> + if (l)
> + {
> + int n[1] = {0};
> + }
> + break;
> + }
> +
> + *g = &h;
> +
> + if (d)
> + (*m)--;
> + d = (f != 0) | (i >= 0);
> +
> + if (c.f0 != 0)
> + __builtin_abort ();
> +
> + return 0;
> +}
>
You test doesn't fail without the fix since your test is a little
bit different from the test at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64007
You missed:
assert (b);
Without it, the bug won't be triggered.
--
H.J.
