Committed as r201734.
================
Comment at: lib/Sema/SemaChecking.cpp:700
@@ +699,3 @@
+ switch (BuiltinID) {
+ case X86::BI_mm_prefetch:
+ return SemaBuiltinMMPrefetch(TheCall);
----------------
Hans Wennborg wrote:
> i'm probably missing something, but why don't we have checks for the other
> builtins too? or is it just that we want to do some extra intelligent checks
> here?
We need to check that the int argument is known at compile time to be in [0,3]
================
Comment at: lib/Sema/SemaChecking.cpp:1978
@@ +1977,3 @@
+
+ if (NumArgs > 2)
+ return Diag(TheCall->getLocEnd(),
----------------
Hans Wennborg wrote:
> we've declared in the .def file that it takes two parameters.. do we still
> have to check that manually?
We do not, I'll remove the test. The updated lit tests verify that this is
still checked elsewhere.
================
Comment at: test/Headers/mmprefetch.c:3
@@ +2,3 @@
+#include <mmintrin.h>
+void _mm_prefetch(char const*, int);
+
----------------
Hans Wennborg wrote:
> maybe add a comment that this tests that redeclaration works?
done.
================
Comment at: test/Headers/mmprefetch.c:6
@@ +5,3 @@
+void f() {
+ static char a = 0;
+ _mm_prefetch(&a, 0);
----------------
Hans Wennborg wrote:
> I guess the static isn't important here?
correct. changed to be an argument to f.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:4790
@@ +4789,3 @@
+ case X86::BI_mm_prefetch: {
+ Value *Locality, *RW, *Address = EmitScalarExpr(E->getArg(0));
+ RW = ConstantInt::get(Int32Ty, 0);
----------------
Hans Wennborg wrote:
> nit pick: I think it would be more in line with existing style to declare
> Locality and RW where they're initialized below
Done. I agree. I have no idea why it was the way it was.
================
Comment at: include/clang/Basic/BuiltinsX86.def:31
@@ +30,3 @@
+BUILTIN(_InterlockedDecrement, "LiLiD*", "n")
+BUILTIN(_InterlockedExchangeAdd, "LiLiD*Li", "n")
+
----------------
David Majnemer wrote:
> These should not be available in all language modes. Perhaps use
> `LANGBUILTIN` with `ALL_MS_LANGUAGES`?
Done. x86 builtins don't support LANGBUILTIN so I had to move them to
Builtins.def. There is an appropriate place to put them.
================
Comment at: test/CodeGen/ms-builtins.c:14
@@ +13,3 @@
+ __noop();
+ __debugbreak();
+};
----------------
Hans Wennborg wrote:
> The __noop() and __debugbreak() seem redundant for the test..
Yes, all of this was part of me debugging before I realized we needed
-fms-extentions to get any of it to work. I forgot to remove it.
http://llvm-reviews.chandlerc.com/D2822
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
