Hi,
On Tue, Nov 05 2024, Jan Hubicka wrote:
>> gcc/ChangeLog:
>>
>> 2024-11-01 Martin Jambor <mjam...@suse.cz>
>>
>> * ipa-prop.cc (skip_a_conversion_op): New function.
>> (ipa_compute_jump_functions_for_edge): Use it.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2024-11-01 Martin Jambor <mjam...@suse.cz>
>>
>> * gcc.dg/ipa/vrp9.c: New test.
>> ---
>> gcc/ipa-prop.cc | 20 ++++++++++++++
>> gcc/testsuite/gcc.dg/ipa/vrp9.c | 48 +++++++++++++++++++++++++++++++++
>> 2 files changed, 68 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.dg/ipa/vrp9.c
>>
>> diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
>> index db8eda8c361..9bd2e4bc60c 100644
>> --- a/gcc/ipa-prop.cc
>> +++ b/gcc/ipa-prop.cc
>> @@ -2305,6 +2305,25 @@ ipa_set_jfunc_vr (ipa_jump_func *jf, const ipa_vr &vr)
>> ipa_set_jfunc_vr (jf, tmp);
>> }
>>
>> +
>> +/* If T is an SSA_NAME that is a result of a simple type conversion
>> statement,
>> + return the operand of that conversion, otherwise treturn T. */
>> +
>> +static tree
>> +skip_a_conversion_op (tree t)
>> +{
>> + if (TREE_CODE (t) != SSA_NAME
>> + || SSA_NAME_IS_DEFAULT_DEF (t))
>> + return t;
>> +
>> + gimple *def = SSA_NAME_DEF_STMT (t);
>> + if (!is_gimple_assign (def)
>> + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def)))
>> + return t;
>
> I was wonderinf if we do not want to skip chains of conversions, but I
> guess in the testcase:
> void test(double);
> void
> test2(long double a)
> {
> test ((float)a);
> }
>
> We can skip float->double but not long double->float.
The patch only ever skips just one conversion, never a chain of them and
deliberately so, for the reasons shown in the example.
However, I have just realized that combining pass_through jump functions
during inlining may unfortunately have exactly the same effect, so we
indeed need to be more careful.
The motivating example converts a bool (integer with precision one) to
an int, so what about the patch below which allows converting between
integers and to a higher precision? (Assuming the patch will pass
bootstrap and testing on x86_64-linux which is underway).
> The types stored
> to summaries are known to match the types of parameters passed to the
> funtion?
Not exactly, they correspond the types of the formal parameters. But
hopefully callers and callees agree on those.
> I am kind of worried about hose implicit C conversions, like
> float->double...
Right, the more restrictive variant should hopefully resolve that.
Thanks a lot for looking at this.
Martin
---------- 8< -------------------- 8< -------------------- 8< ----------
Originally, we did not stream any formal parameter types into WPA and
were generally very conservative when it came to type mismatches in
IPA-CP. Over the time, mismatches that happen in code and blew up in
WPA made us to be much more resilient and also to stream the types of
the parameters which we now use commonly.
With that information, we can safely skip conversions when looking at
the IL from which we build jump functions and then simply fold convert
the constants and ranges to the resulting type, as long as we are
careful that performing the corresponding folding of constants gives
the corresponding results. In order to do that, we must ensure that
the old value can be represented in the new one without any loss. The
motivating example in the testcase converts bool to an int and so this
patch allows this particular case, i.e. conversions to an integer type
with more precision.
With this change, we can nicely propagate non-NULLness in IPA-VR as
demonstrated with the new test case.
I have gone through all other uses of (all components of) jump
functions which could be affected by this and verified they do indeed
check types and can handle mismatches.
gcc/ChangeLog:
2024-11-14 Martin Jambor <mjam...@suse.cz>
* ipa-prop.cc (skip_a_widening_conversion_op): New function.
(ipa_compute_jump_functions_for_edge): Use it.
gcc/testsuite/ChangeLog:
2024-11-01 Martin Jambor <mjam...@suse.cz>
* gcc.dg/ipa/vrp9.c: New test.
---
gcc/ipa-prop.cc | 25 +++++++++++++++++
gcc/testsuite/gcc.dg/ipa/vrp9.c | 48 +++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/ipa/vrp9.c
diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index 2a0d4503f52..db9aca70eb1 100644
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -2312,6 +2312,30 @@ ipa_set_jfunc_vr (ipa_jump_func *jf, const ipa_vr &vr)
ipa_set_jfunc_vr (jf, tmp);
}
+
+/* If T is an SSA_NAME that is the result of a simple type conversion statement
+ from an integer type to another integer type with the same or bigger
+ precision, return the operand of that conversion, otherwise return T. */
+
+static tree
+skip_a_widening_conversion_op (tree t)
+{
+ if (TREE_CODE (t) != SSA_NAME
+ || SSA_NAME_IS_DEFAULT_DEF (t))
+ return t;
+
+ gimple *def = SSA_NAME_DEF_STMT (t);
+ if (is_gimple_assign (def)
+ && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def))
+ && INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (def)))
+ && INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (def)))
+ && (TYPE_PRECISION (TREE_TYPE (gimple_assign_lhs (def)))
+ >= TYPE_PRECISION (TREE_TYPE (gimple_assign_rhs1 (def)))))
+ return gimple_assign_rhs1 (def);
+
+ return t;
+}
+
/* Compute jump function for all arguments of callsite CS and insert the
information in the jump_functions array in the ipa_edge_args corresponding
to this callsite. */
@@ -2416,6 +2440,7 @@ ipa_compute_jump_functions_for_edge (struct
ipa_func_body_info *fbi,
gcc_assert (!jfunc->m_vr);
}
+ arg = skip_a_widening_conversion_op (arg);
if (is_gimple_ip_invariant (arg)
|| (VAR_P (arg)
&& is_global_var (arg)
diff --git a/gcc/testsuite/gcc.dg/ipa/vrp9.c b/gcc/testsuite/gcc.dg/ipa/vrp9.c
new file mode 100644
index 00000000000..461a2e757d2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/vrp9.c
@@ -0,0 +1,48 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+int some_f1 (int);
+int some_f2 (int);
+int some_f3 (int);
+
+void remove_this_call ();
+
+int g;
+
+static int __attribute__((noinline))
+bar (int p)
+{
+ if (p)
+ remove_this_call ();
+ return g++;
+}
+
+static int __attribute__((noinline))
+foo (int (*f)(int))
+{
+ return bar (f == (void *)0);
+}
+
+int
+baz1 (void)
+{
+ int (*f)(int);
+ if (g)
+ f = some_f1;
+ else
+ f = some_f2;
+ return foo (f);
+}
+
+int
+baz2 (void)
+{
+ int (*f)(int);
+ if (g)
+ f = some_f2;
+ else
+ f = some_f3;
+ return foo (f);
+}
+
+/* { dg-final { scan-tree-dump-not "remove_this_call" "optimized" } } */
--
2.47.0
