This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, ARM, LRA] Prepare ARM build with LRA


Eric Botcazou <ebotcazou@adacore.com> writes:
>> FWIW, I'd prefer to keep it as-is, since must_be_base_p (x) and
>> must_be_index_p (x) don't imply that we should look specifically at
>> XEXP (x, 0) (rather than just X, or XEXP (x, 1), etc.).  I think it's
>> better to keep the code tests and the associated XEXPs together.
>
> Feel free to revert this part, but then add appropriate comments explaining 
> why we are interested in LO_SUM for set_address_base and in MULT and the 5 
> others for set_address_index.  If it's because the former is rather a base 
> than an index and vice-versa for the latter, then it's even clearer with the 
> predicates I think.

The idea is that must_be_base_p and must_be_index_p are used when deciding
how to classify the operands of a PLUS.  set_address_base and set_address_index
are called once we've decided which is which and want to record the choice.
There's no backing out by that stage.

So in the set_* routines it isn't about whether the value is definitely
a base or a definitely an index.  It's just about drilling down through
what we've already decided is a base or index to get the inner reg or mem,
and knowing which XEXPs to look at.  We could instead have used a for_each_rtx,
or something like that, without any code checks.  But I wanted to be precise
about the types of address we allow, so that we can assert for things we
don't understand.  In other words, it was "designed" to require the kind
of extension Yvan is adding here.

E.g. although it's a bit of a stretch, it might be in future that
someone wants to allow NOT in an index.  It would then make sense
to add NOT to must_be_index_p.  But the existing check:

  if ((GET_CODE (*inner) == MULT || GET_CODE (*inner) == ASHIFT)
      && CONSTANT_P (XEXP (*inner, 1)))
    inner = strip_address_mutations (&XEXP (*inner, 0));

wouldn't make sense as:

  if (must_be_index_p (*inner) && CONSTANT_P (XEXP (*inner, 1)))
    inner = strip_address_mutations (&XEXP (*inner, 0));

in that case, since NOT only has one operand.  And it might be that
the NOT is allowed only outside the scale, only inside the scale,
or in both positions.

Not the best example, sorry.

Thanks,
Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]