This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC, PATCH][LRA, MIPS] ICE: in decompose_normal_address, at rtlanal.c:5817
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- Cc: Jeff Law <law at redhat dot com>, Robert Suchanek <Robert dot Suchanek at imgtec dot com>, Steven Bosscher <stevenb dot gcc at gmail dot com>, "vmakarov\ at redhat dot com" <vmakarov at redhat dot com>, "gcc-patches\ at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 10 Jan 2015 22:09:20 +0000
- Subject: Re: [RFC, PATCH][LRA, MIPS] ICE: in decompose_normal_address, at rtlanal.c:5817
- Authentication-results: sourceware.org; auth=none
- References: <B5E67142681B53468FAF6B7C31356562440589AC at hhmail02 dot hh dot imgtec dot org> <CABu31nPzNNhAOG6RE3pEu+REkNRu-kA9poQOGNJxeAqmBrRmZQ at mail dot gmail dot com> <B5E67142681B53468FAF6B7C3135656244116DFB at hhmail02 dot hh dot imgtec dot org> <54B00CAA dot 60207 at redhat dot com> <87h9vyg5n7 dot fsf at googlemail dot com> <6D39441BF12EF246A7ABCE6654B0235320F9D8B7 at LEMAIL01 dot le dot imgtec dot org>
Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> Richard Sandiford <rdsandiford@googlemail.com> writes:
>> Jeff Law <law@redhat.com> writes:
>> > On 01/09/15 04:32, Robert Suchanek wrote:
>> >> Hi Steven/Vladimir,
>> >>
>> >>> It's hard to say what the correct fix should be, but it sounds like
>> >>> the address you get after the substitutions should be simplified
>> >>> (folded).
>> >>
>> >> Coming back to the original testcase and re-analyzing the problem, it
>> >> appears that there is, indeed, a missing case for simplification of
>> LO_SUM/HIGH pair.
>> >> The patch attached resolves the issue.
>> >>
>> >> Although the testcase is not reproducible on the trunk, I think it is
>> >> still worth to include it in case the ICE reoccurred.
>> >>
>> >> The patch has been bootstrapped and regtested on
>> >> x86_64-unknown-linux-gnu target and similarly tested against SVN
>> >> revision r212763 where it can be reproduced.
>> >>
>> >> Regards,
>> >> Robert
>> >>
>> >> 2015-01-08 Robert Suchanek <robert.suchanek@imgtec.com>
>> >>
>> >> gcc/
>> >> * simplify-rtx.c (simplify_replace_fn_rtx): Simplify (lo_sum (high
>> x)
>> >> (const (plus x offset))) to (const (plus x offset)).
>> > You have to be careful here. Whether or not this transformation is
>> > valid depends on the size of the offset and whether or not the port
>> > has an overlap between its sethi and losum insns and whether or not
>> > any rounding occurs when applying the relocations for sethi/losum as
>> > well as potentially other factors.
>> >
>> > We don't currently have a way for ports to indicate what offsets would
>> > make this kind of simplification valid. In fact, there's at least
>> one
>> > port (PA) where the validity of this kind of simplification can't be
>> > determined until after LRA/reload when you know the full context of
>> > how the result is going to be used.
>>
>> I agree this is the kind of thing we'd need to consider if we were
>> deciding whether it's valid to connect a (lo_sum (high x+N) x+N) to an
>> existing (high x). But this code is handling cases where the connection
>> has already been made and we're trying to simplify the result. Would it
>> be valid RTL to use:
>>
>> (lo_sum (high x) (const (plus x offset)))
>>
>> to mean anything other than x+offset?
>>
>> Hmm, admittedly the documentation doesn't guarantee that...
>
> I guess so. I took the phrasing below for (high:m exp) to mean that high
> only made sense when used with lo_sum.
>
> "It is used with lo_sum to represent the typical two-instruction sequence
> used in RISC machines to reference a global memory location"
>
> I don't know if that was the intended meaning though. There is nothing
> to state that lo_sum has to be used with high though so lo_sum could
> be applied to a different base quite legitimately it seems.
Yeah, it's also used for small-data accesses, where the base is the
global pointer. You can also add a variable offset in between the
high and lo_sum, which is the main reason we can't just take the
second operand of the lo_sum as golden. Seeing the high is important.
>> If we do go for the change, I think we should generalise it to handle
>> (lo_sum (high x+N) x+N') and (lo_sum (high x-N) x) too. Things like
>> get_related_value or split_const could help there.
>
> Seems reasonable to cover these cases but I think the code already does
> that (this is directly above the new case Robert added):
>
> /* (lo_sum (high x) x) -> x */
> if (GET_CODE (op0) == HIGH && rtx_equal_p (XEXP (op0, 0), op1))
> return op1;
>
> So, (lo_sum (high x) x) is already simplified to x but the special case
> where a constant is only added to the lo_sum operand is missing.
No, the cases I was describing were where the high is a (const (plus x -N))
and the lo_sum is just x, or where the high is (const (plus x N)) and the
lo_sum is (const (plus x N')).
I.e. rather than have the two special cases of (lo_sum (high x) x)
and (lo_sum (high x) (const (plus x N))), we should handle all cases
of (lo_sum (high x) y) in which y and x have the same base.
E.g. if we have an int array called "x" that is aligned to 64 bits,
we can access x[3] from the same high as x[2]. Neither the existing
code nor the patch would be able to simplify that back to x[3].
Thanks,
Richard
- References:
- RE: [RFC, PATCH][LRA, MIPS] ICE: in decompose_normal_address, at rtlanal.c:5817
- Re: [RFC, PATCH][LRA, MIPS] ICE: in decompose_normal_address, at rtlanal.c:5817
- Re: [RFC, PATCH][LRA, MIPS] ICE: in decompose_normal_address, at rtlanal.c:5817
- RE: [RFC, PATCH][LRA, MIPS] ICE: in decompose_normal_address, at rtlanal.c:5817