Created attachment 41584 [details] c code Hi All, I've been investigating function multi-versioning feature and it works as expected except when I compile with -Ofast. When adding -Ofast to the compile what I see is that each time the multi-versioned function is called is that the *resolver* is called but that neither of the implementations are called. Code, build commands and example outputs & .asm attached. Regards, Billy
I see the same behaviour with 5.4.0.
COLLECT_GCC=gcc-6 COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/6/lto-wrapper Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 6.2.0-3ubuntu11~16.04' --with-bugurl=file:///usr/share/doc/gcc-6/README.Bugs --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-6 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-6-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-6-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-6-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 6.2.0 20160901 (Ubuntu 6.2.0-3ubuntu11~16.04)
Created attachment 41585 [details] Preprocessed main file
Confirmed. Even -O1 breaks it, I suppose we somehow inline the resolver improperly which means a bogus cgraph is constructed? GCC 4.8 gets it right at -O1 but fails at -O2, so I guess this never worked correctly. Declaring resolve_do_it_right_at_runtime with __attribute__((noinline,noclone)) doesn't fix this though... main () ends up calling the resolver directly, no ifunc is emitted. I'm not 100% sure this is proper use of attribute ifunc, defering to Martin here.
Well, I'm also not sure how it should behave. What happens currently is that we have static alias and static resolver. Thus ipa-visibility finds out that call to do_it_right_at_runtime can be replaced with resolve_do_it_right_at_runtime. Then inlining happens. I'm adding Nathan who implemented ifunc support in GCC and should know answer. I suggest to not evaluate ifunc alias as not interposable. That would prevent the problem.
the ifunc attribute is essentially slightly different syntactic sugar to the alias attribute. It sounds like the optimizer is treating the attribute as-if it were an alias.
Created attachment 41607 [details] Patch candidate Idea is to not allow to inline via an ifunc alias and do not allow to can_replace_by_local_alias ifunc aliases. Thoughts?
Author: marxin Date: Wed Jun 28 12:47:24 2017 New Revision: 249735 URL: https://gcc.gnu.org/viewcvs?rev=249735&root=gcc&view=rev Log: Do not allow to inline ifunc resolvers (PR ipa/81128). 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * ipa-visibility.c (non_local_p): Handle visibility. 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * c-attribs.c (handle_alias_ifunc_attribute): Append ifunc alias to a function declaration. 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * gcc.target/i386/pr81128.c: New test. Added: trunk/gcc/testsuite/gcc.target/i386/pr81128.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-attribs.c trunk/gcc/ipa-visibility.c trunk/gcc/testsuite/ChangeLog
Fixed on trunk so far.
Hi All, thanks to Martin, Nathan and Richard for working on and fixing this issue. Can anyone say if the fix will be back ported to future point releases of the 5.x, 6.x, 7.x versions? Regards, Billy
(In reply to Billy O'Mahony from comment #10) > Hi All, > > thanks to Martin, Nathan and Richard for working on and fixing this issue. > > Can anyone say if the fix will be back ported to future point releases of > the 5.x, 6.x, 7.x versions? > > Regards, > Billy Well, in general I don't tend to backport multiversioning patches as many dependent patches were applied in GCC 8 cycle. But this one is quite independent. I'm planning to do backports next week.
Thanks, Martin. That's great. Can you update the ticket with the fixed versions when you get a chance? (In reply to Martin Liška from comment #11) > (In reply to Billy O'Mahony from comment #10) > > Hi All, > > > > thanks to Martin, Nathan and Richard for working on and fixing this issue. > > > > Can anyone say if the fix will be back ported to future point releases of > > the 5.x, 6.x, 7.x versions? > > > > Regards, > > Billy > > Well, in general I don't tend to backport multiversioning patches as many > dependent patches were applied in GCC 8 cycle. But this one is quite > independent. I'm planning to do backports next week.
Author: marxin Date: Fri Sep 15 08:12:30 2017 New Revision: 252782 URL: https://gcc.gnu.org/viewcvs?rev=252782&root=gcc&view=rev Log: Backport r249735 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * ipa-visibility.c (non_local_p): Handle visibility. 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * c-attribs.c (handle_alias_ifunc_attribute): Append ifunc alias to a function declaration. 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * gcc.target/i386/pr81128.c: New test. Added: branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/pr81128.c Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/c-family/ChangeLog branches/gcc-7-branch/gcc/c-family/c-attribs.c branches/gcc-7-branch/gcc/ipa-visibility.c branches/gcc-7-branch/gcc/testsuite/ChangeLog
Author: marxin Date: Fri Sep 15 09:13:29 2017 New Revision: 252792 URL: https://gcc.gnu.org/viewcvs?rev=252792&root=gcc&view=rev Log: Backport r249735 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * ipa-visibility.c (non_local_p): Handle visibility. 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * c-attribs.c (handle_alias_ifunc_attribute): Append ifunc alias to a function declaration. 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * gcc.target/i386/pr81128.c: New test. Added: branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/pr81128.c Modified: branches/gcc-6-branch/gcc/ChangeLog branches/gcc-6-branch/gcc/c-family/ChangeLog branches/gcc-6-branch/gcc/c-family/c-common.c branches/gcc-6-branch/gcc/ipa-visibility.c branches/gcc-6-branch/gcc/testsuite/ChangeLog
Fixed.
Author: marxin Date: Fri Sep 15 12:13:04 2017 New Revision: 252808 URL: https://gcc.gnu.org/viewcvs?rev=252808&root=gcc&view=rev Log: Backport r249735 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * ipa-visibility.c (non_local_p): Handle visibility. 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * c-attribs.c (handle_alias_ifunc_attribute): Append ifunc alias to a function declaration. 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR ipa/81128 * gcc.target/i386/pr81128.c: New test. Added: branches/gcc-5-branch/gcc/testsuite/gcc.target/i386/pr81128.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/c-family/ChangeLog branches/gcc-5-branch/gcc/c-family/c-common.c branches/gcc-5-branch/gcc/ipa-visibility.c branches/gcc-5-branch/gcc/testsuite/ChangeLog