Discussion:
LRA reload produces invalid insn
Paul Koning
2018-11-02 00:25:05 UTC
Permalink
I'm running the testsuite on the pdp11 target, and I get a failure when using LRA that works correctly with the old allocator. The issue is that LRA is producing an insn that is invalid (it violates the constraints stated in the insn definition).

The insn in the IRA dump looks like this:

(insn 240 238 241 13 (set (reg/f:HI 155)
(plus:HI (reg/f:HI 5 r5)
(const_int -58 [0xffffffffffffffc6]))) "/Users/pkoning/Documents/svn/combined/gcc/testsuite/gcc.c-torture/execute/builtins/strcat-chk.c":68:7 68 {addhi3}
(expr_list:REG_EQUIV (plus:HI (reg/f:HI 5 r5)
(const_int -58 [0xffffffffffffffc6]))
(nil)))

(note that R5 is FRAME_POINTER_REGNUM.)

The reload dump from LRA shows this:

(insn 240 238 241 13 (set (reg/f:HI 5 r5 [155])
(plus:HI (reg/f:HI 6 sp)
(const_int 12 [0xc]))) "/Users/pkoning/Documents/svn/combined/gcc/testsuite/gcc.c-torture/execute/builtins/strcat-chk.c":68:7 68 {addhi3}
(expr_list:REG_EQUIV (plus:HI (reg/f:HI 5 r5)
(const_int -58 [0xffffffffffffffc6]))
(nil)))

But that's not valid because ADD is a two-operand instruction:

