This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR63404, gcc 5 miscompiles linux block layer
- From: Richard Henderson <rth at redhat dot com>
- To: Jiong Wang <jiong dot wang at arm dot com>, Jeff Law <law at redhat dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 10 Oct 2014 09:52:35 -0700
- Subject: Re: [PATCH] PR63404, gcc 5 miscompiles linux block layer
- Authentication-results: sourceware.org; auth=none
- References: <5429A102 dot 4000602 at arm dot com> <5429A5D9 dot 9040404 at redhat dot com> <5429B204 dot 7040200 at arm dot com> <542A2FC5 dot 4020605 at redhat dot com> <542AC02D dot 70508 at arm dot com> <542ADA9D dot 8070709 at redhat dot com> <CAAfDdZ0ti4nfkOL1qTHTtsgpYLCmC5NQcM=_GEBFfJHpcx+XCw at mail dot gmail dot com> <543558B8 dot 3060006 at arm dot com> <54380267 dot 8030705 at redhat dot com> <54380BAF dot 9000602 at arm dot com>
On 10/10/2014 09:39 AM, Jiong Wang wrote:
>> (1) Don't bother modifying single_set; just look for a bare SET.
>> (2) Tighten the set of expressions we're willing to move.
>> (3) Use direct "return false" in the failure case, rather than
>> counting a non-zero number of non-constants, etc.
>>
>> Tested on x86_64, and against Andi's test case (unfortunately unreduced).
>
> minor nit, after this patch, gcc.target/aarch64/shrink_wrap_symbol_ref_1.c
> still not shrink-wrapped under -mabi=ilp32,
> the reason is as Pinski reported, LO_SUM is with with RTX_OBJ class, while it
> could be treated as expression.
> in your fix, if it's RTX_OBJ, and LO_SUM, then the move aborted by return NULL.
I missed that message. Interesting.
I wonder what kind of fallout there would be from changing LO_SUM to
RTX_BIN_ARITH, which is what it should have been all along.
My guess is that someone looked at HIGH being RTX_CONST_OBJ and thought that
LO_SUM should also be a kind of object too. But it's really a lot more like a
kind of PLUS. If instead we had a LOW to match HIGH it would have been
(plus reg (low symbol))
and thus more obvious.
I'll see if I can bootstrap such a change on e.g. sparc or ppc32, which uses
lo_sum heavily.
> anyway, thanks for the fix, I verified the shrink-wrapping of some hotpot
> functions in benchmark on aarch64 are not affected.
Thanks for the additional testing.
r~