Hi Martin, > On 19 Oct 2026, at 16:47, Martin Jambor <[email protected]> wrote: > > Hello, > > sorry for taking so long to get to this....
No problem, thanks for the review! > > On Wed, Jun 10 2026, [email protected] wrote: >> From: Kyrylo Tkachov <[email protected]> >> >> eliminated_by_inlining_prob only credited a write to the return value as >> free-after-inlining when the RHS was a register or invariant (a scalar >> "return x;"). A function that returns an aggregate by value >> ("V t = a; ...; return t;") therefore had the copy of the result into the >> return slot charged in full to the inliner's growth estimate, even though >> return-slot optimization / SRA eliminates that copy once the call is inlined >> (the inlined callee writes the caller's destination in place). >> >> For a small value-type wrapper -- a SIMD/array lane type, a std::array-like >> POD, std::complex, a short Tensor/Matrix -- whose overloaded operators are >> written "V op(const V &a, const V &b) { V t = a; t op= b; return t; }", the >> mispriced return-slot copy pushes the wrapper over --param >> early-inlining-insns, so the early inliner leaves it out of line. Such >> operators are called pervasively, and keeping them out of line also forces >> their operands and result through memory in the caller. >> >> Credit aggregate copies whose destination is the return value (a RESULT_DECL, >> or a store through the invisible-reference return pointer) as eliminated by >> inlining, mirroring the existing treatment of reads of by-reference >> parameters. Statements with volatile operands are excluded: a volatile >> access is never elided, so such a copy is not free after inlining. >> >> This is a size-model refinement only; no IR is changed and no semantics are >> affected. >> >> This gives about 8.5% on an aarch64 machine on the 766.femflow_r >> benchmark from SPEC2026 where these routines are critical to performance >> and their vectorised form needs to be inlined for good performance. >> >> Bootstrapped and tested on aarch64-none-linux-gnu. >> >> Signed-off-by: Kyrylo Tkachov <[email protected]> >> >> gcc/ChangeLog: >> >> * ipa-fnsummary.cc (eliminated_by_inlining_prob): Credit aggregate >> copies into the return slot (a RESULT_DECL or invisible-reference >> return) as eliminated by inlining. >> >> gcc/testsuite/ChangeLog: >> >> * g++.dg/ipa/inline-aggregate-retval.C: New test. >> * gcc.dg/ipa/inline-aggregate-retval.c: New test. >> --- >> gcc/ipa-fnsummary.cc | 27 ++++++++++++++++ >> .../g++.dg/ipa/inline-aggregate-retval.C | 21 +++++++++++++ >> .../gcc.dg/ipa/inline-aggregate-retval.c | 31 +++++++++++++++++++ >> 3 files changed, 79 insertions(+) >> create mode 100644 gcc/testsuite/g++.dg/ipa/inline-aggregate-retval.C >> create mode 100644 gcc/testsuite/gcc.dg/ipa/inline-aggregate-retval.c >> >> diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc >> index a6ccdb1852a..c05ffc313ed 100644 >> --- a/gcc/ipa-fnsummary.cc >> +++ b/gcc/ipa-fnsummary.cc >> @@ -1476,6 +1476,33 @@ eliminated_by_inlining_prob (ipa_func_body_info *fbi, >> gimple *stmt) >> if (lhs_free >> && (is_gimple_reg (rhs) || is_gimple_min_invariant (rhs))) >> rhs_free = true; >> + /* A by-value aggregate return ("T t = ...; return t;") stores the >> + result into the return slot. Under the return-slot optimization the >> + inlined callee writes the caller's destination in place and SRA then > > If this is basically anticipating intra-procedural SRA and is meant for > "small value-type wrappers," should we be perhaps making sure the size > of the aggregate being copied is not bigger than > param_sra_max_scalarization_size_speed/param_sra_max_scalarization_size_size > as appropriate (see sra_get_max_scalarization_size)? I think that limit controls total scalarization. It does not bound scalarization of existing field accesses. I checked a 64-byte aggregate on AArch64 with --param sra-max-scalarization-size-Ospeed=1, using the unpatched compiler. The early SRA dump contains both: Too big to totally scalarize: t Removing load: <retval> = t; SRA replaces all eight fields. The inlined caller has no aggregate copy. I therefore kept this heuristic without that limit. The source is now restricted to an automatic local variable or parameter whose address is not taken, following Kugan's earlier comment. This is an estimate that the copy can disappear, rather than a guarantee for every caller. > >> + scalarises the small aggregate, so the store is eliminated by inlining >> + -- the return-value analogue of the by-reference parameter reads >> + credited above. The is_gimple_reg/is_gimple_min_invariant test only >> + credits scalar returns; also credit an aggregate return whose source >> is >> + an ordinary memory value. This is the value-type wrapper idiom >> + (a SIMD/array lane type, std::array, std::complex, a short Tensor): >> + "T operator OP (const T &a, const T &b) { T t = a; t OP= b; return t; >> }". >> + This is the whole-aggregate-copy case. */ >> + if (!rhs_free >> + && !gimple_has_volatile_ops (stmt) > > I guess that this test should be done way earlier in the function if we > are going to introduce it. Yeah, agreed. > >> + && AGGREGATE_TYPE_P (TREE_TYPE (lhs)) > > Conceptually, is the above strictly necessary? > It is not needed to recognize the return slot. I kept it to restrict the new full-credit rule to aggregate copies. >> + && gimple_assign_rhs_class (stmt) == GIMPLE_SINGLE_RHS > > If the answer to the above question is true, then I believe this > condition is superfluous. Agreed, removed. > >> + && (TREE_CODE (inner_lhs) == RESULT_DECL > > Both testcases test the above case... > >> + || (TREE_CODE (inner_lhs) == MEM_REF >> + && TREE_CODE (TREE_OPERAND (inner_lhs, 0)) == SSA_NAME >> + && SSA_NAME_VAR (TREE_OPERAND (inner_lhs, 0)) >> + && TREE_CODE (SSA_NAME_VAR (TREE_OPERAND (inner_lhs, 0))) >> + == RESULT_DECL)) > > ...do you have a testcase which triggers the above? I'd like to see it. > Yes. A class with a user-provided destructor uses an invisible reference for its result: struct V { long d[8]; ~V () {} }; V from_local (const V &a) { V t = a; return (t.d[0] += 1, t); } The comma expression prevents NRVO. After the empty destructor is inlined, the source local is not address-taken. The summary contains: struct V & _6(D); *_6(D) = t; return _6(D); _6(D) is the SSA name for the RESULT_DECL. I added a regression test for this form. Its total-scalarization limit is set to one byte so target defaults cannot remove the copy before fnsummary. Trunk charges the complete copy, while the patch credits it as eliminated. >> + && (TREE_CODE (inner_rhs) == MEM_REF >> + || TREE_CODE (inner_rhs) == COMPONENT_REF >> + || VAR_P (inner_rhs) >> + || TREE_CODE (inner_rhs) == PARM_DECL >> + || TREE_CODE (inner_rhs) == RESULT_DECL)) > > What are the above tests of inner_rhs trying to achieve? For example, > since inner_rhs is a result get_base_address, it can never be a > COMPONENT_REF. But what is it all guarding against? They were intended to limit the rule to copies from memory, but they were too broad. You are right that get_base_address cannot return a COMPONENT_REF. > > Nevertheless, thanks for looking into what looks like an important case > for the heuristics, Thanks again, I’m sending the respin shortly. Kyrill > > Martin > > >> + return 2; >> if (lhs_free && rhs_free) >> return 1; >> } >> diff --git a/gcc/testsuite/g++.dg/ipa/inline-aggregate-retval.C >> b/gcc/testsuite/g++.dg/ipa/inline-aggregate-retval.C >> new file mode 100644 >> index 00000000000..a60bad00b66 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/ipa/inline-aggregate-retval.C >> @@ -0,0 +1,21 @@ >> +// The value-type operator-overload idiom: a by-value 'operator*' returns >> its >> +// result aggregate, and the copy of that result into the return slot is >> +// eliminated by inlining (return-slot optimization / SRA). It used to be >> +// charged to the early-inliner growth, pushing the operator over >> +// --param early-inlining-insns. -fno-inline-functions isolates the early >> +// inliner so the operator is inlined only when the copy is credited. >> +// { dg-do compile } >> +// { dg-options "-O3 --param early-inlining-insns=14 -fno-inline-functions >> -fno-partial-inlining -fdump-tree-einline-optimized" } >> + >> +struct V { >> + double d[8]; >> + V &operator*= (const V &o) { for (int i = 0; i < 8; i++) d[i] *= o.d[i]; >> return *this; } >> +}; >> + >> +static V operator* (const V &a, const V &b) { V t = a; return t *= b; } >> + >> +V ga, gb, gc, r; >> + >> +void use () { r = ga * gb * gc; } >> + >> +// { dg-final { scan-tree-dump "Inlining.*operator.*into.*use" "einline" } } >> diff --git a/gcc/testsuite/gcc.dg/ipa/inline-aggregate-retval.c >> b/gcc/testsuite/gcc.dg/ipa/inline-aggregate-retval.c >> new file mode 100644 >> index 00000000000..e21cff3438f >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/ipa/inline-aggregate-retval.c >> @@ -0,0 +1,31 @@ >> +/* A small by-value wrapper that returns its aggregate result by value. The >> + copy of the result into the return slot is eliminated by inlining >> + (return-slot optimization / SRA combines it with the caller's >> destination), >> + but it used to be charged in full to the early-inliner growth estimate, >> + pushing the wrapper over --param early-inlining-insns and leaving it out >> of >> + line. -fno-inline-functions isolates the early inliner, so 'mul' can >> only >> + be inlined when the return-slot copy is correctly credited as >> eliminable. */ >> +/* { dg-do compile } */ >> +/* { dg-options "-O3 --param early-inlining-insns=14 -fno-inline-functions >> -fno-partial-inlining -fdump-tree-einline-optimized" } */ >> + >> +struct V { double d[8]; }; >> + >> +static struct V >> +mul (const struct V *a, const struct V *b) >> +{ >> + struct V t = *a; >> + for (int i = 0; i < 8; i++) >> + t.d[i] *= b->d[i]; >> + return t; >> +} >> + >> +struct V ga, gb, gc, r; >> + >> +void >> +use (void) >> +{ >> + struct V x = mul (&ga, &gb); >> + r = mul (&x, &gc); >> +} >> + >> +/* { dg-final { scan-tree-dump "Inlining mul.* into use" "einline" } } */ >> -- >> 2.50.1 (Apple Git-155)
