How to recognize registers after reload ?.

How to recognize registers after reload ?.

[Henri Cloetens]

Hello all,

I have an issue with my custom compiler back-end after reload.

1. The machine has move-instructions that can do following:
    a. From register file to (register indirect offset) or to register file.
    b. From (register indirect offset), register file, immediate to 
register file

2. I have now 3 patterns to emit moves in the .md-file:

     1. movesi_internal_fromreg (from register file to memory or 
register file)
     2. movesi_internal_toreg (from memory , immediate, register file to 
register file)
     3. movesi : define_expand. It has C-code that analyses the operands 
and selects 1,2 or both.

Motivation for the split was problems with the "combine" step. Suppose 
following code:
     *a = 10.
     Even if my front_end (define_expand) splits this in
     r100 = 10
    *r101 = r100
    the combine step, if these is only one movesi_internal, willl group 
it again, to then find out
    there is no instruction pattern.

Now, it being split, it goes wrong during reload. The issue is, that the 
code of the define_expand is wrong.
I there uses the tests (GET_CODE(operands[0,1]) == REG), and this is not 
correct any more. I mean,
this evaluated true also for a register that the reload step knows needs 
to end up in memory.
Question :
   - What tests are there to find out an operand is intended to be a 
pseudo in memory, and not a register ?.
   - What is a register, especially during reload ?.
     The issue is the movesi_internal_fromreg:

(define_insn "movsi_internal_fromreg"
   [(set (match_operand:SI 0 "memoryIndirect_or_reg" "r,MemInd,r"
           (match_operand:SI 1 "gpc_reg_operand" "r,r,           MemInd"))]
""
..

Please note the MemInd qualifier on the second line, at the end.
   - When I add this, the reload goes fine.
   - Without it, it gets in an endless loop. The backend is called with 
the "from" operand
      actually a pseudo on the stack, and this is incorrectly recognized 
as a register.
   - In my opinion, it needs to be solved by changing the 
"define_expand" for "movsi", so that it
     recognizes the variable on the stack, and calls the other 
instruction : "movsi_internal_toreg".
     Only I dont know how to recognize the stackvar.

Best Regards,

Henri.

Re: How to recognize registers after reload ?.

[Jeff Law]

On 10/22/20 2:02 AM, Henri Cloetens wrote:
> Hello all,
>
> I have an issue with my custom compiler back-end after reload.
>
> 1. The machine has move-instructions that can do following:
>    a. From register file to (register indirect offset) or to register
> file.
>    b. From (register indirect offset), register file, immediate to
> register file
>
> 2. I have now 3 patterns to emit moves in the .md-file:
>
>     1. movesi_internal_fromreg (from register file to memory or
> register file)
>     2. movesi_internal_toreg (from memory , immediate, register file
> to register file)
>     3. movesi : define_expand. It has C-code that analyses the
> operands and selects 1,2 or both.
>
> Motivation for the split was problems with the "combine" step. Suppose
> following code:
>     *a = 10.
>     Even if my front_end (define_expand) splits this in
>     r100 = 10
>    *r101 = r100
>    the combine step, if these is only one movesi_internal, willl group
> it again, to then find out
>    there is no instruction pattern.

This is an indication the insn's condition or operand's predicate or
operand constraints are wrong.

Jeff

Re: How to recognize registers after reload ?.

[Segher Boessenkool]

On Thu, Oct 22, 2020 at 03:30:08PM -0600, Jeff Law via Gcc-help wrote:
> On 10/22/20 2:02 AM, Henri Cloetens wrote:
> > Motivation for the split was problems with the "combine" step. Suppose
> > following code:
> >     *a = 10.
> >     Even if my front_end (define_expand) splits this in
> >     r100 = 10
> >    *r101 = r100
> >    the combine step, if these is only one movesi_internal, willl group
> > it again, to then find out
> >    there is no instruction pattern.
> 
> This is an indication the insn's condition or operand's predicate or
> operand constraints are wrong.

Yes, but I do not understand what Henri means at all.

On one side, combine will try to combine any such pair, and then it
does discover there is no insn for that, and then not do the
combination.  This is exactly what combine is supposed to do.

On the other side, it could mean combine *does* allow the combo.  Then,
you *do* have a define_insn for it, or it would *not* allow it.  And
then some time later that is a problem?  But that has nothing to do with
combine, that just is a buggy machine description.

(My money is on the predicate btw ;-) )


Segher

Re: How to recognize registers after reload ?.

[Jeff Law]

On 10/22/20 4:24 PM, Segher Boessenkool wrote:
> On Thu, Oct 22, 2020 at 03:30:08PM -0600, Jeff Law via Gcc-help wrote:
>> On 10/22/20 2:02 AM, Henri Cloetens wrote:
>>> Motivation for the split was problems with the "combine" step. Suppose
>>> following code:
>>>     *a = 10.
>>>     Even if my front_end (define_expand) splits this in
>>>     r100 = 10
>>>    *r101 = r100
>>>    the combine step, if these is only one movesi_internal, willl group
>>> it again, to then find out
>>>    there is no instruction pattern.
>> This is an indication the insn's condition or operand's predicate or
>> operand constraints are wrong.
> Yes, but I do not understand what Henri means at all.
>
> On one side, combine will try to combine any such pair, and then it
> does discover there is no insn for that, and then not do the
> combination.  This is exactly what combine is supposed to do.
>
> On the other side, it could mean combine *does* allow the combo.  Then,
> you *do* have a define_insn for it, or it would *not* allow it.  And
> then some time later that is a problem?  But that has nothing to do with
> combine, that just is a buggy machine description.
>
> (My money is on the predicate btw ;-) )

