Issue with reload

Issue with reload

[Henri Cloetens]

Hello All,

I am working on a custom front-end, ran in an issue with reload. (step 277).
The GCC version is 9.2.0

a. After expand, (step 234), I get:

(insn 228 227 229 15 (set (reg:SI 186 [ x__lsm.224 ])
         (zero_extend:SI (subreg:HI (reg:SI 378) 0))) 
"/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/ldtoa.c":784:5 43 
{zero_extendhisi2_internal}
      (nil))

b. The machine does have the instruction to do this. 
(zero_extendhisi2_internal). Here is the pattern description:

(define_insn "zero_extendhisi2_internal"
   [(set (match_operand:SI 0 "gpc_reg_operand" "=r,r,r")
         (zero_extend:SI (match_operand:HI 1 "movhi_operand_0" "Z,C,r")))]

c. Then the whole code runs through reload, and there it somewhere goes 
wrong.
    The reloader changes the target register, and the operation gets 
distorted.
    The subregging operation is gone !!.

  583: r990:SI=zero_extend(r378:SI#0)

d. This then leads further to error. The reloader restructures:

  583: r990:SI=zero_extend(r1056:HI)
     Inserting slow/invalid mem reload before:
  1381: r1055:SI=[sfp:SI+0x2c]
  1382: r1056:HI=r1055:SI#0

but the last operation (1382) is impossible. The reloader gets in an 
infinite loop, and crashes on
max. number of reloads reached.

Now, yes, obviously, I made a mistake .. But where to look ?. What did I 
do wrong the subreg got lost ?.

Best Regards,

Henri.

Re: Issue with reload

[Henri Cloetens]

Hello All,

Sorry, I found it myself.

 > 583: r990:SI=zero_extend(r378:SI#0)

- The statement (r378:SI#0) takes the '0' subreg form reg r378:SI. 
(shorthand in the reload dump.)

 > (define_insn "zero_extendhisi2_internal"
 >   [(set (match_operand:SI 0 "gpc_reg_operand" "=r,r,r")
 >        (zero_extend:SI (match_operand:HI 1 "movhi_operand_0" "Z,C,r")))]

- However, the predicate "r" in the statement above, does not allow a 
subreg.
- So, I defined another predicate, that allows the subreg, and then it's 
gone.

Best Regards,

Henri


On 11/3/20 2:00 PM, Henri Cloetens wrote:
> Hello All,
>
> I am working on a custom front-end, ran in an issue with reload. (step 
> 277).
> The GCC version is 9.2.0
>
> a. After expand, (step 234), I get:
>
> (insn 228 227 229 15 (set (reg:SI 186 [ x__lsm.224 ])
>         (zero_extend:SI (subreg:HI (reg:SI 378) 0))) 
> "/home/henri/blueICe/gcc/newlib/newlib/libc/stdlib/ldtoa.c":784:5 43 
> {zero_extendhisi2_internal}
>      (nil))
>
> b. The machine does have the instruction to do this. 
> (zero_extendhisi2_internal). Here is the pattern description:
>
> (define_insn "zero_extendhisi2_internal"
>   [(set (match_operand:SI 0 "gpc_reg_operand" "=r,r,r")
>         (zero_extend:SI (match_operand:HI 1 "movhi_operand_0" "Z,C,r")))]
>
> c. Then the whole code runs through reload, and there it somewhere 
> goes wrong.
>    The reloader changes the target register, and the operation gets 
> distorted.
>    The subregging operation is gone !!.
>
>  583: r990:SI=zero_extend(r378:SI#0)
>
> d. This then leads further to error. The reloader restructures:
>
>  583: r990:SI=zero_extend(r1056:HI)
>     Inserting slow/invalid mem reload before:
>  1381: r1055:SI=[sfp:SI+0x2c]
>  1382: r1056:HI=r1055:SI#0
>
> but the last operation (1382) is impossible. The reloader gets in an 
> infinite loop, and crashes on
> max. number of reloads reached.
>
> Now, yes, obviously, I made a mistake .. But where to look ?. What did 
> I do wrong the subreg got lost ?.
>
> Best Regards,
>
> Henri.
>

Re: Issue with reload

[Segher Boessenkool]

Hi Henri,

On Tue, Nov 03, 2020 at 04:25:32PM +0100, Henri Cloetens wrote:
> > 583: r990:SI=zero_extend(r378:SI#0)
> 
> - The statement (r378:SI#0) takes the '0' subreg form reg r378:SI. 
> (shorthand in the reload dump.)

And it doesn't say what mode subreg it is (QImore or HImode here).
There are multiple cases like this, where the abbreviated RTL is not
conclusive (so cannot be parsed back).

> > (define_insn "zero_extendhisi2_internal"
> >   [(set (match_operand:SI 0 "gpc_reg_operand" "=r,r,r")
> >        (zero_extend:SI (match_operand:HI 1 "movhi_operand_0" 
> "Z,C,r")))]
> 
> - However, the predicate "r" in the statement above, does not allow a 
> subreg.

Yes it does.  See register_operand in recog.c for example.  It is of
course possible your gpc_reg_operand doesn't allow it (bad idea, but I
don't know your code).

You would normally start the gpc_reg_operand code with something like

  if (SUBREG_P (op))
    op = SUBREG_REG (op);

(as well as pretty much all other register operand predicates).


Segher

Re: Issue with reload

[Henri Cloetens]

Hello Segher,


- However, the predicate "r" in the statement above, does not allow a
subreg.

Yes it does.  See register_operand in recog.c for example.  It is of
course possible your gpc_reg_operand doesn't allow it (bad idea, but I
don't know your code).

It does not. When I change the description to :

  (define_insn "movhi_internal"
[(set (match_operand:HI 0 "movhi_operand_0" "=r,Z,C,r,r,u,u,r,r,r")
(match_operand:HI 1  "movhi_operand_1"       "x9,r,r,Z,C,Z,C,O,k,i"))]

with x9 defined as "ext_gpc_reg_operand"
and
(define_predicate "ext_gpc_reg_operand"
   (and  (match_operand 0 "register_operand")
         ( ior (match_test "( (GET_CODE(op) == REG && (
                         (REGNO(op) >= ARG_POINTER_REGNUM && !CA_REGNO_P(REGNO(op)))
                         || REGNO(op) == SFP_REGNO
                         || REGNO (op) == ARG_POINTER_REGNUM
                         || REGNO (op) <= MAX_REGFILE_REGNO)))")
               (match_test "( (GET_CODE(op) == SUBREG && SUBREG_BYTE(op) == 0 && (
                         (REGNO(SUBREG_REG(op)) >= ARG_POINTER_REGNUM && !CA_REGNO_P(REGNO(SUBREG_REG(op))))
                         || REGNO(SUBREG_REG(op)) == SFP_REGNO
                         || REGNO (SUBREG_REG(op)) == ARG_POINTER_REGNUM
                         || REGNO (SUBREG_REG(op)) <= MAX_REGFILE_REGNO)))"))))


Then, its gone. It now crashes on other "r"'s in the same instruction, 
as it apparently
wants to move this operand on the stack, and for that it needs the "x9" 
also in other places.

It is of course possible something is wrong in my description, but the 
"r" does not seem to allow the
subreg. At least not in my port.

I will try putting more "x9"'s tomorrow.

Best Regards,

Henri.

Re: Issue with reload

[Henri Cloetens]

Hello Segher, all,

I have the impression there is a small bug in the front-end.

//#define LOAD_EXTEND_OP(MODE) ZERO_EXTEND
#define LOAD_EXTEND_OP(MODE) UNKNOWN      // HACK HENRI !!.
(this is in my back-end)

I have to turn the LOAD_EXTEND_OP(MODE) to UNKNOWN.

The issue seems to be following:
- There are operations in the code of the type
R100:SI = zero_extend(R101:HI)
- Now, during reload, R101:HI is moved on the stack.
- Then, internally in the compiler, there is confusion on the 
zero_extend operator.
   a. If the load-extend-mode is ZERO_EXTEND:
       there is confusion as to use the MOVHI instruction or the 
ZERO_EXTEND instruction.
       (they both do the same.) This seems to lead to the strange bugs.
       (It seems to me there are optimizers that recognize the 
ZERO_EXTEND as MOVHI, and others
        that dont, leading to the crashes.)
   b. If I turn it off, all seems to go OK.
- So, I turned it off ... Hope this does not lead to problems.

Best Regards,

Henri.


On 11/3/20 6:23 PM, Segher Boessenkool wrote:
> Hi Henri,
>
> On Tue, Nov 03, 2020 at 04:25:32PM +0100, Henri Cloetens wrote:
>>> 583: r990:SI=zero_extend(r378:SI#0)
>> - The statement (r378:SI#0) takes the '0' subreg form reg r378:SI.
>> (shorthand in the reload dump.)
> And it doesn't say what mode subreg it is (QImore or HImode here).
> There are multiple cases like this, where the abbreviated RTL is not
> conclusive (so cannot be parsed back).
>
>>> (define_insn "zero_extendhisi2_internal"
>>>    [(set (match_operand:SI 0 "gpc_reg_operand" "=r,r,r")
>>>          (zero_extend:SI (match_operand:HI 1 "movhi_operand_0"
>> "Z,C,r")))]
>>
>> - However, the predicate "r" in the statement above, does not allow a
>> subreg.
> Yes it does.  See register_operand in recog.c for example.  It is of
> course possible your gpc_reg_operand doesn't allow it (bad idea, but I
> don't know your code).
>
> You would normally start the gpc_reg_operand code with something like
>
>    if (SUBREG_P (op))
>      op = SUBREG_REG (op);
>
> (as well as pretty much all other register operand predicates).
>
>
> Segher