]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
Fix wrong code out of NRV + RSO + inlining
authorEric Botcazou <ebotcazou@adacore.com>
Tue, 1 Oct 2024 15:54:00 +0000 (17:54 +0200)
committerEric Botcazou <ebotcazou@adacore.com>
Tue, 1 Oct 2024 16:03:38 +0000 (18:03 +0200)
The testcase is miscompiled with -O -flto beccause the three optimizations
NRV + RSO + inlining are applied to the same call: when the LHS of the call
is marked write-only before inlining, it will keep the mark after inlining
although it may be read in GIMPLE from that point on.

The fix is to apply the removal of the store, that would have been applied
later if the call was not inlined, right before inlining, which will prevent
the problematic references to the LHS from being generated during inlining.

gcc/
* tree-inline.cc (expand_call_inline): Remove the store to the
return slot if it is a global variable that is only written to.

gcc/testsuite/
* gnat.dg/lto28.adb: New test.
* gnat.dg/lto28_pkg1.ads: New helper.
* gnat.dg/lto28_pkg2.ads: Likewise.
* gnat.dg/lto28_pkg2.adb: Likewise.
* gnat.dg/lto28_pkg3.ads: Likewise.

gcc/testsuite/gnat.dg/lto28.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/lto28_pkg1.ads [new file with mode: 0644]
gcc/testsuite/gnat.dg/lto28_pkg2.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/lto28_pkg2.ads [new file with mode: 0644]
gcc/testsuite/gnat.dg/lto28_pkg3.ads [new file with mode: 0644]
gcc/tree-inline.cc

diff --git a/gcc/testsuite/gnat.dg/lto28.adb b/gcc/testsuite/gnat.dg/lto28.adb
new file mode 100644 (file)
index 0000000..f5f1804
--- /dev/null
@@ -0,0 +1,9 @@
+-- { dg-do run }
+-- { dg-options "-O -flto" { target lto } }
+
+with Lto28_Pkg1;
+
+procedure Lto28 is
+begin
+   null;
+end;
diff --git a/gcc/testsuite/gnat.dg/lto28_pkg1.ads b/gcc/testsuite/gnat.dg/lto28_pkg1.ads
new file mode 100644 (file)
index 0000000..8205cda
--- /dev/null
@@ -0,0 +1,5 @@
+with Lto28_Pkg2;
+
+package Lto28_Pkg1 is
+   package I is new Lto28_Pkg2.G;
+end Lto28_Pkg1;
diff --git a/gcc/testsuite/gnat.dg/lto28_pkg2.adb b/gcc/testsuite/gnat.dg/lto28_pkg2.adb
new file mode 100644 (file)
index 0000000..f592d11
--- /dev/null
@@ -0,0 +1,10 @@
+package body Lto28_Pkg2 is
+
+   function F return Lto28_Pkg3.Q_Rec is
+   begin
+      return Result : Lto28_Pkg3.Q_Rec := Lto28_Pkg3.Default_Q_Rec do
+         Result.A := 1.0;
+      end return;
+   end;
+
+end Lto28_Pkg2;
diff --git a/gcc/testsuite/gnat.dg/lto28_pkg2.ads b/gcc/testsuite/gnat.dg/lto28_pkg2.ads
new file mode 100644 (file)
index 0000000..ba2d0ae
--- /dev/null
@@ -0,0 +1,11 @@
+with Lto28_Pkg3;
+
+package Lto28_Pkg2 is
+
+   function F return Lto28_Pkg3.Q_Rec;
+
+   generic
+      Q_Conf : Lto28_Pkg3.Q_Rec := F;
+   package G is end;
+
+end Lto28_Pkg2;
diff --git a/gcc/testsuite/gnat.dg/lto28_pkg3.ads b/gcc/testsuite/gnat.dg/lto28_pkg3.ads
new file mode 100644 (file)
index 0000000..026ffb9
--- /dev/null
@@ -0,0 +1,19 @@
+package Lto28_Pkg3 is
+
+   type Discr_Type is (P, Q);
+
+   type Rec (Discr : Discr_Type) is record
+      case Discr is
+         when Q =>
+            A : Duration := 0.0;
+            B : Duration := 0.0;
+         when P =>
+            null;
+      end case;
+   end record;
+
+   subtype Q_Rec is Rec (Q);
+
+   Default_Q_Rec : constant Q_Rec := (Discr => Q, others => <>);
+
+end Lto28_Pkg3;
index f31a34ac41055db3135211c4cf53bcca1c6763aa..037fd1e946ae1e2bc6c99760a98daa6d69412d1c 100644 (file)
@@ -5130,9 +5130,23 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
       if (DECL_P (modify_dest))
        suppress_warning (modify_dest, OPT_Wuninitialized);
 
+      /* If we have a return slot, we can assign it the result directly,
+        except in the case where it is a global variable that is only
+        written to because, the callee being permitted to read or take
+        the address of its DECL_RESULT, this could invalidate the flag
+        on the global variable; instead we preventively remove the store,
+        which would have happened later if the call was not inlined.  */
       if (gimple_call_return_slot_opt_p (call_stmt))
        {
-         return_slot = modify_dest;
+         tree base = get_base_address (modify_dest);
+
+         if (VAR_P (base)
+             && (TREE_STATIC (base) || DECL_EXTERNAL (base))
+             && varpool_node::get (base)->writeonly)
+           return_slot = NULL;
+         else
+           return_slot = modify_dest;
+
          modify_dest = NULL;
        }
     }