Re: [PATCH 01/13] [APX EGPR] middle-end: Add insn argument to base_reg_class

[Uros Bizjak <ubizjak@gmail.com>]

On Wed, Sep 6, 2023 at 9:43 PM Vladimir Makarov <vmakarov@redhat.com> wrote:
>
>
> On 9/1/23 05:07, Hongyu Wang wrote:
> > Uros Bizjak via Gcc-patches <gcc-patches@gcc.gnu.org> 于2023年8月31日周四 18:16写道：
> >> On Thu, Aug 31, 2023 at 10:20 AM Hongyu Wang <hongyu.wang@intel.com> wrote:
> >>> From: Kong Lingling <lingling.kong@intel.com>
> >>>
> >>> Current reload infrastructure does not support selective base_reg_class
> >>> for backend insn. Add insn argument to base_reg_class for
> >>> lra/reload usage.
> >> I don't think this is the correct approach. Ideally, a memory
> >> constraint should somehow encode its BASE/INDEX register class.
> >> Instead of passing "insn", simply a different constraint could be used
> >> in the constraint string of the relevant insn.
> > We tried constraint only at the beginning, but then we found the
> > reload infrastructure
> > does not work like that.
> >
> > The BASE/INDEX reg classes are determined before choosing alternatives, in
> > process_address under curr_insn_transform. Process_address creates the mem
> > operand according to the BASE/INDEX reg class. Then, the memory operand
> > constraint check will evaluate the mem op with targetm.legitimate_address_p.
> >
> > If we want to make use of EGPR in base/index we need to either extend BASE/INDEX
> > reg class in the backend, or, for specific insns, add a target hook to
> > tell reload
> > that the extended reg class with EGPR can be used to construct memory operand.
> >
> > CC'd Vladimir as git send-mail failed to add recipient.
> >
> >
> I think the approach proposed by Intel developers is better.  In some way
> we already use such approach when we pass memory mode to get the base
> reg class.  Although we could use different memory constraints for
> different modes when the possible base reg differs for some memory
> modes.
>
> Using special memory constraints probably can be implemented too (I
> understand attractiveness of such approach for readability of the
> machine description).  But in my opinion it will require much bigger
> work in IRA/LRA/reload.  It also significantly slow down RA as we need
> to process insn constraints for processing each memory in many places
> (e.g. for calculation of reg classes and costs in IRA).  Still I think
> there will be a few cases for this approach resulting in a bigger
> probability of assigning hard reg out of specific base reg class and
> this will result in additional reloads.
>
> So the approach proposed by Intel is ok for me.  Although if x86 maintainers
> are strongly against this approach and the changes in x86 machine
> dependent code and Intel developers implement Uros approach, I am
> ready to review this.  But still I prefer the current Intel developers
> approach for reasons I mentioned above.

My above proposal is more or less a wish from a target maintainer PoV.
Ideally, we would have a bunch of different memory constraints, and a
target hook that returns corresponding BASE/INDEX reg classes.
However, I have no idea about the complexity of the implementation in
the infrastructure part of the compiler.

Uros.