compnerd closed this revision.
compnerd added a comment.
SVN r277928
https://reviews.llvm.org/D22774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mstorsjo added a comment.
Can someone commit this for me?
https://reviews.llvm.org/D22774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
compnerd accepted this revision.
compnerd added a comment.
Ah, okay. Sounds good.
https://reviews.llvm.org/D22774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rengolin added a comment.
The include_next is to make sure that, whatever other environment that happens
to have the same include name, should pick it up if the user or the environment
provides one, or errors out if it doesn't. Pretty standard.
https://reviews.llvm.org/D22774
__
mstorsjo added a comment.
In https://reviews.llvm.org/D22774#507014, @compnerd wrote:
> Why `include_next` the header if not on Windows?
Mostly because intrin.h uses the same logic. Normally, there's probably no such
system header in other SDKs, but say if you happen to have a custom one named
compnerd added a comment.
I had reversed the condition in my head. Why `include_next` the header if not
on Windows?
https://reviews.llvm.org/D22774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
mstorsjo added a comment.
Ping @compnerd
https://reviews.llvm.org/D22774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mstorsjo added a comment.
Ping - any reply from @compnerd?
In any case, feel free to formulate the ifdefs whichever way you want on commit
as well (since I don't have commit access), as long as the included test passes.
https://reviews.llvm.org/D22774
___
mstorsjo added inline comments.
Comment at: lib/Headers/armintr.h:26
@@ +25,3 @@
+#ifndef _MSC_VER
+#include_next
+#else
compnerd wrote:
> Hmm, why not do __has_header instead? armv7-windows-itanium won't define
> `_MSC_VER` but you may have the header from the
compnerd added inline comments.
Comment at: lib/Headers/armintr.h:26
@@ +25,3 @@
+#ifndef _MSC_VER
+#include_next
+#else
Hmm, why not do __has_header instead? armv7-windows-itanium won't define
`_MSC_VER` but you may have the header from the SDK.
https://revi
rengolin accepted this revision.
rengolin added a reviewer: rengolin.
rengolin added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks!
https://reviews.llvm.org/D22774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ht
mstorsjo updated this revision to Diff 66306.
mstorsjo added a comment.
Added a test, added "windows" to the header title line, added an `#ifndef
_MSC_VER #include_next` to the header.
https://reviews.llvm.org/D22774
Files:
lib/Headers/CMakeLists.txt
lib/Headers/armintr.h
lib/Headers/int
rengolin added inline comments.
Comment at: lib/Headers/armintr.h:27
@@ +26,3 @@
+
+typedef enum
+{
mstorsjo wrote:
> rengolin wrote:
> > mstorsjo wrote:
> > > rengolin wrote:
> > > > Maybe an ifdef Windows wrapper, to make sure not to mess up *nix
> > > > enviro
mstorsjo added inline comments.
Comment at: lib/Headers/armintr.h:27
@@ +26,3 @@
+
+typedef enum
+{
rengolin wrote:
> mstorsjo wrote:
> > rengolin wrote:
> > > Maybe an ifdef Windows wrapper, to make sure not to mess up *nix
> > > environments?
> > I can add the
rengolin added inline comments.
Comment at: lib/Headers/armintr.h:27
@@ +26,3 @@
+
+typedef enum
+{
mstorsjo wrote:
> rengolin wrote:
> > Maybe an ifdef Windows wrapper, to make sure not to mess up *nix
> > environments?
> I can add the same as at the top of intr
mstorsjo added a comment.
In https://reviews.llvm.org/D22774#501993, @rengolin wrote:
> Can you add a test on test/Headers, like the others, please?
Sure, there seems to be a good spot for that in ms-intrin.cpp.
Comment at: lib/Headers/armintr.h:1
@@ +1,2 @@
+/*=== armint
rengolin added a reviewer: compnerd.
rengolin added a subscriber: compnerd.
rengolin added a comment.
Can you add a test on test/Headers, like the others, please?
I'm not well versed on Windows. @compnerd?
Comment at: lib/Headers/armintr.h:1
@@ +1,2 @@
+/*=== armintr.h - AR
mstorsjo created this revision.
mstorsjo added a subscriber: cfe-commits.
Herald added subscribers: samparker, rengolin, aemerson.
This fixes compiling with headers from the Windows SDK for ARM, where the
YieldProcessor function (in winnt.h) refers to _ARM_BARRIER_ISHST.
The actual MSVC armintr.
18 matches
Mail list logo
