This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: "Bin.Cheng" <amker dot cheng at gmail dot com>
- Cc: Jeff Law <law at redhat dot com>, Richard Earnshaw <rearnsha at arm dot com>, Bin Cheng <bin dot cheng at arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 1 Sep 2014 06:38:50 -0500
- Subject: Re: [PATCH PR62151]Fix uninitialized register issue caused by distribute_notes in combine pass
- Authentication-results: sourceware.org; auth=none
- References: <000a01cfc1de$cac1c230$60454690$ at arm dot com> <53FDB440 dot 5030904 at arm dot com> <CAHFci28BS75pySKVtLTeY8BQsYupJHvXs73ZdYq1RkfHLbXocQ at mail dot gmail dot com> <5401680D dot 60907 at redhat dot com> <20140831121844 dot GA8949 at gate dot crashing dot org> <CAHFci28OzU8wOyYnWxF1jSOn7jnUTubZ=Mjv=y7NhZR=C2G5BQ at mail dot gmail dot com>
On Mon, Sep 01, 2014 at 11:36:07AM +0800, Bin.Cheng wrote:
> > In the testcase (and comment in the proposed patch), why is combine
> > combining four insns at all? That means it rejected combining just the
> > first three. Why did it do that?
> It is explicitly reject by below code in can_combine_p.
>
> if (GET_CODE (PATTERN (i3)) == PARALLEL)
> for (i = XVECLEN (PATTERN (i3), 0) - 1; i >= 0; i--)
> if (GET_CODE (XVECEXP (PATTERN (i3), 0, i)) == CLOBBER)
> {
> /* Don't substitute for a register intended as a clobberable
> operand. */
> rtx reg = XEXP (XVECEXP (PATTERN (i3), 0, i), 0);
> if (rtx_equal_p (reg, dest))
> return 0;
>
> Since insn i2 in the list of i0/i1/i2 as below contains parallel
> clobber of dest_of_insn76/use_of_insn77.
> 32: r84:SI=0
> 76: flags:CC=cmp(r84:SI,0x1)
> REG_DEAD r84:SI
> 77: {r84:SI=-ltu(flags:CC,0);clobber flags:CC;}
> REG_DEAD flags:CC
> REG_UNUSED flags:CC
Archaeology suggests this check is because the clobber might be an
earlyclobber. Which seems silly: how can it be a valid insn at all
in that case? It seems to me the check can just be removed. That
will hide your issue, maybe even solve it (but I doubt it).
If not, then combining the four insns (your case that explodes)
should not be allowed either (it's just the same, with a register
copy tucked on the end). I haven't looked, but a missing can_combine_p
call perhaps?
Another question is why is r84 set twice in the first place?
Segher