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,avr]: Ad PR rtl-optimization/52543: Undo the MEM->UNSPEC hack


Denis Chertykov wrote:
Georg-Johann wrote:
Denis Chertykov wrote:
Georg-Johann Lay wrote:

PR52543 required to represent a load from non-generic address spaces as
UNSPEC
instead of as MEM to avoid a gross code bloat.

http://gcc.gnu.org/PR52543

lower-subreg's cost model is still broken: It assumes that any loads from
MEM
are from the generic address space and does not care for address spaces
in its
cost model.

This patch undoes the changes from SVN r185605

http://gcc.gnu.org/viewcvs?view=revision&revision=185605

and installs a different but less intrusive hack around PR52543:

targetm.mode_dependent_address_p has an address space parameter so that
the
backend can pretend all non-generic addresses are mode-dependent.

This keeps lower-subreg.c from splitting the loads, and it is possible to
represent the loads as MEM and there is no more the need to represent
them as
UNSPECs.

This patch is still not an optimal solution but the code is much closer
to a
clean solution now.

Ok for trunk?

You can apply it.


Denis.

I also applied the following change:


http://gcc.gnu.org/viewcvs?view=revision&revision=191825

        * config/avr/avr.md (adjust_len): Add lpm.
        (reload_in<mode>): Use avr_out_lpm for output.  Use "lpm" for
        adjust_len.
        * config/avr/avr-protos.h (avr_out_lpm): New prototype.
        * config/avr/avr.c (avr_out_lpm): Make global.
        (adjust_insn_length): Handle ADJUST_LEN_LPM.

The reload_in<mode> insns used the wrong output functions.

Notice that this change is just a cosmetic change because the secondary
reload for the non-generic spaces are ignored.  That is:  despite
avr_secondary_reload, REG <- MEM input reloads are not mapped to their
secondary reload insn and the mov insn for that load is used.

This leads to a situation where the insn output function is not supplied
with the needed clobber register, thus the avr_find_unused_d_reg function is
needed to work around that.

What would happen if no unused_d_reg ?

In that case the output function backup a d-reg in tmp_reg to get one d-register free, similar to the situation in other output functions that cook up a scratch, avr.c:output_reload_in_const is one example.


It is the else case in avr.c:avr_out_lpm(), %5 stands for tmp_reg:

      xop[4] = GEN_INT (segment);
      xop[3] = avr_find_unused_d_reg (insn, lpm_addr_reg_rtx);

      if (xop[3] != NULL_RTX)
        {
          avr_asm_len ("ldi %3,%4" CR_TAB
                       "out %i6,%3", xop, plen, 2);
        }
      else if (segment == 1)
        {
          avr_asm_len ("clr %5" CR_TAB
                       "inc %5" CR_TAB
                       "out %i6,%5", xop, plen, 3);
        }
      else
        {
          avr_asm_len ("mov %5,%2"   CR_TAB
                       "ldi %2,%4"   CR_TAB
                       "out %i6,%2"  CR_TAB
                       "mov %2,%5", xop, plen, 4);
        }

Denis, do you know why the secondary reloads requested by
avr_secondary_reload are ignored?  I see calls to this hook and sri->icode
is set to the right insn code but ignored afterwards.

The only calls to that hook with the right operands are from ira cost
computation.

I have tried to use secondary a few years ago (may be 5 or 7). I have definitely remember only one thing: secondary reload should be avoided as long as possible.

Currently each mov has to be decorated with moving the segment to RAMPZ and (depending on target) restoring RAMPZ afterwards.


GCC has no concept of a segmented layout and there is no way to describe that.

One way is to hack with UNSPEC and bypass ira/reload altogether but IMO that is no good solution. Besides that is only works because the mov insns have special constraints (there will be writes to flash, flash does not change after load time, etc.)

The better way to got a knowledge about it is a GDB ;-)

I think reload.c:push_secondary_reload() should be the right place but it does not call targetm.secondary_reload so that no secondary is generated.


It's hard to tell where the place is that is responsible for the bypassing of calling the hook.

From the internals I don't see why it is skipped and the responsiveness in
the gcc-help@ list on such topics is zero :-(

IMHO it's a question to gcc@ not to gcc-help@

Ok, I will try my luck again.


Do you have an idea for a better approach, i.e. not set RAMPZ over and over again?

One idea is mode_switching pass but I did not try if it is worth the effort.


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