Re: [patch 0/2] PR49847: Add hook to place read-only lookup-tables in named address-space

[Georg-Johann Lay <avr at gjlay dot de>]

On 28.07.2017 09:34, Richard Biener wrote:
> On Thu, Jul 27, 2017 at 3:32 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
> > On 27.07.2017 14:34, Richard Biener wrote:
> > > On Thu, Jul 27, 2017 at 2:29 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
> > > > For some targets, the best place to put read-only lookup tables as
> > > > generated by -ftree-switch-conversion is not the generic address space
> > > > but some target specific address space.
> > > >
> > > > This is the case for AVR, where for most devices .rodata must be
> > > > located in RAM.
> > > >
> > > > Part #1 adds a new, optional target hook that queries the back-end
> > > > about the desired address space.  There is currently only one user:
> > > > tree-switch-conversion.c::build_one_array() which re-builds value_type
> > > > and array_type if the address space returned by the backend is not
> > > > the generic one.
> > > >
> > > > Part #2 is the AVR part that implements the new hook and adds some
> > > > sugar around it.
> > >
> > >
> > > Given that switch-conversion just creates a constant initializer doesn't
> > > AVR
> > > benefit from handling those uniformly (at RTL expansion?).  Not sure but
> > > I think it goes through the regular constant pool handling.
> > >
> > > Richard.
> >
> >
> > avr doesn't use constant pools because they are not profitable for
> > several reasons.
> >
> > Moreover, it's not sufficient to just patch the pool's section, you'd
> > also have to patch *all* accesses to also use the exact same address
> > space so that they use the correct instruction (LPM instead of LD).
>
> Sure.  So then not handle it in constant pool handling but in expanding
> the initializers of global vars.  I think the entry for this is
> varasm.c:assemble_variable - that sets DECL_RTL for the variable.

That cannot work, and here is why; just for completeness:

cgraphunit.c::symbol_table::compile()

runs

  ...
  expand_all_functions ();
  output_variables (); // runs assemble_variable
  ...

So any patching of DECL_RTL will result in wrong code: The address
space of the decl won't match the address space used by the code
accessing decl.

Johann