Re: [RFC][PR target PR90000] (rs6000) Compile time hog w/impossible asm constraint lra loop
On Thu, Nov 12, 2020 at 09:15:11AM -0700, Jeff Law wrote:
> > void foo (void)
> > {
> > register float __attribute__ ((mode(SD))) r31 __asm__ ("r31");
> > register float __attribute__ ((mode(SD))) fr1 __asm__ ("fr1");
> >
> > __asm__ ("#" : "=d" (fr1));
> > r31 = fr1;
> > __asm__ ("#" : : "r" (r31));
> > }
>
> Looking at this again after many months away, I wonder the real problem
> is the reloads we have to generate for copies to/from he fr1 local
> variable, which is bound to hard reg fr1 rather than the asm statements
> themselves. It's not clear to me from the BZ and I don't have a PPC
> cross handy to look directly.
We should never do a reload of a (local) register variable.
Unfortunately we cannot currently tell during reload that something is
one!
See also PR97708, and many more, going many years back.
Segher
Re: [RFC][PR target PR90000] (rs6000) Compile time hog w/impossible asm constraint lra loop
On 4/23/20 9:48 AM, will schmidt wrote:
> On Wed, 2020-04-22 at 12:26 -0600, Jeff Law wrote:
>> On Fri, 2020-04-10 at 16:40 -0500, will schmidt via Gcc-patches
>> wrote:
>>> [RFC][PR target/9] Compile time hog w/impossible asm constraint
>>> lra loop
>>>
>>> Hi,
>>> RFC for a bandaid/patch to partially address target PR/9.
>>>
>>> This adds an escape condition from the forever loop where
>>> LRA gets stuck while attempting to handle constraints from an
>>> instruction that has previously suffered an impossible constraint
>>> error.
>>>
>>> This is somewhat inspired by MAX_RELOAD_INSNS_NUMBER as
>>> seen in lra-constraints.c lra_constraints(). This utilizes the
>>> existing counter variable lra_constraint_iter.
>>>
>>> More needs to be done here, as this does replace a spin-forever
>>> situation with an ICE.
>>>
>>> Thanks
>>> -Will
>>>
>>>
>>> gcc/
>>> 2020-04-10 Will Schmidt
>>>
>>> * lra.c: Add include of rtl-error.h.
>>> (MAX_LRA_CONSTRAINT_PASSES): New define.
>>> (lra): Add check of lra_constraint_iter value.
>> Doesn't this argue that there's some other datastructure that needs
>> to be updated
>> when we removed the impossible asm?
> Yes, i think so. I'm just not sure exactly what or where.
> The submitted patch is minimally allowing for manageable-in-size reload
> dumps for my continued debug. :-)
>
> There is an old patch that addressed what looks like a similar issue,
> but i wasn't able to directly apply that to this situation without
> failing in other places.
>
>> commit e86c0101ae59b32c3f10edcca78398cbf8848eaa
>> Author: Steven Bosscher
>> Date: Thu Jan 24 10:30:26 2013 +
>>re PR inline-asm/55934 (LRA inline asm error recovery)
> Which does a bit more, but at it's core is this:
>
> + PATTERN (insn) = gen_rtx_USE (VOIDmode, const0_rtx);
> + lra_set_insn_deleted (insn);
>
>
> I suspect this particular scenario with the testcase is a dependency across
> several 'insns', so marking just one as deleted is not enough.
> (but i'm not sure,..
>
> void foo (void)
> {
> register float __attribute__ ((mode(SD))) r31 __asm__ ("r31");
> register float __attribute__ ((mode(SD))) fr1 __asm__ ("fr1");
>
> __asm__ ("#" : "=d" (fr1));
> r31 = fr1;
> __asm__ ("#" : : "r" (r31));
> }
Looking at this again after many months away, I wonder the real problem
is the reloads we have to generate for copies to/from he fr1 local
variable, which is bound to hard reg fr1 rather than the asm statements
themselves. It's not clear to me from the BZ and I don't have a PPC
cross handy to look directly.
jeff
Re: [RFC][PR target PR90000] (rs6000) Compile time hog w/impossible asm constraint lra loop
On Wed, 2020-04-22 at 12:26 -0600, Jeff Law wrote:
> On Fri, 2020-04-10 at 16:40 -0500, will schmidt via Gcc-patches
> wrote:
> > [RFC][PR target/9] Compile time hog w/impossible asm constraint
> > lra loop
> >
> > Hi,
> > RFC for a bandaid/patch to partially address target PR/9.
> >
> > This adds an escape condition from the forever loop where
> > LRA gets stuck while attempting to handle constraints from an
> > instruction that has previously suffered an impossible constraint
> > error.
> >
> > This is somewhat inspired by MAX_RELOAD_INSNS_NUMBER as
> > seen in lra-constraints.c lra_constraints(). This utilizes the
> > existing counter variable lra_constraint_iter.
> >
> > More needs to be done here, as this does replace a spin-forever
> > situation with an ICE.
> >
> > Thanks
> > -Will
> >
> >
> > gcc/
> > 2020-04-10 Will Schmidt
> >
> > * lra.c: Add include of rtl-error.h.
> > (MAX_LRA_CONSTRAINT_PASSES): New define.
> > (lra): Add check of lra_constraint_iter value.
>
> Doesn't this argue that there's some other datastructure that needs
> to be updated
> when we removed the impossible asm?
Yes, i think so. I'm just not sure exactly what or where.
The submitted patch is minimally allowing for manageable-in-size reload
dumps for my continued debug. :-)
There is an old patch that addressed what looks like a similar issue,
but i wasn't able to directly apply that to this situation without
failing in other places.
>commit e86c0101ae59b32c3f10edcca78398cbf8848eaa
>Author: Steven Bosscher
>Date: Thu Jan 24 10:30:26 2013 +
>re PR inline-asm/55934 (LRA inline asm error recovery)
Which does a bit more, but at it's core is this:
+ PATTERN (insn) = gen_rtx_USE (VOIDmode, const0_rtx);
+ lra_set_insn_deleted (insn);
I suspect this particular scenario with the testcase is a dependency across
several 'insns', so marking just one as deleted is not enough.
(but i'm not sure,..
void foo (void)
{
register float __attribute__ ((mode(SD))) r31 __asm__ ("r31");
register float __attribute__ ((mode(SD))) fr1 __asm__ ("fr1");
__asm__ ("#" : "=d" (fr1));
r31 = fr1;
__asm__ ("#" : : "r" (r31));
}
thanks
-Will
>
> Jeff
> >
>
>
Re: [RFC][PR target PR90000] (rs6000) Compile time hog w/impossible asm constraint lra loop
On Fri, 2020-04-10 at 16:40 -0500, will schmidt via Gcc-patches wrote: > [RFC][PR target/9] Compile time hog w/impossible asm constraint lra loop > > Hi, > RFC for a bandaid/patch to partially address target PR/9. > > This adds an escape condition from the forever loop where > LRA gets stuck while attempting to handle constraints from an > instruction that has previously suffered an impossible constraint error. > > This is somewhat inspired by MAX_RELOAD_INSNS_NUMBER as > seen in lra-constraints.c lra_constraints(). This utilizes the > existing counter variable lra_constraint_iter. > > More needs to be done here, as this does replace a spin-forever > situation with an ICE. > > Thanks > -Will > > > gcc/ > 2020-04-10 Will Schmidt > > * lra.c: Add include of rtl-error.h. > (MAX_LRA_CONSTRAINT_PASSES): New define. > (lra): Add check of lra_constraint_iter value. Doesn't this argue that there's some other datastructure that needs to be updated when we removed the impossible asm? Jeff >
[RFC][PR target PR90000] (rs6000) Compile time hog w/impossible asm constraint lra loop
[RFC][PR target/9] Compile time hog w/impossible asm constraint lra loop
Hi,
RFC for a bandaid/patch to partially address target PR/9.
This adds an escape condition from the forever loop where
LRA gets stuck while attempting to handle constraints from an
instruction that has previously suffered an impossible constraint error.
This is somewhat inspired by MAX_RELOAD_INSNS_NUMBER as
seen in lra-constraints.c lra_constraints(). This utilizes the
existing counter variable lra_constraint_iter.
More needs to be done here, as this does replace a spin-forever
situation with an ICE.
Thanks
-Will
gcc/
2020-04-10 Will Schmidt
* lra.c: Add include of rtl-error.h.
(MAX_LRA_CONSTRAINT_PASSES): New define.
(lra): Add check of lra_constraint_iter value.
diff --git a/gcc/lra.c b/gcc/lra.c
index 5e8b75b..36f5dd9 100644
--- a/gcc/lra.c
+++ b/gcc/lra.c
@@ -116,15 +116,22 @@ along with GCC; see the file COPYING3.If not see
#include "ira.h"
#include "recog.h"
#include "expr.h"
#include "cfgrtl.h"
#include "cfgbuild.h"
+#include "rtl-error.h"
#include "lra.h"
#include "lra-int.h"
#include "print-rtl.h"
#include "function-abi.h"
+/* Bail-out from lra-constraint processing.
+ This band-aid prevents an infinite loop after we have previously
+ experienced an invalid constraint error, and our lra processing
+ has gotten stuck repeatedly inserting instructions. (PR/90060) */
+#define MAX_LRA_CONSTRAINT_PASSES 100
+
/* Dump bitmap SET with TITLE and BB INDEX. */
void
lra_dump_bitmap_with_title (const char *title, bitmap set, int index)
{
unsigned int i;
@@ -2520,10 +2527,17 @@ lra (FILE *f)
if (! live_p) {
/* We need the correct reg notes for work of constraint sub-pass.
*/
lra_create_live_ranges (true, true);
live_p = true;
}
+ if (lra_constraint_iter > MAX_LRA_CONSTRAINT_PASSES)
+ {
+ internal_error
+ ("maximum number of lra constraint passes achieved (%d)",
+MAX_LRA_CONSTRAINT_PASSES);
+ break;
+ }
}
/* Don't clear optional reloads bitmap until all constraints are
satisfied as we need to differ them from regular reloads. */
bitmap_clear (_optional_reload_pseudos);
bitmap_clear (_subreg_reload_pseudos);
