Hello,
among other things, IPA-SRA checks whether splitting out a bit of an
aggregate or something passed by reference would lead into a clash
with an already known IPA-CP constant a way which would cause problems
later on. Unfortunately the test is done only in
adjust_parameter_descriptions and is missing when accesses are
propagated from callees to callers, which leads to miscompilation
reported as PR 118243 (where the callee is a function created by
ipa-split).
The matter is then further complicated by the fact that we consider
complex numbers as scalars even though they can be modified piecemeal
(IPA-CP can detect and propagate the pieces separately too) which then
confuses the parameter manipulation machinery furter.
This patch simply adds the missing check to avoid the IPA-SRA
transform in these cases too, which should be suitable for backporting
to all affected release branches. It is a bit of a shame as in the PR
testcase we do propagate both components of the complex number in
question and the transformation phase could recover. I have some
prototype patches in this direction but that is something for (a)
stage 1.
Bootstrapped and tested on x86_64-linux. OK for master and (assuming it
applies cleanly and passes the checks there too) to all active release
branches?
Thanks,
Martin
gcc/ChangeLog:
2025-02-10 Martin Jambor <mjam...@suse.cz>
PR ipa/118243
* ipa-sra.cc (pull_accesses_from_callee): New parameters
caller_ipcp_ts and param_idx. Check that scalar pulled accesses would
not clash with a known IPA-CP aggregate constant.
(param_splitting_across_edge): Pass IPA-CP transformation summary and
caller parameter index to pull_accesses_from_callee.
gcc/testsuite/ChangeLog:
2025-02-10 Martin Jambor <mjam...@suse.cz>
PR ipa/118243
* g++.dg/ipa/pr118243.C: New test.
---
gcc/ipa-sra.cc | 38 +++++++++++++++++++--------
gcc/testsuite/g++.dg/ipa/pr118243.C | 40 +++++++++++++++++++++++++++++
2 files changed, 68 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ipa/pr118243.C
diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc
index ad80d22f8ce..5d1703ed394 100644
--- a/gcc/ipa-sra.cc
+++ b/gcc/ipa-sra.cc
@@ -3640,15 +3640,19 @@ enum acc_prop_kind {ACC_PROP_DONT, ACC_PROP_COPY,
ACC_PROP_CERTAIN};
/* Attempt to propagate all definite accesses from ARG_DESC to PARAM_DESC,
(which belongs to CALLER) if they would not violate some constraint there.
- If successful, return NULL, otherwise return the string reason for failure
- (which can be written to the dump file). DELTA_OFFSET is the known offset
- of the actual argument withing the formal parameter (so of ARG_DESCS within
- PARAM_DESCS), ARG_SIZE is the size of the actual argument or zero, if not
- known. In case of success, set *CHANGE_P to true if propagation actually
- changed anything. */
+ CALLER_IPCP_TS describes the caller, PARAM_IDX is the index of the parameter
+ described by PARAM_DESC. If successful, return NULL, otherwise return the
+ string reason for failure (which can be written to the dump file).
+ DELTA_OFFSET is the known offset of the actual argument withing the formal
+ parameter (so of ARG_DESCS within PARAM_DESCS), ARG_SIZE is the size of the
+ actual argument or zero, if not known. In case of success, set *CHANGE_P to
+ true if propagation actually changed anything. */
static const char *
-pull_accesses_from_callee (cgraph_node *caller, isra_param_desc *param_desc,
+pull_accesses_from_callee (cgraph_node *caller,
+ ipcp_transformation *caller_ipcp_ts,
+ int param_idx,
+ isra_param_desc *param_desc,
isra_param_desc *arg_desc,
unsigned delta_offset, unsigned arg_size,
bool *change_p)
@@ -3673,6 +3677,17 @@ pull_accesses_from_callee (cgraph_node *caller,
isra_param_desc *param_desc,
continue;
unsigned offset = argacc->unit_offset + delta_offset;
+
+ if (caller_ipcp_ts && !AGGREGATE_TYPE_P (argacc->type))
+ {
+ ipa_argagg_value_list avl (caller_ipcp_ts);
+ tree value = avl.get_value (param_idx, offset);
+ if (value && ((tree_to_uhwi (TYPE_SIZE (TREE_TYPE (value)))
+ / BITS_PER_UNIT)
+ != argacc->unit_size))
+ return " propagated access would conflict with an IPA-CP constant";
+ }
+
/* Given that accesses are initially stored according to increasing
offset and decreasing size in case of equal offsets, the following
searches could be written more efficiently if we kept the ordering
@@ -3781,6 +3796,8 @@ param_splitting_across_edge (cgraph_edge *cs)
cgraph_node *callee = cs->callee->function_symbol (&availability);
isra_func_summary *from_ifs = func_sums->get (cs->caller);
gcc_checking_assert (from_ifs && from_ifs->m_parameters);
+ ipcp_transformation *caller_ipcp_ts
+ = ipcp_get_transformation_summary (cs->caller);
isra_call_summary *csum = call_sums->get (cs);
gcc_checking_assert (csum);
@@ -3851,8 +3868,8 @@ param_splitting_across_edge (cgraph_edge *cs)
else
{
const char *pull_failure
- = pull_accesses_from_callee (cs->caller, param_desc, arg_desc,
- 0, 0, &res);
+ = pull_accesses_from_callee (cs->caller, caller_ipcp_ts, idx,
+ param_desc, arg_desc, 0, 0, &res);
if (pull_failure)
{
if (dump_file && (dump_flags & TDF_DETAILS))
@@ -3913,7 +3930,8 @@ param_splitting_across_edge (cgraph_edge *cs)
else
{
const char *pull_failure
- = pull_accesses_from_callee (cs->caller, param_desc, arg_desc,
+ = pull_accesses_from_callee (cs->caller, caller_ipcp_ts, idx,
+ param_desc, arg_desc,
ipf->unit_offset,
ipf->unit_size, &res);
if (pull_failure)
diff --git a/gcc/testsuite/g++.dg/ipa/pr118243.C
b/gcc/testsuite/g++.dg/ipa/pr118243.C
new file mode 100644
index 00000000000..5efaa276da1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr118243.C
@@ -0,0 +1,40 @@
+/* { dg-do run } */
+/* { dg-options "-O3 -std=gnu++11" } */
+
+using complex_t = int __complex__;
+
+struct A {
+ complex_t value;
+ A(double r) : value{0, r} {}
+};
+
+[[gnu::noipa]]
+void f(int a)
+{
+ if (a != 1)
+ __builtin_abort();
+}
+[[gnu::noipa]] void g(const char *, int x){}
+
+
+void test(const complex_t &c, const int &x) {
+ if (x < 0)
+ g("%d\n", x);
+ else
+ {
+ f( __imag__ c);
+ }
+}
+
+void f1() {
+ {
+ A a{1};
+ test(a.value, 123);
+ }
+}
+
+int main()
+{
+ f1();
+ return 0;
+}
--
2.47.1
