Re: [PATCH v4] S/390: Allow relative addressing of literal pool entries

[Ilya Leoshkevich <iii at linux dot ibm dot com>]

> Am 09.11.2018 um 18:30 schrieb Ulrich Weigand <uweigand@de.ibm.com>:
> 
> Ilya Leoshkevich wrote:
> 
>>   gcc_assert (GET_CODE (x) != SYMBOL_REF
>> -	      || !CONSTANT_POOL_ADDRESS_P (x));
>> +	      || !CONSTANT_POOL_ADDRESS_P (x)
>> +	      || s390_symbol_relative_long_p (x));
> 
> Hmm, it's a bit weird that this routine now uses a different check
> than the other two.  It would look more straightforward for
> find_constant_pool_ref to use the same s390_safe_relative_long_p
> check as the others.
> 
> (This would then also make s390_symbol_relative_long_p redundant.)

Ok, sounds good.

> If we do that, it might even make sense to pull the 
> s390_safe_relative_long_p check up into the callers:
> 
> E.g. in s390_mainpool_finish, replace
> 
>  /* Replace all literal pool references.  */
> 
>  for (rtx_insn *insn = get_insns (); insn; insn = NEXT_INSN (insn))
>    {
>      if (NONJUMP_INSN_P (insn) || CALL_P (insn))
>        {
> 
> with
> 
>      if ((NONJUMP_INSN_P (insn) || CALL_P (insn))
>          && !s390_safe_relative_long_p (insn))
> 
> (This last change is just a suggestion, only if it makes the
> overall code simpler and more readable.)

I would have to do that in 8 places (4 for find/replace and 4 for 
annotate), so it’s probably better to keep the current 3 checks.