(define_insn_and_split "addhi3"
[(set (match_operand:HI 0 "nonimmediate_operand" "=rR,rR,Q,Q")
(plus:HI (match_operand:HI 1 "general_operand" "%0,0,0,0")
(match_operand:HI 2 "general_operand" "rRLM,Qi,rRLM,Qi")))]

The old allocator produces two insns for this:

(insn 443 238 240 13 (set (reg/f:HI 5 r5 [155])
(const_int 12 [0xc])) "/Users/pkoning/Documents/svn/combined/gcc/testsuite/gcc.c-torture/execute/builtins/strcat-chk.c":68:7 25 {movhi}
(nil))
(insn 240 443 241 13 (set (reg/f:HI 5 r5 [155])
(plus:HI (reg/f:HI 5 r5 [155])
(reg/f:HI 6 sp))) "/Users/pkoning/Documents/svn/combined/gcc/testsuite/gcc.c-torture/execute/builtins/strcat-chk.c":68:7 68 {addhi3}
(expr_list:REG_EQUIV (plus:HI (reg/f:HI 6 sp)
(const_int 12 [0xc]))
(nil)))

which is the correct sequence given the matching operand constraint in the define_insn.

Is this an LRA bug, or is there something I need to do in the target to prevent this happening?

paul
Peter Bergner
2018-11-02 00:49:36 UTC
Permalink
Post by Paul Koning
I'm running the testsuite on the pdp11 target, and I get a failure when using LRA that works correctly with the old allocator. The issue is that LRA is producing an insn that is invalid (it violates the constraints stated in the insn definition).
[snip]
Post by Paul Koning
which is the correct sequence given the matching operand constraint in the define_insn.
Is this an LRA bug, or is there something I need to do in the target to prevent this happening?
What do you mean by "old allocator"? Just an older revision? Does it work before my
revision 264897 commit and broken after? If so, could you try the following to see
whether that fixes things for you?

https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01757.html

My commit above exposed some latent LRA bugs and my patch above tries
to fix issues similar to what you're seeing.

Peter
Segher Boessenkool
2018-11-02 01:40:57 UTC
Permalink
Hi Peter,
Post by Peter Bergner
Post by Paul Koning
I'm running the testsuite on the pdp11 target, and I get a failure when using LRA that works correctly with the old allocator. The issue is that LRA is producing an insn that is invalid (it violates the constraints stated in the insn definition).
[snip]
Post by Paul Koning
which is the correct sequence given the matching operand constraint in the define_insn.
Is this an LRA bug, or is there something I need to do in the target to prevent this happening?
What do you mean by "old allocator"?
I think Paul just means old reload.


Segher
Peter Bergner
2018-11-02 02:21:38 UTC
Permalink
Post by Segher Boessenkool
Hi Peter,
Post by Peter Bergner
Post by Paul Koning
I'm running the testsuite on the pdp11 target, and I get a failure when using LRA that works correctly with the old allocator. The issue is that LRA is producing an insn that is invalid (it violates the constraints stated in the insn definition).
[snip]
Post by Paul Koning
which is the correct sequence given the matching operand constraint in the define_insn.
Is this an LRA bug, or is there something I need to do in the target to prevent this happening?
What do you mean by "old allocator"?
I think Paul just means old reload.
In that case, my patch may still help.

Peter
Paul Koning
2018-11-02 12:18:13 UTC
Permalink
Post by Peter Bergner
Post by Paul Koning
I'm running the testsuite on the pdp11 target, and I get a failure when using LRA that works correctly with the old allocator. The issue is that LRA is producing an insn that is invalid (it violates the constraints stated in the insn definition).
[snip]
Post by Paul Koning
which is the correct sequence given the matching operand constraint in the define_insn.
Is this an LRA bug, or is there something I need to do in the target to prevent this happening?
What do you mean by "old allocator"? Just an older revision? Does it work before my
revision 264897 commit and broken after? If so, could you try the following to see
whether that fixes things for you?
https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01757.html
My commit above exposed some latent LRA bugs and my patch above tries
to fix issues similar to what you're seeing.
Peter
That doesn't cure this particular problem; the ICE is still there with the same error message (identical failing insn) as before.

paul
Vladimir Makarov
2018-11-02 03:37:20 UTC
Permalink
Post by Paul Koning
I'm running the testsuite on the pdp11 target, and I get a failure when using LRA that works correctly with the old allocator. The issue is that LRA is producing an insn that is invalid (it violates the constraints stated in the insn definition).
(insn 240 238 241 13 (set (reg/f:HI 155)
(plus:HI (reg/f:HI 5 r5)
(const_int -58 [0xffffffffffffffc6]))) "/Users/pkoning/Documents/svn/combined/gcc/testsuite/gcc.c-torture/execute/builtins/strcat-chk.c":68:7 68 {addhi3}
(expr_list:REG_EQUIV (plus:HI (reg/f:HI 5 r5)
(const_int -58 [0xffffffffffffffc6]))
(nil)))
(note that R5 is FRAME_POINTER_REGNUM.)
Is this an LRA bug, or is there something I need to do in the target to prevent this happening?
It is hard to say whose code is responsible for this.  It might be a
wrong machine-dependent code or a LRA bug.

Paul, could you send me full LRA dump file (.reload).  It might help me
to say more specific reason for the bug.  LRA has iterated sub-passes
and the full dump can say where LRA started to behave wrongly.
Peter Bergner
2018-11-02 13:34:07 UTC
Permalink
Post by Paul Koning
Is this an LRA bug, or is there something I need to do in the target to prevent this happening?
It is hard to say whose code is responsible for this.  It might be a wrong machine-dependent code or a LRA bug.
Paul, could you send me full LRA dump file (.reload).  It might help me to say more specific reason for the bug.  LRA has iterated sub-passes and the full dump can say where LRA started to behave wrongly.
I'll note that when we ported the rs6000 (ie, ppc*) port over to LRA
from reload, we hit many target problems. It seems LRA is much less
forgiving to bad constraints, predicates, etc. than reload was.
I think that's actually a good thing.

Peter
Paul Koning
2018-11-02 13:46:26 UTC
Permalink
Post by Peter Bergner
Post by Paul Koning
Is this an LRA bug, or is there something I need to do in the target to prevent this happening?
It is hard to say whose code is responsible for this. It might be a wrong machine-dependent code or a LRA bug.
Paul, could you send me full LRA dump file (.reload). It might help me to say more specific reason for the bug. LRA has iterated sub-passes and the full dump can say where LRA started to behave wrongly.
I'll note that when we ported the rs6000 (ie, ppc*) port over to LRA
from reload, we hit many target problems. It seems LRA is much less
forgiving to bad constraints, predicates, etc. than reload was.
I think that's actually a good thing.
Peter
Yes, I ran into that, and Segher (I think) helped me with a bad predicate case. It doesn't help that the documentation isn't all that explicit about what the requirements are.

paul

Loading...