This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR 61225
- From: Jeff Law <law at redhat dot com>
- To: Zhenqiang Chen <zhenqiang dot chen at arm dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 09 Dec 2014 11:51:51 -0700
- Subject: Re: [PATCH] Fix PR 61225
- Authentication-results: sourceware.org; auth=none
- References: <CACgzC7A7+JXBQ-HCcoqjhmbjoALCi8nngJy2J8SXHF1GRvyQPQ at mail dot gmail dot com> <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>
On 12/09/14 02:49, Zhenqiang Chen wrote:
Do you need to verify SETA and SETB satisfy single_set? Or has that
already been done elsewhere?
A is NEXT_INSN (insn)
B is prev_nonnote_nondebug_insn (insn),
For I1 -> I2 -> B; I2 -> A;
LOG_LINK can make sure I1 and I2 are single_set, but not A and B. And I did
found codes in function try_combine, which can make sure B (or I3) is
single_set.
So I think the check can skip failed cases at early stage.
Thanks for doing the research on this.
The check is to make sure the correctness. Here is a case,
int
f1 (int *x)
{
int t = --*x;
if (!t)
foo (x);
return t;
}
_4 = *x_3(D);
_5 = _4 + -1;
*x_3(D) = _5;
# DEBUG t => _5
if (_5 == 0)
...
<bb 4>:
return _5;
"_5" is used in "return _5". So we can not remove "_5 = _4 + -1".
Right, but ISTM that if the # uses > 2, then we could just return false
rather than bothering to see if all the uses are consumed by A or B.
It's not a big deal, I just have a hard time seeing that doing something
more complex than "if (# uses > 2) return false;" makes sense.
So you've got two new combine cases here, but I think the testcase only
tests one of them. Can you include a testcase for both of hte major
paths above (I1->I2->I3; I2->insn and I2->I3; I2->INSN)
pr61225.c is the case to cover I1->I2->I3; I2->insn.
For I2 -> I3; I2 -> insn, I tried my test cases and found peephole2 can also
handle them. So I removed the code from the patch.
Seems like the reasonable thing to do.
Here is the final patch.
Bootstrap and no make check regression on X86-64.
ChangeLog:
2014-11-09 Zhenqiang Chen <zhenqiang.chen@linaro.org>
Part of PR rtl-optimization/61225
* combine.c (can_reuse_cc_set_p): New function.
(combine_instructions): Handle I1 -> I2 -> I3; I2 -> insn.
(try_combine): Add one more parameter TO_COMBINED_INSN, which
is used to create a new insn parallel (TO_COMBINED_INSN, I3).
testsuite/ChangeLog:
2014-11-09 Zhenqiang Chen<zhenqiang.chen@linaro.org>
* gcc.target/i386/pr61225.c: New test.
OK for the trunk.
jeff