[Bug rtl-optimization/99581] New: internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread()

[Bug rtl-optimization/99581] New: internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread()

[raj.khem at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

            Bug ID: 99581
           Summary: internal compiler error: during RTL pass: final - void
                    QTWTF::TCMalloc_PageHeap::scavengerThread()
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: raj.khem at gmail dot com
  Target Milestone: ---

Created attachment 50384
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50384&action=edit
testcase

when building the attached source for ppc64 target GCC is ICE'ing

$ powerpc64le-yoe-linux-musl-g++ a.cpp -O2 -c

during RTL pass: final
a.cpp: In member function 'void QTWTF::TCMalloc_PageHeap::scavengerThread()':
a.cpp:15583:1: internal compiler error: in print_operand_address, at
config/rs6000/rs6000.c:14082
15583 | }
      | ^
0x1532d58 internal_error(char const*, ...)
        ???:0
0x5e46b7 fancy_abort(char const*, int, char const*)
        ???:0
0x991823 output_address(machine_mode, rtx_def*)
        ???:0
0x991771 output_operand(rtx_def*, int)
        ???:0
0x99241c output_asm_insn(char const*, rtx_def**)
        ???:0
0x996228 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
        ???:0

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread()

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0
            Summary|internal compiler error:    |[11 Regression] internal
                   |during RTL pass: final -    |compiler error: during RTL
                   |void                        |pass: final - void
                   |QTWTF::TCMalloc_PageHeap::s |QTWTF::TCMalloc_PageHeap::s
                   |cavengerThread()            |cavengerThread()

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread()

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Reduced testcase:
struct A { void foo () { __asm__("stw %1, %0" : "=o" (c) : "r" (0)); } int c; }
d;
struct B { A *e; B (A *g) : e(g) { e->foo (); } };
struct C { void *i; };
struct D { void bar (); A e; C l[68]; } m[68];
struct E { E (A *); };
int p;
void D::bar () { E (&m[p].e); }
void baz () { B h(&d); }

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11 Regression] internal    |[11 Regression] internal
                   |compiler error: during RTL  |compiler error: during RTL
                   |pass: final - void          |pass: final - void
                   |QTWTF::TCMalloc_PageHeap::s |QTWTF::TCMalloc_PageHeap::s
                   |cavengerThread()            |cavengerThread() since
                   |                            |r11-7526
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-03-15
                 CC|                            |segher at gcc dot gnu.org,
                   |                            |vmakarov at gcc dot gnu.org
           Priority|P3                          |P1
     Ever confirmed|0                           |1
           Keywords|                            |ra

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r11-7526-g9105757a59b890194ebf5b4fcbacd58db34ef332
o in =o is a generic constraint though (but maybe the bug is in the rs6000
backend).

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
C test (for -O2 too):
char e[37540];
struct A { int c; } d;

void
bar (int n)
{
  __asm__("" : : "r" (e));
}

