This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR 61225
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Jeff Law <law at redhat dot com>
- Cc: Zhenqiang Chen <zhenqiang dot chen at arm dot com>, gcc-patches at gcc dot gnu dot org
- Date: Wed, 10 Dec 2014 07:47:33 -0600
- Subject: Re: [PATCH] Fix PR 61225
- Authentication-results: sourceware.org; auth=none
- References: <CABu31nP5TqCU3iB-3jBTU93bORrCAmohq5nzcVBdjx2cqabrzA at mail dot gmail dot com> <CACgzC7D40=bxqag59GYQuXtze_ZAquo3PhpLmh0EGxZte5Mw4A at mail dot gmail dot com> <53C73EBD dot 9070909 at redhat dot com> <CACgzC7CJnoaOsFMDjXDZ0CLiECWPbejxDHg2GOk_AUtjrzC80w at mail dot gmail dot com> <547CE75B dot 6090709 at redhat dot com> <000001d00f9e$64f43600$2edca200$ at arm dot com> <54861817 dot 4030409 at redhat dot com> <000301d01395$675d5890$361809b0$ at arm dot com> <20141209190744 dot GA8314 at gate dot crashing dot org> <54874A52 dot 3030304 at redhat dot com>
On Tue, Dec 09, 2014 at 12:15:30PM -0700, Jeff Law wrote:
> >>@@ -3323,7 +3396,11 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn
> >>*i1, rtx_insn *i0,
> >> rtx old = newpat;
> >> total_sets = 1 + extra_sets;
> >> newpat = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (total_sets));
> >>- XVECEXP (newpat, 0, 0) = old;
> >>+
> >>+ if (to_combined_insn)
> >>+ XVECEXP (newpat, 0, --total_sets) = old;
> >>+ else
> >>+ XVECEXP (newpat, 0, 0) = old;
> >> }
> >
> >Is this correct? If so, it needs a big fat comment, because it is
> >not exactly obvious :-)
> >
> >Also, it doesn't handle at all the case where the new pattern already is
> >a PARALLEL; can that never happen?
> I'd convinced myself it was. But yes, a comment here would be good.
>
> Presumably you're thinking about a PARALLEL that satisfies single_set_p?
I wasn't thinking about anything in particular; this code does not handle
a PARALLEL newpat with to_combined_insn correctly, and it doesn't say it
cannot happen.
But yes, I don't see why it could not happen? E.g. a parallel of multiple
sets with all but one of those dead?
Why should it be single_set here anyway? (Maybe I need more coffee, sorry
if so).
Segher