[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Martin Liška changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #15 from Martin Liška --- Let's close this as we're not planning to backport that.
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Jakub Jelinek changed: What|Removed |Added Target Milestone|8.2 |8.3 --- Comment #14 from Jakub Jelinek --- GCC 8.2 has been released.
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Jakub Jelinek changed: What|Removed |Added Target Milestone|8.0 |8.2 --- Comment #13 from Jakub Jelinek --- GCC 8.1 has been released.
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 --- Comment #12 from Jan Hubicka --- Author: hubicka Date: Fri Apr 13 08:51:47 2018 New Revision: 259367 URL: https://gcc.gnu.org/viewcvs?rev=259367=gcc=rev Log: PR lto/71991 * config/i386/i386.c (ix86_can_inline_p): Allow safe transitions for always inline. * gcc.target/i386/pr71991.c: New testcase. Added: trunk/gcc/testsuite/gcc.target/i386/pr71991.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.c trunk/gcc/testsuite/ChangeLog
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #11 from Jakub Jelinek --- (In reply to Jan Hubicka from comment #5) > Created attachment 43798 [details] > proposed fix > > > this patch simply while-lists some transitions of target flags for always > inline functions. It is ugly but I can't think of anything else which would > look safe. > Martin, you mentioned there was packages broken by this. Perhaps we can try > rebuild with this patch to see if all of the real world issues are solved? > If not, i guess we will need to decide case-by-case what is safe and what > not :( I think that patch is quite reasonable, with a couple of small nits LGTM s/alwyas/always/ put = on the next line in bool always_inline = and perhaps add int safe_mask = always_inline ? always_inline_safe_mask : 0; and use unconditionally (caller_opts->x_target_flags & ~safe_mask) != (callee_opts->x_target_flags & ~safe_mask) maybe use unsigned HOST_WIDE_INT instead of int for the masks, so when the masks grow beyond 32 bits we don't suddenly misbehave (or add gcc_chec king_assert that sizeof (callee_opts->x_target_flags) == sizeof (always_inline_safe_mask) The xen case should just be fixed in xen IMHO.
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991
--- Comment #10 from Jan Hubicka ---
> > Well, I have tried to discuss this on IRC couple times but we got no
> > conclusion what to do here. I think I will simply go with the proposed patch
> > + additional hack to ignore arch mismatches when callee has no explicit
> > target attribute?
>
> Or we can reject that and fix the affected packages if you believe it's just
> hack. Eventually one can remove the problematic always_inline attribute.
Well, in theory, it is all lazyness on the side of gcc - of course one can in
theory track ISA attributes at instruction granuality and make all those
inlines safe. But of course this is becuase optimize/target attributes are
hacks by themselves.
The proposed patch is consistent with hacks we already have in ipa-inline
/* When user added an attribute to the callee honor it. */
else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl))
&& opts_for_fn (caller->decl) != opts_for_fn (callee->decl))
It may not be that easy to fix these at user side - for example current logic
will block you from using xmmintrin.h functions from any function with custom
target attribute specifying ISA flags, which I think was one of original
motivations
for those.
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 --- Comment #9 from Martin Liška --- (In reply to Jan Hubicka from comment #8) > Well, I have tried to discuss this on IRC couple times but we got no > conclusion what to do here. I think I will simply go with the proposed patch > + additional hack to ignore arch mismatches when callee has no explicit > target attribute? Or we can reject that and fix the affected packages if you believe it's just hack. Eventually one can remove the problematic always_inline attribute.
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 --- Comment #8 from Jan Hubicka --- Well, I have tried to discuss this on IRC couple times but we got no conclusion what to do here. I think I will simply go with the proposed patch + additional hack to ignore arch mismatches when callee has no explicit target attribute?
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Martin Liška changed: What|Removed |Added Status|NEW |ASSIGNED
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Martin Liška changed: What|Removed |Added Assignee|marxin at gcc dot gnu.org |hubicka at gcc dot gnu.org --- Comment #7 from Martin Liška --- Assigning to Honza.
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Martin Liška changed: What|Removed |Added Target Milestone|--- |8.0
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991
--- Comment #6 from Martin Liška ---
(In reply to Jan Hubicka from comment #5)
> Created attachment 43798 [details]
> proposed fix
>
>
> this patch simply while-lists some transitions of target flags for always
> inline functions. It is ugly but I can't think of anything else which would
> look safe.
> Martin, you mentioned there was packages broken by this. Perhaps we can try
> rebuild with this patch to see if all of the real world issues are solved?
> If not, i guess we will need to decide case-by-case what is safe and what
> not :(
Thanks Honza for the patch. I see 2 failing packages:
1) xf86-video, it uses https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991#c0
Thus it will be fixed.
2) xen
It contains:
cat fuzz-emul.i
__attribute__((__always_inline__)) a() {}
#pragma GCC target "no-sse"
b() { a(); }
Where 'a' is 'memcpy' and 'b' is a function in xen.
Can you Honza also take a look r251333 where we started to reject such code?
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Jan Hubicka changed: What|Removed |Added CC||hubicka at gcc dot gnu.org --- Comment #5 from Jan Hubicka --- Created attachment 43798 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43798=edit proposed fix this patch simply while-lists some transitions of target flags for always inline functions. It is ugly but I can't think of anything else which would look safe. Martin, you mentioned there was packages broken by this. Perhaps we can try rebuild with this patch to see if all of the real world issues are solved? If not, i guess we will need to decide case-by-case what is safe and what not :(
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Martin Liška changed: What|Removed |Added CC||dilyan.palauzov at aegee dot org --- Comment #4 from Martin Liška --- *** Bug 84926 has been marked as a duplicate of this bug. ***
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 --- Comment #3 from Martin Liška --- Ok, the first test-case started to fail w/ LTO from r217659. Honza, can you please take a look and provide hint what to do?
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991
--- Comment #2 from Martin Liška ---
(In reply to Richard Biener from comment #1)
> The testcase misses an 'inline' on fn1 (thus all the warnings). But yes,
> confirmed. And I think the error is somewhat correct though I think it
> is due to some defect in target attribute handling somewhere - it works
> with "sse3" for example.
Ah, okey, this would be better sample:
$cat xf86-video/tc.i
static __attribute__ ((__always_inline__)) inline int fn1 () { return 0; }
static __attribute__ ((target("inline-all-stringops"))) inline int fn2 () { fn1
(); }
int main()
{
fn2();
}
$ gcc xf86-video/tc.i -O2
$ gcc xf86-video/tc.i -O2 -flto
xf86-video/tc.i: In function ‘fn2’:
xf86-video/tc.i:1:55: error: inlining failed in call to always_inline ‘fn1’:
target specific option mismatch
static __attribute__ ((__always_inline__)) inline int fn1 () { return 0; }
^~~
xf86-video/tc.i:2:77: note: called from here
static __attribute__ ((target("inline-all-stringops"))) inline int fn2 () {
fn1 (); }
sse3 works because ix86_can_inline_p returns true:
/* Callee's isa options should a subset of the caller's, i.e. a SSE4
function
can inline a SSE2 function but a SSE2 function can't inline a SSE4
function. */
if ((caller_opts->x_ix86_isa_flags & callee_opts->x_ix86_isa_flags)
!= callee_opts->x_ix86_isa_flags)
ret = false;
However following snippet is rejected w/ and w/o LTO:
cat xf86-video/tc2.i
static __attribute__ ((__always_inline__, target("sse4"))) inline int fn1 () {
return 0; }
static __attribute__ ((target("sse2"))) inline int fn2 () { fn1 (); }
int main()
{
fn2();
}
[Bug target/71991] Inconsistency for __attribute__ ((__always_inline__)) among LTO and non-LTO compilation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71991 Richard Biener changed: What|Removed |Added Keywords||lto Target||x86_64-*-* Status|UNCONFIRMED |NEW Last reconfirmed||2016-07-25 Component|lto |target Ever confirmed|0 |1 --- Comment #1 from Richard Biener --- The testcase misses an 'inline' on fn1 (thus all the warnings). But yes, confirmed. And I think the error is somewhat correct though I think it is due to some defect in target attribute handling somewhere - it works with "sse3" for example.