I'd bet on the predicates and the insn condition.  I wouldn't be
surprised at all if this is a risc-like architecture where only one
operand can be a non-register.  Predicates can't really describe that,
so it's usually handled in the insn predicate.


jeff

>
>
> Segher
>

Re: How to recognize registers after reload ?.

[Henri Cloetens]

Hello Segher, Jeff,

- The machine is indeed such that for each operation there is only one
   non-register operand possible.
- Take a first example, to describe the problem with the combine:
void set_to_one(int *a)
{
*a = 1 ;
}

In the machine, this becomes:

R30 = 1  // move immediate to register
[R20] = R30 // move to *a
return

Now, to get this, there is not one movsi - pattern, because if there is 
only one,
combine will combine both moves into something like
[R20] = 1
and this does not exist, and combine crashes. So, there are 2 moves:
/movesi_internal_fromreg/ (moving from a register to memory or register)
/movesi_internal_toreg/ (moving from immediate, memory, register to 
register).

This is all nice and fine, until the reload step. In case the number of 
internal registers is
exceeded, stuff needs to go on the stack. Now, suppose I have the operation

R30 = 1

and the compiler wants to put R30 on the stack, it sees this is not 
possible, and will make a
helper move :

R30 = 1 (old one)
R100 = R30

and then, it will try to put R100 on the stack. Now, to do the move, 
R100 = R30, it calls the
/movsi/ pattern in the machine description. Only, it declares in the RTX 
R100 as a register operand,
which it is not, or not entirely. It is a stack operand, but my 
/define_expand movsi/ recognizes it as a register
operand, end emits /movesi_internal_toreg/, while it should emit 
/movesi_internal_fromreg/, and the
whole system ends in an endless loop.
To solve this decently, I need to find in the /define_expand movsi/ if 
R100 is a stack operand or not.
There is one way, that is to implement TARGET_SECONDARY_RELOAD, have 
that parse all the arguments,
put it in an operand database, and have /define_expand movsi/ 
interrogate this database. Now, I will only
do that if there is no other option.

Best Regards,

Henri.
>

AW: How to recognize registers after reload ?.

[Stefan Franke]

