On 3 November 2017 at 15:38, Richard Biener <richard.guent...@gmail.com> wrote:
> On Fri, Nov 3, 2017 at 6:15 AM, Prathamesh Kulkarni
> <prathamesh.kulka...@linaro.org> wrote:
>> Hi Martin,
>> As mentioned in PR, the issue here for propagating value of 'm' from
>> f_c1 to foo() is that the jump function operation is FLOAT_EXPR, and
>> the type of input param 'm' is int, so fold_unary() doesn't do the
>> conversion to real_type. The attached patch fixes that by calling
>> fold_convert if operation is FLOAT_EXPR / FIX_TRUNC_EXPR /
>> CONVERT_EXPR and converts it to the type of corresponding parameter in
>> callee.
>>
>> There are still two issues:
>> a) Using NOP_EXPR for early_exit in ipa_get_jf_pass_through_result.
>> I suppose we need to change to some other code to indicate that there
>> is no operation ?
>> b) Patch does not passing param_type from all callers.
>> I suppose we could fix these incrementally ?
>>
>> Bootstrap+tested on x86_64-unknown-linux-gnu.
>> OK for trunk ?
>
> This doesn't look like a well designed fix. Both fold_unary and fold_binary
> calls get a possibly bogus type and you single out only a few ops.
>
> Either _fully_ list a set of operations that are know to have matching
> input/output types or always require param_type to be non-NULL.
>
> For a) simply remove the special-casing and merge it with CONVERT_EXPR
> handling (however it will end up looking).
>
> Please don't use fold_convert, using fold_unary is fine.
Hi Richard,
Sorry for the delay. In the attached version, parm_type is made non
NULL in ipa_get_jf_pass_through_result().
ipa_get_jf_pass_through_result() is called from two
places - propagate_vals_across_pass_through() and ipa_value_from_jfunc().
However it appears ipa_value_from_jfunc() is called from multiple
functions and it's
hard to detect parm_type in the individual callers. So I have passed
correct parm_type from propagate_vals_across_pass_through(), and kept
the old behavior for ipa_value_from_jfunc().
Would it be OK to fix that incrementally ?
Patch is bootstrapped+tested on x86_64-unknown-linux-gnu.
Thanks,
Prathamesh
>
> Thanks,
> Richard.
>
>> Thanks,
>> Prathamesh
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index bc1e3ae799d..c7b67e6d007 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1224,7 +1224,8 @@ initialize_node_lattices (struct cgraph_node *node)
determined or be considered an interprocedural invariant. */
static tree
-ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input)
+ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input,
+ tree parm_type)
{
tree restype, res;
@@ -1233,10 +1234,12 @@ ipa_get_jf_pass_through_result (struct ipa_jump_func
*jfunc, tree input)
if (!is_gimple_ip_invariant (input))
return NULL_TREE;
+ gcc_assert (parm_type != NULL_TREE);
+
if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
== tcc_unary)
res = fold_unary (ipa_get_jf_pass_through_operation (jfunc),
- TREE_TYPE (input), input);
+ parm_type, input);
else
{
if (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
@@ -1312,7 +1315,7 @@ ipa_value_from_jfunc (struct ipa_node_params *info,
struct ipa_jump_func *jfunc)
return NULL_TREE;
if (jfunc->type == IPA_JF_PASS_THROUGH)
- return ipa_get_jf_pass_through_result (jfunc, input);
+ return ipa_get_jf_pass_through_result (jfunc, input, TREE_TYPE (input));
else
return ipa_get_jf_ancestor_result (jfunc, input);
}
@@ -1567,7 +1570,8 @@ ipcp_lattice<valtype>::add_value (valtype newval,
cgraph_edge *cs,
static bool
propagate_vals_across_pass_through (cgraph_edge *cs, ipa_jump_func *jfunc,
ipcp_lattice<tree> *src_lat,
- ipcp_lattice<tree> *dest_lat, int src_idx)
+ ipcp_lattice<tree> *dest_lat, int src_idx,
+ tree parm_type)
{
ipcp_value<tree> *src_val;
bool ret = false;
@@ -1581,7 +1585,8 @@ propagate_vals_across_pass_through (cgraph_edge *cs,
ipa_jump_func *jfunc,
else
for (src_val = src_lat->values; src_val; src_val = src_val->next)
{
- tree cstval = ipa_get_jf_pass_through_result (jfunc, src_val->value);
+ tree cstval = ipa_get_jf_pass_through_result (jfunc, src_val->value,
+ parm_type);
if (cstval)
ret |= dest_lat->add_value (cstval, cs, src_val, src_idx);
@@ -1627,7 +1632,8 @@ propagate_vals_across_ancestor (struct cgraph_edge *cs,
static bool
propagate_scalar_across_jump_function (struct cgraph_edge *cs,
struct ipa_jump_func *jfunc,
- ipcp_lattice<tree> *dest_lat)
+ ipcp_lattice<tree> *dest_lat,
+ tree param_type)
{
if (dest_lat->bottom)
return false;
@@ -1662,7 +1668,7 @@ propagate_scalar_across_jump_function (struct cgraph_edge
*cs,
if (jfunc->type == IPA_JF_PASS_THROUGH)
ret = propagate_vals_across_pass_through (cs, jfunc, src_lat,
- dest_lat, src_idx);
+ dest_lat, src_idx,
param_type);
else
ret = propagate_vals_across_ancestor (cs, jfunc, src_lat, dest_lat,
src_idx);
@@ -2279,7 +2285,7 @@ propagate_constants_across_call (struct cgraph_edge *cs)
else
{
ret |= propagate_scalar_across_jump_function (cs, jump_func,
- &dest_plats->itself);
+ &dest_plats->itself,
param_type);
ret |= propagate_context_across_jump_function (cs, jump_func, i,
&dest_plats->ctxlat);
ret
diff --git a/gcc/testsuite/gcc.dg/ipa/pr82808.c
b/gcc/testsuite/gcc.dg/ipa/pr82808.c
new file mode 100644
index 00000000000..e9a90e3ce2e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr82808.c
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -flto -fno-inline" } */
+
+void foo(double *a, double x)
+{
+ *a = x;
+}
+
+double f_c1(int m, double *a)
+{
+ foo(a, m);
+ return *a;
+}
+
+int main(){
+ double data;
+ double ret = 0 ;
+
+ if ((ret = f_c1(2, &data)) != 2)
+ {
+ __builtin_abort ();
+ }
+ return 0;
+}
