[PATCH] Adding _mm_prefetch and other builtins

Hans Wennborg hans at chromium.org
Tue Feb 18 16:59:51 PST 2014


  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



More information about the cfe-commits mailing list