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

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

> Am 09.11.2018 um 14:43 schrieb Ulrich Weigand <uweigand@de.ibm.com>:
> 
> Ilya Leoshkevich wrote:
> 
>> +  /* Return unannotated constant pool references, so that the corresponding
>> +     entries are added to the back-end-managed pool.  Not doing so would result
>> +     in those entries being placed in the middle-end-managed pool, which would
>> +     in turn prevent merging them with identical ones in the back-end-managed
>> +     pool.  */
> 
> I'm still not convinced this is necessary; how frequently does the case occur
> that a constant is duplicated, and is it worth the extra code complexity?
> (Also, note that the middle-end pool is shared across the whole translation
> unit, while the back-end pool is duplicated in every function ... so if the
> same constant is used by many functions, forcing it into the back-end pool
> may *increase* overall size.)
> 
> In any case, does this even happen with your current code?  You find the
> unannotated reference here, and therefore copy the constant into the local
> pool, but then replace_constant_pool_ref will still ignore the insn, and
> thus the insn will actually continue to refer to the middle-end pool
> constant, *not* the copy created in the local pool.  Right?

Whoops, that’s right.  This code must be currently achieving nothing at
all!  I did not run into such cases and I don’t have a test case for
them, so they are hypothetical.  So let’s keep it simple here.

> 
>>       if (DISP_IN_RANGE (INTVAL (frame_off)))
>> 	{
>> -	  insn = gen_rtx_SET (frame_pointer,
>> -			      gen_rtx_PLUS (Pmode, frame_pointer, frame_off));
>> -	  insn = emit_insn (insn);
>> +	  insn = emit_insn (copy_rtx (cfa));
>> 	}
> 
> This seems unrelated?

Now that insn is of type rtx_insn *, gen_rtx_SET () result cannot be
assigned to it.  Let me rewrite this by creating an rtx temporary,
which should be more readable.