[Bug tree-optimization/107275] New: [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[Bug tree-optimization/107275] New: [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[law at gcc dot gnu.org]

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

            Bug ID: 107275
           Summary: [13 Regression] Recent ifcvt changes resulting in
                    references to SSA_NAME on free list
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: law at gcc dot gnu.org
  Target Milestone: ---

This change:
commit 25413fdb2ac24933214123e24ba165026452a6f2 (HEAD, refs/bisect/bad)
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Tue Oct 11 10:49:27 2022 +0100

    vect: Teach vectorizer how to handle bitfield accesses

Is resulting in a PHI node with a dangling SSA_NAME.  This in turn causes the
compiler to abort when compiling the linux kernel on mips64-linux-gnu.

If I put a breakpoint in exit, then go up the call chain enough we'll see that
we're calling verify_gimple_phi for this PHI:

#8  0x00000000013a4bd4 in verify_gimple_phi (phi=0x7ffff77e5f00) at
/home/jlaw/test/gcc/gcc/tree-cfg.cc:5201
5201          if (!useless_type_conversion_p (TREE_TYPE (phi_result), TREE_TYPE
(t)))
(gdb) p debug_gimple_stmt (phi)
.MEM_12 = PHI <_7(6), .MEM_4(D)(10)>


Note the _7.  And if we look at the actual underlying node:

gdb) p debug_tree (t)
 <ssa_name 0x7ffff76c5c18 type <error_mark 0x7ffff76c96c0>
    nothrow
    def_stmt 
    version:7 in-free-list>
              ^^^^^^^^^^^^


If I put a breakpoint in ifcvt and look at the key block (#3) it looks like
this:




;;   basic block 3, loop depth 1
;;    pred:       6
;;                5
  # link_10 = PHI <link_8(6), link_3(D)(5)>
  # .MEM_12 = PHI <.MEM_7(6), .MEM_4(D)(5)>
  # .MEM_6 = VDEF <.MEM_12>
  link_10->direct_complete = 0;
  # .MEM_7 = VDEF <.MEM_6>
  __asm__ __volatile__("" :  :  : "memory");
  # VUSE <.MEM_7>
  link_8 = link_10->c_node.next;
  if (dev_5(D) != link_8)
    goto <bb 6>; [89.00%]
  else
    goto <bb 4>; [11.00%]
;;    succ:       6
;;                4

;
Quite sensible.  In particular note the reference to MEM_7 in the PHI.  But
when ifcvt is done:

;;   basic block 3, loop depth 1
;;    pred:       6
;;                10
  # link_10 = PHI <link_8(6), link_3(D)(10)>
  # .MEM_12 = PHI <_7(6), .MEM_4(D)(10)>
  # VUSE <.MEM_12>
  _ifc__16 = link_10->D.1530;
  _ifc__17 = BIT_INSERT_EXPR <_ifc__16, 0, 0 (1 bits)>;
  # VUSE <_7>
  link_8 = link_10->c_node.next;
  if (dev_5(D) != link_8)
    goto <bb 6>; [89.00%]
  else
    goto <bb 4>; [11.00%]
;;    succ:       6
;;                4

Now it's in the free list, but there's still two references in bb3.   Se seemed
to have removed the volatile asm which defined _7.



I haven't dug any further than that.


Testcase:

struct list_head
{
  struct list_head *next;
};
struct device
{
  struct list_head suppliers;
};
struct device_link
{
  struct list_head c_node;
  int direct_complete:1;
};
void
dpm_clear_superiors_direct_complete (struct device *dev)
{
  struct device_link *link;
  for (; &link->c_node != (&dev->suppliers);)
    {
      link->direct_complete = 0;
      __asm__ __volatile__ ("":::"memory");
      link = link->c_node.next;
    }
}


Compile with -O2 -mabi=64 -I./  on mips64-linux-gnu.

[Bug tree-optimization/107275] [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[avieira at gcc dot gnu.org]

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

avieira at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |avieira at gcc dot gnu.org

--- Comment #1 from avieira at gcc dot gnu.org ---
I'll have a look, thank you for the reduced testcase!

[Bug tree-optimization/107275] [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0

[Bug tree-optimization/107275] [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[avieira at gcc dot gnu.org]

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

avieira at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-10-17
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from avieira at gcc dot gnu.org ---
ifcvt's dce seems to be removing the asm, which is rather odd...

Moving the 'struct device_link *link;' outside of the function, making it a
global seems to give a different ICE too, related to vdefs. So I suspect my
vdef/vuse update is confusing things. I've never quite understood what and how
the vdef/vuse update is supposed to happen, update_stmt used to be my goto
fix-all, but that doesnt' seem to be helping. As a side-not, I also noticed
that doing the gimple_move_vops after inserting seems to yield different
results as well...

Just to say I am nowhere yet, if anyone has an idea of what might be going
wrong I welcome the suggestion, in the meantime I'll continue prodding this.

[Bug tree-optimization/107275] [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[avieira at gcc dot gnu.org]

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

--- Comment #3 from avieira at gcc dot gnu.org ---
The prodding helped! The problem is that dce was indeed removing the ASM as it
wasn't recognizing it as a stmt that was live. This is because ifcvt would have
normally bailed out when encountering such an asm stmt when doing
'find_data_references_in_loop'.

I have a patch that fixes this, will test it and post it upstream. My plan is
to bring forward the references check, as we do not need to lower bitfields if
that fails, given loop-vectorization will fail altogether anyway.

[Bug tree-optimization/107275] [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug tree-optimization/107275] [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andre Simoes Dias Vieira
<avieira@gcc.gnu.org>:

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

commit r13-3355-gaae016f99b121b55fc1bcdfc2403fd22f04fa2df
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Tue Oct 18 10:51:09 2022 +0100

    ifcvt: Do not lower bitfields if we can't analyze dr's [PR107275]

    The ifcvt dead code elimination code was not built to deal with inline
    assembly, loops with such would never be if-converted in the past since we
can't
    do data-reference analysis on them and vectorization would eventually fail.
 For
    this reason we now also do not lower bitfields if the data-reference
analysis
    fails, as we would not end up vectorizing it.  As a consequence this also
fixes
    this PR as the dead code elimination will not run for such cases and
wrongfully
    eliminate inline assembly statements.

    gcc/ChangeLog:

            PR tree-optimization/107275
            * tree-if-conv.cc (if_convertible_loop_p_1): Move
            find_data_references_in_loop call from here...
            (if_convertible_loop_p): And move data-reference vector
initialization
            from here...
            (tree_if_conversion):... to here.

    gcc/testsuite/ChangeLog:

            * gcc.dg/vect/pr107275.c: New test.

[Bug tree-optimization/107275] [13 Regression] Recent ifcvt changes resulting in references to SSA_NAME on free list

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #5 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Fixed by Andre's patch on the trunk.