rnk added a comment.
Now might be the time to solve the larger problem of wider intrinsic
availability. Like I mentioned, all these intrinsics really ought to be
available all the time, regardless of CPU subtarget.
Repository:
rL LLVM
https://reviews.llvm.org/D21959
_
guyblank added a comment.
removing the MSC_VER check will not be enough, the feature guards from the
intrinsic and the builtin need to be removed to make it work. not sure if this
is the right way to go, any thoughts on this?
Repository:
rL LLVM
https://reviews.llvm.org/D21959
__
rnk added a comment.
Hm, resending my comments because it doesn't appear to work from email. I swear
it used to...
In https://reviews.llvm.org/D21959#519179, @guyblank wrote:
> Still, __XSAVE__ should have been defined when compiling for a target that
> supports the feature.
That's not how M
guyblank added a comment.
Still, __XSAVE__ should have been defined when compiling for a target that
supports the feature.
But anyway, the xsaveintrin.h is quite small so always including it shouldn't
be an issue.
Are you ok with me removing the #if just for this header file, or would you
like
rnk added a comment.
The source isn't that interesting, it includes intrin.h and immintrin.h before
using _xgetbv.
I think the issue is that Nico added the _MSC_VER check to intrin.h in
http://reviews.llvm.org/D20291:
#if !defined(_MSC_VER) || __has_feature(modules) || defined(__XSAVE__)
#incl
guyblank added a comment.
Sorry about that, forgot that i changed the ms_intrin test.
about the failure, I think that xsaveintrin.h is not being included because it
requires the xsave feature - which should be on if the target supports it.
do you know what in which target the failure occurred? a
rnk added a subscriber: rnk.
rnk added a comment.
Reverted in r278814, it appears to break usage of _xgetbv on Windows:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin%28dll%29/builds/5846/steps/compile/logs/stdio
../../base/cpu.cc(194,10): error: use of undeclared identifier '_xge
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278783: [X86] Add xgetbv/x[X86] Add xgetbv xsetbv intrinsics
to non-windows platforms (authored by myatsina).
Changed prior to commit:
https://reviews.llvm.org/D21959?vs=65676&id=68146#toc
Repository:
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D21959
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li
guyblank added a comment.
If there aren't any further objections, I'd like go ahead with the commit.
https://reviews.llvm.org/D21959
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
guyblank added a comment.
ping
https://reviews.llvm.org/D21959
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
guyblank updated this revision to Diff 65676.
guyblank marked an inline comment as done.
https://reviews.llvm.org/D21959
Files:
include/clang/Basic/BuiltinsX86.def
lib/CodeGen/CGBuiltin.cpp
lib/Headers/intrin.h
lib/Headers/xsaveintrin.h
test/CodeGen/builtins-x86.c
test/CodeGen/x86_32-
craig.topper added a comment.
This change seems consistent with similar being done in r250158 when the other
xsave intrinsics were added. intrin.h includes x86intrin.h which in turn
includes xsaveintrin.h. So this just makes xgetbv/xsetbv available on
non-microsoft platforms.
guyblank added a comment.
the include is because i added calls to the intrinsics themselves
in the test, no just the builtins.
Comment at: lib/Headers/intrin.h:905
@@ -906,9 +904,3 @@
}
-static __inline__ unsigned __int64 __cdecl __DEFAULT_FN_ATTRS
-_xgetbv(unsigned int __xcr
delena added a comment.
#include in the test is not clear for me. Does it mean that you
broke backward compatibility?
Comment at: lib/CodeGen/CGBuiltin.cpp:6779
@@ -6776,1 +6778,3 @@
}
+ case X86::BI__builtin_ia32_xgetbv: {
+return Builder.CreateCall(CGM.getIntrinsic(In
guyblank created this revision.
guyblank added reviewers: aaboud, delena, craig.topper, AsafBadouh,
m_zuckerman, igorb.
guyblank added a subscriber: cfe-commits.
[X86] Add xgetbv xsetbv intrinsics
http://reviews.llvm.org/D21959
Files:
include/clang/Basic/BuiltinsX86.def
lib/CodeGen/CGBuilt
16 matches
Mail list logo
