LRA reload produces invalid insn

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

LRA reload produces invalid insn

Paul Koning-6
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

Reply | Threaded
Open this post in threaded view
|

Re: LRA reload produces invalid insn

Peter Bergner-4
On 11/1/18 7:25 PM, Paul Koning wrote:
> 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]
> 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

Reply | Threaded
Open this post in threaded view
|

Re: LRA reload produces invalid insn

Segher Boessenkool
Hi Peter,

On Thu, Nov 01, 2018 at 07:49:36PM -0500, Peter Bergner wrote:
> On 11/1/18 7:25 PM, Paul Koning wrote:
> > 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]
> > 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
Reply | Threaded
Open this post in threaded view
|

Re: LRA reload produces invalid insn

Peter Bergner-4
On 11/1/18 8:40 PM, Segher Boessenkool wrote:

> Hi Peter,
>
> On Thu, Nov 01, 2018 at 07:49:36PM -0500, Peter Bergner wrote:
>> On 11/1/18 7:25 PM, Paul Koning wrote:
>>> 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]
>>> 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

Reply | Threaded
Open this post in threaded view
|

Re: LRA reload produces invalid insn

Vladimir Makarov
In reply to this post by Paul Koning-6
On 11/01/2018 08:25 PM, Paul Koning wrote:

> 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.)
>
>
>
> 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.
Reply | Threaded
Open this post in threaded view
|

Re: LRA reload produces invalid insn

Paul Koning-6
In reply to this post by Peter Bergner-4


> On Nov 1, 2018, at 8:49 PM, Peter Bergner <[hidden email]> wrote:
>
> On 11/1/18 7:25 PM, Paul Koning wrote:
>> 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]
>> 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

Reply | Threaded
Open this post in threaded view
|

Re: LRA reload produces invalid insn

Peter Bergner-4
In reply to this post by Vladimir Makarov
On 11/1/18 10:37 PM, Vladimir Makarov wrote:
> On 11/01/2018 08:25 PM, Paul Koning wrote:
>> 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

Reply | Threaded
Open this post in threaded view
|

Re: LRA reload produces invalid insn

Paul Koning-6


> On Nov 2, 2018, at 9:34 AM, Peter Bergner <[hidden email]> wrote:
>
> On 11/1/18 10:37 PM, Vladimir Makarov wrote:
>> On 11/01/2018 08:25 PM, Paul Koning wrote:
>>> 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