Nice!

================
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);
----------------
nit pick: I think it would be more in line with existing style to declare 
Locality and RW where they're initialized below

================
Comment at: lib/Sema/SemaChecking.cpp:700
@@ +699,3 @@
+  switch (BuiltinID) {
+  case X86::BI_mm_prefetch:
+    return SemaBuiltinMMPrefetch(TheCall);
----------------
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?

================
Comment at: lib/Sema/SemaChecking.cpp:1978
@@ +1977,3 @@
+
+  if (NumArgs > 2)
+    return Diag(TheCall->getLocEnd(),
----------------
we've declared in the .def file that it takes two parameters.. do we still have 
to check that manually?

================
Comment at: test/Headers/mmprefetch.c:3
@@ +2,3 @@
+#include <mmintrin.h>
+void _mm_prefetch(char const*, int);
+
----------------
maybe add a comment that this tests that redeclaration works?

================
Comment at: test/Headers/mmprefetch.c:6
@@ +5,3 @@
+void f() {
+  static char a = 0;
+  _mm_prefetch(&a, 0);
----------------
I guess the static isn't important here?


http://llvm-reviews.chandlerc.com/D2822
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to