| Message ID | 5613124.rdbgypaU67@fomalhaut |
|---|---|
| State | New |
| Headers | show |
| Series | Fix wrong code out of NRV + RSO + inlining (take 2) | expand |
On Tue, Oct 1, 2024 at 12:01 PM Eric Botcazou <botcazou@adacore.com> wrote: > > Hi, > > the attached Ada testcase compiled with -O -flto exhibits a wrong code issue > when the 3 optimizations NRV + RSO + inlining are applied to the same call: if > the LHS of the call is marked write-only before inlining, then it will keep > the mark after inlining although it may be read in GIMPLE from that point on. > > The proposed fix is to apply the removal of the store that would have been > applied by execute_fixup_cfg if the call was not inlined: > > /* For calls we can simply remove LHS when it is known > to be write-only. */ > if (is_gimple_call (stmt) > && gimple_get_lhs (stmt)) > { > tree lhs = get_base_address (gimple_get_lhs (stmt)); > > if (VAR_P (lhs) > && (TREE_STATIC (lhs) || DECL_EXTERNAL (lhs)) > && varpool_node::get (lhs)->writeonly) > { > gimple_call_set_lhs (stmt, NULL); > update_stmt (stmt); > todo |= TODO_update_ssa | TODO_cleanup_cfg; > } > } > > right before inlining, which will prevent the problematic references to the > LHS from being generated during inlining. > > Tested on x86-64/Linux, OK for the mainline? Nice solution - OK. Thanks, Richard. > > 2024-10-01 Eric Botcazou <ebotcazou@adacore.com> > > * tree-inline.cc (expand_call_inline): Remove the store to the > return slot if it is a global variable that is only written to. > > > 2024-10-01 Eric Botcazou <ebotcazou@adacore.com> > > * 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. > > -- > Eric Botcazou
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc index f31a34ac410..8d43b033319 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 would 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; } }
Hi, the attached Ada testcase compiled with -O -flto exhibits a wrong code issue when the 3 optimizations NRV + RSO + inlining are applied to the same call: if the LHS of the call is marked write-only before inlining, then it will keep the mark after inlining although it may be read in GIMPLE from that point on. The proposed fix is to apply the removal of the store that would have been applied by execute_fixup_cfg if the call was not inlined: /* For calls we can simply remove LHS when it is known to be write-only. */ if (is_gimple_call (stmt) && gimple_get_lhs (stmt)) { tree lhs = get_base_address (gimple_get_lhs (stmt)); if (VAR_P (lhs) && (TREE_STATIC (lhs) || DECL_EXTERNAL (lhs)) && varpool_node::get (lhs)->writeonly) { gimple_call_set_lhs (stmt, NULL); update_stmt (stmt); todo |= TODO_update_ssa | TODO_cleanup_cfg; } } right before inlining, which will prevent the problematic references to the LHS from being generated during inlining. Tested on x86-64/Linux, OK for the mainline? 2024-10-01 Eric Botcazou <ebotcazou@adacore.com> * tree-inline.cc (expand_call_inline): Remove the store to the return slot if it is a global variable that is only written to. 2024-10-01 Eric Botcazou <ebotcazou@adacore.com> * 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.