[Bug rtl-optimization/114924] New: [11/12/13/14/15 Regression] Wrong update of MEM_EXPR by RTL loop unrolling since r11-2963-gd6a05b494b4b71

[acoplan at gcc dot gnu.org via Gcc-bugs]

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

            Bug ID: 114924
           Summary: [11/12/13/14/15 Regression] Wrong update of MEM_EXPR
                    by RTL loop unrolling since r11-2963-gd6a05b494b4b71
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

The following testcase is reduced from
libgomp/testsuite/libgomp.fortran/imperfect-destructor.f90:

module m
  type t
    contains
      final fini
  end type
  integer ccount(3)
  contains
    subroutine init(x, n)
      type(t) x
      xi = n
      ccount = 1
    end
    subroutine fini(x)
      type(t) x
      dcount= s1 (a3)
  do i = 1, 1
    block
      do j = 1, 2
        block
          do k = 1, a3
            block
              type (t) local3
              call init (local3, 3)
            end block
          end do
        end block
      end do
    end block
  end do
end
end

compiling with -O2 -funroll-loops -da and looking at the RTL dumps, I see the
following insn in 284r.loop2_invariant:

(insn 44 40 45 8 (set (mem/c:SI (plus:DI (reg/f:DI 121)
                (const_int 8 [0x8])) [3 ccount[2]+0 S4 A64])
        (subreg:SI (reg:V2SI 111) 0)) "t.f90":11:16 discrim 2 69
{*movsi_aarch64}
     (expr_list:REG_DEAD (reg:V2SI 111)
        (nil)))

then in 285r.loop2_unroll, I see:

(insn 44 40 45 8 (set (mem/c:SI (plus:DI (reg/f:DI 121)
                (const_int 8 [0x8])) [3 ccount+0 S4 A64])
        (subreg:SI (reg:V2SI 111) 0)) "t.f90":11:16 discrim 2 69
{*movsi_aarch64}
     (expr_list:REG_DEAD (reg/f:DI 121)
        (expr_list:REG_DEAD (reg:V2SI 111)
            (nil))))

notably the MEM_EXPR has been changed from ccount[2] to ccount, without a
corresponding change in offset.  This is incorrect.  Setting a watchpoint on
the
MEM_ATTRS of the relevant MEM showed that the update happens in
cfgrtl.cc:duplicate_insn_chain, which does the following:

        /* We cannot adjust MR_DEPENDENCE_CLIQUE in-place
           since MEM_EXPR is shared so make a copy and
           walk to the subtree again.  */
        tree new_expr = unshare_expr (MEM_EXPR (*iter));
        if (TREE_CODE (new_expr) == WITH_SIZE_EXPR)
          new_expr = TREE_OPERAND (new_expr, 0);
        while (handled_component_p (new_expr))
          new_expr = TREE_OPERAND (new_expr, 0);
        MR_DEPENDENCE_CLIQUE (new_expr) = newc;
        set_mem_expr (const_cast <rtx> (*iter), new_expr);

so the code (correctly) looks through the ARRAY_REF in this case to find
the underlying MEM_REF and updates MR_DEPENDENCE_CLIQUE for that
MEM_REF, but then proceeds to pass the MEM_REF to set_mem_expr, thereby
incorrectly dropping the ARRAY_REF in this case.

The code above was introduced in
r11-2963-gd6a05b494b4b714e996a5ca09c5a4a1c41dbd648 so I assume this is a
regression in GCC 11 and beyond.

I have a straightforward patch to fix this which passes bootstrap on
aarch64-linux-gnu, I will post that shortly.

While I don't have a wrong-code reproducer at the moment, we may want to
consider backporting the fix as incorrect MEM_EXPR information could
lead to wrong code.  I found the issue while working on a patch series
that has the side effect of introducing some consistency checking of the
MEM_EXPR information.