From be2f7a1871ae7a256f34393eeba583ff575cb7e8 Mon Sep 17 00:00:00 2001 From: Eric Botcazou <ebotcazou@adacore.com> Date: Tue, 1 Oct 2024 17:54:00 +0200 Subject: [PATCH] Fix wrong code out of NRV + RSO + inlining 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 | 9 +++++++++ gcc/testsuite/gnat.dg/lto28_pkg1.ads | 5 +++++ gcc/testsuite/gnat.dg/lto28_pkg2.adb | 10 ++++++++++ gcc/testsuite/gnat.dg/lto28_pkg2.ads | 11 +++++++++++ gcc/testsuite/gnat.dg/lto28_pkg3.ads | 19 +++++++++++++++++++ gcc/tree-inline.cc | 16 +++++++++++++++- 6 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gnat.dg/lto28.adb create mode 100644 gcc/testsuite/gnat.dg/lto28_pkg1.ads create mode 100644 gcc/testsuite/gnat.dg/lto28_pkg2.adb create mode 100644 gcc/testsuite/gnat.dg/lto28_pkg2.ads create mode 100644 gcc/testsuite/gnat.dg/lto28_pkg3.ads diff --git a/gcc/testsuite/gnat.dg/lto28.adb b/gcc/testsuite/gnat.dg/lto28.adb new file mode 100644 index 000000000000..f5f1804af866 --- /dev/null +++ b/gcc/testsuite/gnat.dg/lto28.adb @@ -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 index 000000000000..8205cdae7315 --- /dev/null +++ b/gcc/testsuite/gnat.dg/lto28_pkg1.ads @@ -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 index 000000000000..f592d119c40f --- /dev/null +++ b/gcc/testsuite/gnat.dg/lto28_pkg2.adb @@ -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 index 000000000000..ba2d0ae5781c --- /dev/null +++ b/gcc/testsuite/gnat.dg/lto28_pkg2.ads @@ -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 index 000000000000..026ffb9f97c4 --- /dev/null +++ b/gcc/testsuite/gnat.dg/lto28_pkg3.ads @@ -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; diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc index f31a34ac4105..037fd1e946ae 100644 --- a/gcc/tree-inline.cc +++ b/gcc/tree-inline.cc @@ -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; } } -- GitLab