[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