[PR 70929] IPA call type compatibility fix/workaround
Martin Jambor
mjambor@suse.cz
Thu Oct 10 10:28:00 GMT 2019
Hi,
On Wed, Oct 09 2019, Richard Biener wrote:
>>
...
>> + /* If we only have the fntype extracted from the call statement, check it
>> + against the type of declarations while being pessimistic about
>> + promotions. */
>> + tree p;
>> +
>> + if (fndecl)
>> + p = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
>> + else
>> + p = TYPE_ARG_TYPES (gimple_call_fntype (stmt));
>
> This else case is bougs - you are then comparing the call arguments
> against the call arguments... Oh, I see it was there before :/
Right, and one hand I noticed id did not make much sense, on the other
there were few cases where it was necessary to make the new predicate as
permissive as the old one (not that any of those that I saw looked
interesting).
>
> So it is that the FEs are expected to promote function arguments
> according to the originally called function and that "ABI" is
> recorded in gimple_call_fntype. That means that we can either
> look at the actual arguments or at TYPE_ARG_TYPES of
> gimple_call_fntype. But the fndecl ABI we want to verify
> against is either its DECL_ARGUMENTS or TYPE_ARG_TYPEs of its type.
>
> Verifying gimple_call_arg () against gimple_call_fntype ()
> is pointless. What should have been used here is
>
> else
> p = TYPE_ARG_TYPES (TREE_TYPE (gimple_call_fn (stmt)));
>
> so, gimple_call_fn is the function called (if no fndecl then
> this is a function pointer), thus look at the pointed-to type
> and then its arguments.
OK, this is a very nice idea, I have made the change in the patch.
>
> Maybe you can test/fix that as independent commit.
>
> Your second case
>
>> + if (fndecl
>> + && TYPE_ARG_TYPES (TREE_TYPE (fndecl))
>> + && TYPE_ARG_TYPES (gimple_call_fntype (stmt)))
>
> then collapses with this and is also the better fallback IMHO
> (so enter this case by using TYPE_ARG_TYPES (TREE_TYPE (gimple_call_fn
> (...))) instead of the fndecl).
>
The fndecl here is not the decl extracted from the gimple statement. It
is received as a function parameter and two callers extract it from a
call graph edge callee and one - speculation resolution - even from the
ipa reference associated with the speculation. So I don't think th
should be replaced.
So, is the following OK (bootstrapped and tested on x86_64-linux, no
LTO bootstrap this time because of PR 92037)?
Martin
2019-10-09 Martin Jambor <mjambor@suse.cz>
PR lto/70929
* cgraph.c (gimple_check_call_args): Also compare types of argumen
types and call statement fntype types.
testsuite/
* g++.dg/lto/pr70929_[01].C: New test.
---
gcc/cgraph.c | 83 ++++++++++++++++++++++------
gcc/testsuite/g++.dg/lto/pr70929_0.C | 18 ++++++
gcc/testsuite/g++.dg/lto/pr70929_1.C | 10 ++++
3 files changed, 95 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/lto/pr70929_0.C
create mode 100644 gcc/testsuite/g++.dg/lto/pr70929_1.C
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 0c3c6e7cac4..4f7bfa28f37 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3636,26 +3636,19 @@ cgraph_node::get_fun () const
static bool
gimple_check_call_args (gimple *stmt, tree fndecl, bool args_count_match)
{
- tree parms, p;
- unsigned int i, nargs;
-
/* Calls to internal functions always match their signature. */
if (gimple_call_internal_p (stmt))
return true;
- nargs = gimple_call_num_args (stmt);
+ unsigned int nargs = gimple_call_num_args (stmt);
- /* Get argument types for verification. */
- if (fndecl)
- parms = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
- else
- parms = TYPE_ARG_TYPES (gimple_call_fntype (stmt));
-
- /* Verify if the type of the argument matches that of the function
- declaration. If we cannot verify this or there is a mismatch,
- return false. */
+ /* If we have access to the declarations of formal parameters, match against
+ those. */
if (fndecl && DECL_ARGUMENTS (fndecl))
{
+ unsigned int i;
+ tree p;
+
for (i = 0, p = DECL_ARGUMENTS (fndecl);
i < nargs;
i++, p = DECL_CHAIN (p))
@@ -3676,17 +3669,75 @@ gimple_check_call_args (gimple *stmt, tree fndecl, bool args_count_match)
}
if (args_count_match && p)
return false;
+
+ return true;
}
- else if (parms)
+
+ /* If we don't have decls of formal parameters, see if we have both the type
+ of the callee arguments in the fntype of the call statement and compare
+ those. We rely on the fact that whatever promotions happened to
+ declarations for exactly the same sequence of types of parameters also
+ happened on the callee side. */
+ if (fndecl
+ && TYPE_ARG_TYPES (TREE_TYPE (fndecl))
+ && TYPE_ARG_TYPES (gimple_call_fntype (stmt)))
{
- for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
+ unsigned int arg_idx = 0;
+ for (tree f = TYPE_ARG_TYPES (TREE_TYPE (fndecl)),
+ s = TYPE_ARG_TYPES (gimple_call_fntype (stmt));
+ f || s;
+ f = TREE_CHAIN (f), s = TREE_CHAIN (s), arg_idx++)
{
+ if (!f
+ || !s
+ || TREE_VALUE (f) == error_mark_node
+ || TREE_VALUE (s) == error_mark_node)
+ return false;
+ if (TREE_CODE (TREE_VALUE (f)) == VOID_TYPE)
+ {
+ if (TREE_CODE (TREE_VALUE (s)) != VOID_TYPE
+ || arg_idx != nargs)
+ return false;
+ else
+ break;
+ }
+
tree arg;
+
+ if (arg_idx >= nargs
+ || (arg = gimple_call_arg (stmt, arg_idx)) == error_mark_node)
+ return false;
+
+ if (TREE_CODE (TREE_VALUE (s)) == VOID_TYPE
+ || (!types_compatible_p (TREE_VALUE (f), TREE_VALUE (s))
+ && !fold_convertible_p (TREE_VALUE (f), arg)))
+ return false;
+ }
+
+ if (args_count_match && arg_idx != nargs)
+ return false;
+
+ return true;
+ }
+
+ /* If we only have the fntype extracted from the call statement, check it
+ against the type of declarations while being pessimistic about
+ promotions. */
+ tree p;
+
+ if (fndecl)
+ p = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
+ else
+ p = TYPE_ARG_TYPES (TREE_TYPE (gimple_call_fn (stmt)));
+ if (p)
+ {
+ for (unsigned i = 0; i < nargs; i++, p = TREE_CHAIN (p))
+ {
/* If this is a varargs function defer inlining decision
to callee. */
if (!p)
break;
- arg = gimple_call_arg (stmt, i);
+ tree arg = gimple_call_arg (stmt, i);
if (TREE_VALUE (p) == error_mark_node
|| arg == error_mark_node
|| TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
diff --git a/gcc/testsuite/g++.dg/lto/pr70929_0.C b/gcc/testsuite/g++.dg/lto/pr70929_0.C
new file mode 100644
index 00000000000..c96fb1c743a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr70929_0.C
@@ -0,0 +1,18 @@
+// { dg-lto-do run }
+// { dg-lto-options { "-O3 -flto" } }
+
+struct s
+{
+ int a;
+ s() {a=1;}
+ ~s() {}
+};
+int t(struct s s);
+int main()
+{
+ s s;
+ int v=t(s);
+ if (!__builtin_constant_p (v))
+ __builtin_abort ();
+ return 0;
+}
diff --git a/gcc/testsuite/g++.dg/lto/pr70929_1.C b/gcc/testsuite/g++.dg/lto/pr70929_1.C
new file mode 100644
index 00000000000..b33aa8f35f0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr70929_1.C
@@ -0,0 +1,10 @@
+struct s
+{
+ int a;
+ s() {a=1;}
+ ~s() {}
+};
+int t(struct s s)
+{
+ return s.a;
+}
--
2.23.0
More information about the Gcc-patches
mailing list