| Message ID | 11a31789-d027-2313-baee-4ea1f460920b@suse.cz |
|---|---|
| State | New |
| Headers | show |
Honza? Thanks, Martin On 06/30/2017 03:50 PM, Martin Liška wrote: > On 06/28/2017 05:18 PM, Jan Hubicka wrote: >>> On 06/28/2017 04:24 PM, Jan Hubicka wrote: >>>>> - /* If callee has no option attributes, then it is ok to inline. */ >>>>> - if (!callee_tree) >>>>> + /* If callee has no option attributes (or default), >>>>> + then it is ok to inline. */ >>>>> + if (!callee_tree || callee_tree == target_option_default_node) >>>> >>>> I am not sure this actually makes sense, because target_option_default_node is not very >>>> meaningful for LTO (it contains whatever was passed to LTO driver). >>> >>> I see! >>> >>> Perhaps one can check >>>> for explicit optimization/machine attribute and whether caller and callee come from > > I'm not sure what you mean by 'for explicit optimization/machine attribute' ? > > I'm attaching a new patch, is it closer? > > Martin > >>>> same compilation unit, though this is quite hackish and will do unexpected things with COMDATs. >>> >>> That's quite cumbersome. Any other idea than marking the PR as won't fix? >> >> Yep, it is not prettiest. The problem is that the concept that callee can change semantics >> when no explicit attribute is present is sloppy. I am not sure how many programs rely on it >> (it is kind of surprising to see functions not being inlined into your target attribute annotated >> function I guess). >> Note that we check for original file in inliner already - this can be done by comparing lto_file_data >> of corresponding cgraph nodes. >> >> Honza >> >
> >>>>> - /* If callee has no option attributes, then it is ok to inline. */ > >>>>> - if (!callee_tree) > >>>>> + /* If callee has no option attributes (or default), > >>>>> + then it is ok to inline. */ > >>>>> + if (!callee_tree || callee_tree == target_option_default_node) > >>>> > >>>> I am not sure this actually makes sense, because target_option_default_node is not very > >>>> meaningful for LTO (it contains whatever was passed to LTO driver). > >>> > >>> I see! > >>> > >>> Perhaps one can check > >>>> for explicit optimization/machine attribute and whether caller and callee come from > > > > I'm not sure what you mean by 'for explicit optimization/machine attribute' ? You can simply do lookup_attribute and see if callee_tree was set because of attribute or because of LTO. In current patch the comparsion with target_option_default_node still does not make much of sense. But perhaps just lookup_attribute ("optimize", DECL_ATTRIBUTES (...)) would do the job. This is already done in ipa-inline. Honza > > > > I'm attaching a new patch, is it closer? > > > > Martin > > > >>>> same compilation unit, though this is quite hackish and will do unexpected things with COMDATs. > >>> > >>> That's quite cumbersome. Any other idea than marking the PR as won't fix? > >> > >> Yep, it is not prettiest. The problem is that the concept that callee can change semantics > >> when no explicit attribute is present is sloppy. I am not sure how many programs rely on it > >> (it is kind of surprising to see functions not being inlined into your target attribute annotated > >> function I guess). > >> Note that we check for original file in inliner already - this can be done by comparing lto_file_data > >> of corresponding cgraph nodes. > >> > >> Honza > >> > >
From c07dd3f079382dca617f1a2c32b83f7eaa1797f9 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Tue, 27 Jun 2017 16:39:27 +0200 Subject: [PATCH] Make inlining consistent in LTO and non-LTO mode (PR target/71991). gcc/testsuite/ChangeLog: 2017-06-28 Martin Liska <mliska@suse.cz> PR target/71991 * gcc.dg/torture/pr71991.c: New test. gcc/ChangeLog: 2017-06-28 Martin Liska <mliska@suse.cz> PR target/71991 * config/i386/i386.c (ix86_can_inline_p): Make inlining consistent in LTO and non-LTO mode. --- gcc/config/i386/i386.c | 9 +++++++-- gcc/testsuite/gcc.dg/torture/pr71991.c | 12 ++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr71991.c diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 2c4479e1751..92cda94b556 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -7438,8 +7438,13 @@ ix86_can_inline_p (tree caller, tree callee) tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller); tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (callee); - /* If callee has no option attributes, then it is ok to inline. */ - if (!callee_tree) + /* If callee has no option attributes (or default), + then it is ok to inline. */ + cgraph_node *caller_node = cgraph_node::get (caller); + cgraph_node *callee_node = cgraph_node::get (callee); + if (!callee_tree + || (callee_tree == target_option_default_node + && caller_node->lto_file_data == callee_node->lto_file_data)) ret = true; /* If caller has no option attributes, but callee does then it is not ok to diff --git a/gcc/testsuite/gcc.dg/torture/pr71991.c b/gcc/testsuite/gcc.dg/torture/pr71991.c new file mode 100644 index 00000000000..79c927f6844 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr71991.c @@ -0,0 +1,12 @@ +/* PR target/71991 */ + +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ +/* { dg-options "-std=c99" } */ + +static inline int __attribute__ ((__always_inline__)) fn1 () { return 0; } +static inline int __attribute__ ((target("inline-all-stringops"))) fn2 () { return fn1 (); } + +int main() +{ + return fn2(); +} -- 2.13.1