> -----Ursprüngliche Nachricht-----
> Von: Gcc-help <gcc-help-bounces@gcc.gnu.org> Im Auftrag von Henri
> Cloetens
> Gesendet: Freitag, 23. Oktober 2020 09:29
> An: Jeff Law <law@redhat.com>; Segher Boessenkool
> <segher@kernel.crashing.org>
> Cc: gcc-help <gcc-help@gcc.gnu.org>
> Betreff: Re: How to recognize registers after reload ?.
> 
> Hello Segher, Jeff,
> 
> - The machine is indeed such that for each operation there is only one
>    non-register operand possible.
> - Take a first example, to describe the problem with the combine:
> void set_to_one(int *a)
> {
> *a = 1 ;
> }
> 
> In the machine, this becomes:
> 
> R30 = 1  // move immediate to register
> [R20] = R30 // move to *a
> return
> 
> Now, to get this, there is not one movsi - pattern, because if there is only
> one, combine will combine both moves into something like [R20] = 1 and this
> does not exist, and combine crashes. So, there are 2 moves:
> /movesi_internal_fromreg/ (moving from a register to memory or register)
> /movesi_internal_toreg/ (moving from immediate, memory, register to
> register).
> 
> This is all nice and fine, until the reload step. In case the number of internal
> registers is exceeded, stuff needs to go on the stack. Now, suppose I have
> the operation
> 
> R30 = 1
> 
> and the compiler wants to put R30 on the stack, it sees this is not possible,
> and will make a helper move :
> 
> R30 = 1 (old one)
> R100 = R30
> 
> and then, it will try to put R100 on the stack. Now, to do the move,
> R100 = R30, it calls the
> /movsi/ pattern in the machine description. Only, it declares in the RTX
> R100 as a register operand,
> which it is not, or not entirely. It is a stack operand, but my /define_expand
> movsi/ recognizes it as a register operand, end emits
> /movesi_internal_toreg/, while it should emit /movesi_internal_fromreg/,
> and the whole system ends in an endless loop.
> To solve this decently, I need to find in the /define_expand movsi/ if
> R100 is a stack operand or not.
> There is one way, that is to implement TARGET_SECONDARY_RELOAD, have
> that parse all the arguments, put it in an operand database, and have
> /define_expand movsi/ interrogate this database. Now, I will only do that if
> there is no other option.
> 
> Best Regards,
> 
> Henri.
> >

Henri,

it seems your define_expand for movsi is too narrow. It should accept all mandatory forms. The distinction - to register/memory - should be done in movsi itself. There is also no need for a database to distinguish.

Regards

Stefan

Re: AW: How to recognize registers after reload ?.

[Henri Cloetens]

Thank you Stefan,

How do you mean too narrow ?.
Can you be more specific ?, how should I change it, so that 'combine' 
does not see non-exisiting patterns, while 'reload' is able to move
to stack, even when declaring a stack variable as a register ?.

Best Regards,

Henri
> Henri,
>
> it seems your define_expand for movsi is too narrow. It should accept all mandatory forms. The distinction - to register/memory - should be done in movsi itself. There is also no need for a database to distinguish.
>
> Regards
>
> Stefan
>
>

Re: How to recognize registers after reload ?.

[Segher Boessenkool]

Hi,

On Fri, Oct 23, 2020 at 09:28:49AM +0200, Henri Cloetens wrote:
> R30 = 1  // move immediate to register
> [R20] = R30 // move to *a
> return
> 
> Now, to get this, there is not one movsi - pattern, because if there is 
> only one,
> combine will combine both moves into something like
> [R20] = 1
> and this does not exist, and combine crashes.

You keep saying this.  The combine pass should *not* crash, and you
haven't shown any evidence it did.  It will attempt to do this
combination, and your backend will either or not say that is an existing
insn; in neither case will combine crash.

Some later pass might well not like what you said is a valid insn.  Just
say *that* if that is what you mean!



Segher

Re: How to recognize registers after reload ?.

[Segher Boessenkool]

On Fri, Oct 23, 2020 at 09:35:20AM +0200, Stefan Franke wrote:
> it seems your define_expand for movsi is too narrow. It should accept all mandatory forms. The distinction - to register/memory - should be done in movsi itself. There is also no need for a database to distinguish.

The requirement is that reload (which should be LRA on all newer ports,
and is recommended for all ports) always has a path to reload whatever
is thrown at it.  This does mean you need both r->m and m->r in one
pattern normally, yes (all r's can be spilled to stack, and there is no
m->m instruction).  Of course you get by far the best code if you do
have all normal moves in one pattern (one per mode), as you say.

You disallow m->m in the insn condition, like
  "gpc_reg_operand (operands[0], SImode)
   || gpc_reg_operand (operands[1], SImode)"


Segher

Re: How to recognize registers after reload ?.

[Henri Cloetens]

Hello Segher all,

Thank you very much !.

You disallow m->m in the insn condition, like
   "gpc_reg_operand (operands[0], SImode)
    || gpc_reg_operand (operands[1], SImode)"


This is the part I missed !. So, I will update next week, means
ONE movsi_internal, and add a condition, like you suggest, to
avoid 'combine' of doing an illegal combination.

Best Regards,

Henri.