Richard Sandiford <rdsandif...@googlemail.com> writes:
> Jeff Law <l...@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.sucha...@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.
> 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. What is
likely to happen overall is:
(lo_sum (high x) (const (plus x ofs))) -> (const (plus x ofs))
Followed by something splitting the simplified result because it is not
valid and getting:
(set (reg) (const (plus x ofs)))
->
(set (reg) (high (const (plus x ofs))))
(set (reg) (lo_sum (reg) (const (plus x ofs))))
The code that combined high/lo_sum has to validate the result and this
will generally mean it has to be split again I expect.
Thanks,
Matthew
