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; } }