void
foo (void)
{
  __asm__("stw %1, %0" : "=o" (d.c) : "r" (0));
}

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #4 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
I've reproduced it too and started to work on it.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Thanks Vladimir.  It is indeed a problem in LRA (or triggered by it).
We have
    8: {[r121:DI+low(unspec[`*.LANCHOR0',%2:DI]
47+0x92a4)]=asm_operands;clobber

so this is an offset that is too big for a machine instruction, those can take
-32768..32767.

Changing the constraint to "m" you get in LRA
    Inserting insn reload before:
   13: r121:DI=high(unspec[`*.LANCHOR0',%2:DI] 47+0x92a4)

but this doesn't happen if you keep it "o", and it dies later.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #6 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #5)
> Thanks Vladimir.  It is indeed a problem in LRA (or triggered by it).
> We have
>     8: {[r121:DI+low(unspec[`*.LANCHOR0',%2:DI]
> 47+0x92a4)]=asm_operands;clobber
> 
> so this is an offset that is too big for a machine instruction, those can
> take
> -32768..32767.
> 
> Changing the constraint to "m" you get in LRA
>     Inserting insn reload before:
>    13: r121:DI=high(unspec[`*.LANCHOR0',%2:DI] 47+0x92a4)
> 
> but this doesn't happen if you keep it "o", and it dies later.

The problem is that before the patches we used wrongly '=' as a constraint for
memory and after the patches we use (rightly) 'o' as the constraint.

The culprit is a function added by Richard Sandiford for arm64 sve:

commit 1aeffdce2dfe718e1337d75eb4f22c3c300df9bb
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Mon Nov 18 15:26:07 2019 +0000

    LRA: handle memory constraints that accept more than "m"

    LRA allows address constraints that are more relaxed than "p":

      /* Target hooks sometimes don't treat extra-constraint addresses as
         legitimate address_operands, so handle them specially.  */
      if (insn_extra_address_constraint (cn)
          && satisfies_address_constraint_p (&ad, cn))
        return change_p;

    For SVE it's useful to allow the same thing for memory constraints.
    The particular use case is LD1RQ, which is an SVE instruction that
    addresses Advanced SIMD vector modes and that accepts some addresses
    that normal Advanced SIMD moves don't.

    Normally we require every memory to satisfy at least "m", which is
    defined to be a memory "with any kind of address that the machine
    supports in general".  However, LD1RQ is very much special-purpose:
    it doesn't really have any relation to normal operations on these
    modes.  Adding its addressing modes to "m" would lead to bad Advanced
    SIMD optimisation decisions in passes like ivopts.  LD1RQ therefore
    has a memory constraint that accepts things "m" doesn't.

...

static bool
valid_address_p (rtx op, struct address_info *ad,
                 enum constraint_num constraint)
{
  address_eliminator eliminator (ad);

  /* Allow a memory OP if it matches CONSTRAINT, even if CONSTRAINT is more     
     forgiving than "m".                                                        
     Need to extract memory from op for special memory constraint,              
     i.e. bcst_mem_operand in i386 backend.  */
  if (MEM_P (extract_mem_from_operand (op))
      && (insn_extra_memory_constraint (constraint)
          || insn_extra_special_memory_constraint (constraint))
=>    && constraint_satisfied_p (op, constraint))
    return true;

  return valid_address_p (ad->mode, *ad->outer, ad->as);
}

He actually added the if-stmt.  And the condition of this if-stmt is true for
our case because constraint_satisfied_p returns true for the memory and
CONSTRAINT_o.  If the condition were false, we would use machine-dependent
legitimate_address_p and it would return false and we would reload the memory
address.

constraint_satisfied_p returns true because **infrastructure** function
offsettable_nonstrict_memref_p returns true for the memory.

So you are right it is not ppc64 target code problem.  But I am stuck right now
how to fix the PR w/o breaking arm sve.  Right now I see only adding a
machine-dependent hook but I don't like it as we already have too many hooks
for RA.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #7 from Segher Boessenkool <segher at gcc dot gnu.org> ---
>From the offending patch:

-/* Return true if the eliminated form of AD is a legitimate target address. 
*/
+/* Return true if the eliminated form of AD is a legitimate target address.
+   If OP is a MEM, AD is the address within OP, otherwise OP should be
+   ignored.  CONSTRAINT is one constraint that the operand may need
+   to meet.  */
 static bool
-valid_address_p (struct address_info *ad)
+valid_address_p (rtx op, struct address_info *ad,
+                enum constraint_num constraint)

The addition of those extra args makes clear that the function is no
longer just testing if it is a valid address.  It should be renamed.
And perhaps most callers should still use the old version, the one that
actually tests if something is a valid address?

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Rather than a target hook, isn't it a property of a particular constraint?
This constraint implies "m", this one doesn't?
Make the implies "m" behavior the default one and add some syntax in the *.md
files to declare constraints that have the new behavior?
Kind like define_memory_constraint vs. define_special_memory_constraint
difference.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #9 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #8)
> Rather than a target hook, isn't it a property of a particular constraint?
> This constraint implies "m", this one doesn't?
> Make the implies "m" behavior the default one and add some syntax in the
> *.md files to declare constraints that have the new behavior?
> Kind like define_memory_constraint vs. define_special_memory_constraint
> difference.

Yes, I think it is better solution.  I guess aarch64 might have the same
problem as in this PR.  We should have different treatment of memory
constraints even for one target.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #10 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #7)

> The addition of those extra args makes clear that the function is no
> longer just testing if it is a valid address.  It should be renamed.
> 

I don't like it too.  When I first look at the patch I though it is a recursive
call.  For active C++ programmer function overloading is not the problem but
imho it is better to rename the function/

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #11 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
Introducing a new memory constraint can take some time.

I guess we could switch off the offending code meanwhile because it is compiler
crash vs unoptimal generated code choice.

I'll investigate how switching the code off affects GCC tests on aarhc64.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #12 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
(In reply to Vladimir Makarov from comment #11)
> Introducing a new memory constraint can take some time.
> 
> I guess we could switch off the offending code meanwhile because it is
> compiler crash vs unoptimal generated code choice.
> 
> I'll investigate how switching the code off affects GCC tests on aarhc64.

Unfortunately, switching off the code results in 480 GCC test failures on
aarhc64.

I'll work on introducing new memory constraint.  I hope to have a patch and
submit it for review on Friday.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #13 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Sorry for not responding until now, but would it work to make
the "o" constraint check memory_address_addr_space_p too,
like the other memory constraints do?  IMO it's wrong for "o"
to accept something that "m" doesn't, and logically "m" =
"o" + "V".

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #14 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Well, V=m-o (not the same thing, these are sets) -- but, it is clear that "o"
should be a subset of "m":

(define_memory_constraint "TARGET_MEM_CONSTRAINT"
  "Matches any valid memory."

(define_memory_constraint "o"
  "Matches an offsettable memory reference."

So yeah, it should get the memory_address_addr_space_p thing.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #15 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
I've submitted the patch defining a new memory constraint:

https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566976.html

The patch itself:

https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210319/c979c698/attachment-0001.bin

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Vladimir Makarov <vmakarov@gcc.gnu.org>:

https://gcc.gnu.org/g:02f2dc441b1954736cc61e3f97687cd23d5586c5

commit r11-7768-g02f2dc441b1954736cc61e3f97687cd23d5586c5
Author: Vladimir N. Makarov <vmakarov@redhat.com>
Date:   Mon Mar 22 13:34:50 2021 -0400

    [PR99581] Define relaxed memory and use it for aarch64

    aarch64 needs to skip memory address validation for LD1R insns.  Skipping
    the address validation may result in LRA crash for some targets when usual
    memory constraint is used.  This patch introduces
define_relaxed_memory_constraint,
    skipping address validation for it, and defining relaxed memory for
    aarch64 LD1r insn memory operand.

    gcc/ChangeLog:

            PR target/99581
            * config/aarch64/constraints.md (UtQ): Use
            define_relaxed_memory_constraint for it.
            * doc/md.texi (define_relaxed_memory_constraint): Describe it.
            * genoutput.c (main): Process DEFINE_RELAXED_MEMORY_CONSTRAINT.
            * genpreds.c (constraint_data): Add bitfield is_relaxed_memory.
            (have_relaxed_memory_constraints): New static var.
            (relaxed_memory_start, relaxed_memory_end): Ditto.
            (add_constraint): Add arg is_relaxed_memory.  Check name for
            relaxed memory.  Set up is_relaxed_memory in constraint_data and
            have_relaxed_memory_constraints.  Adjust calls.
            (choose_enum_order): Process relaxed memory.
            (write_tm_preds_h): Ditto.
            (main): Process DEFINE_RELAXED_MEMORY_CONSTRAINT.
            * gensupport.c (process_rtx): Process
DEFINE_RELAXED_MEMORY_CONSTRAINT.
            * ira-costs.c (record_reg_classes): Process CT_RELAXED_MEMORY.
            * ira-lives.c (single_reg_class): Use
            insn_extra_relaxed_memory_constraint.
            * ira.c (ira_setup_alts): CT_RELAXED_MEMORY.
            * lra-constraints.c (valid_address_p): Use
            insn_extra_relaxed_memory_constraint instead of other memory
            constraints.
            (process_alt_operands): Process CT_RELAXED_MEMORY.
            (curr_insn_transform): Use insn_extra_relaxed_memory_constraint.
            * recog.c (asm_operand_ok, preprocess_constraints): Process
            CT_RELAXED_MEMORY.
            * reload.c (find_reloads): Ditto.
            * rtl.def (DEFINE_RELAXED_MEMORY_CONSTRAINT): New.
            * stmt.c (parse_input_constraint): Use
            insn_extra_relaxed_memory_constraint.

    gcc/testsuite/ChangeLog:

            PR target/99581
            * gcc.target/powerpc/pr99581.c: New.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
           Assignee|unassigned at gcc dot gnu.org      |vmakarov at gcc dot gnu.org

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug target/99581] [11 Regression] internal compiler error: during RTL pass: final - void QTWTF::TCMalloc_PageHeap::scavengerThread() since r11-7526

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581

--- Comment #18 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Vladimir Makarov <vmakarov@gcc.gnu.org>:

https://gcc.gnu.org/g:be70bb5e4babdf9d3d33e8f4658452038407fa8e

commit r11-7807-gbe70bb5e4babdf9d3d33e8f4658452038407fa8e
Author: Vladimir N. Makarov <vmakarov@redhat.com>
Date:   Tue Mar 23 17:51:21 2021 -0400

    [PR99581] Use relaxed memory for more aarch64 memory constraints

    The original patch for PR99581 resulted in GCC testsuite regression as
    some constraints were not declared as relaxed memory ones.  This patch
    fixes this.

    gcc/ChangeLog:

            PR target/99581
            * config/aarch64/constraints.md (Utq, UOb, UOh, UOw, UOd, UOty):
            Use define_relaxed_memory_constraint for them.