rsmith added inline comments.
================
Comment at: include/clang/Basic/Builtins.def:1249-1255
@@ +1248,9 @@
+BUILTIN(__builtin_nontemporal_store, "v.", "t")
+BUILTIN(__builtin_nontemporal_store_1, "vcc*.", "")
+BUILTIN(__builtin_nontemporal_store_2, "vss*.", "")
+BUILTIN(__builtin_nontemporal_store_4, "vii*.", "")
+BUILTIN(__builtin_nontemporal_store_8, "vLLiLLi*.", "")
+BUILTIN(__builtin_nontemporal_store_16, "vLLLiLLLi*.", "")
+BUILTIN(__builtin_nontemporal_store_f, "vff*.", "")
+BUILTIN(__builtin_nontemporal_store_d, "vdd*.", "")
+
----------------
Do we need these typed versions?
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6198-6203
@@ +6197,8 @@
+ "address argument to nontemporal builtin must be a pointer (%0 invalid)">;
+def err_nontemporal_builtin_must_be_pointer_intfltptr : Error<
+ "address argument to nontemporal builtin must be a pointer to integer, float
"
+ "or pointer (%0 invalid)">;
+def err_nontemporal_builtin_pointer_size : Error<
+ "address argument to nontemporal builtin must be a pointer to 1,2,4,8 or 16 "
+ "byte type (%0 invalid)">;
+
----------------
Nontemporal loads and stores of, say, vector types seem useful, but I can
understand if you want to restrict the complexity of the CodeGen changes here.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:128-129
@@ +127,4 @@
+ // Convert the type of the pointer to a pointer to the stored type.
+ Value *BC = CGF.Builder.CreateBitCast(
+ Address, llvm::PointerType::getUnqual(ValueTy), "cast");
+ StoreInst *SI = CGF.Builder.CreateStore(Val, BC);
----------------
You should pass `Val` through `EmitToMemory` first to widen `bool` from `i1` to
`i8`.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:149
@@ +148,3 @@
+ LI->setAlignment(Align);
+ return LI;
+}
----------------
Likewise, you should pass this though `EmitFromMemory`.
================
Comment at: lib/Sema/SemaChecking.cpp:2235-2241
@@ +2234,9 @@
+
+ // Ensure that we have at least one argument to do type inference from.
+ if (TheCall->getNumArgs() < numArgs) {
+ Diag(TheCall->getLocEnd(), diag::err_typecheck_call_too_few_args_at_least)
+ << 0 << 1 << TheCall->getNumArgs()
+ << TheCall->getCallee()->getSourceRange();
+ return ExprError();
+ }
+
----------------
You should also reject calls with too many arguments here. Maybe just call
`checkArgCount(*this, TheCall, numArgs)`?
================
Comment at: lib/Sema/SemaChecking.cpp:2275-2357
@@ +2274,85 @@
+
+#define BUILTIN_ROW(x)
\
+ { Builtin::BI##x##_1, Builtin::BI##x##_2, Builtin::BI##x##_4,
\
+ Builtin::BI##x##_8, Builtin::BI##x##_16,
\
+ Builtin::BI##x##_f, Builtin::BI##x##_d }
+
+ static const unsigned BuiltinIndices[][7] = {
+ BUILTIN_ROW(__builtin_nontemporal_store),
+ BUILTIN_ROW(__builtin_nontemporal_load)
+ };
+#undef BUILTIN_ROW
+
+ // Determine the index of the size.
+ unsigned SizeIndex;
+ if (ValType->isFloatingType()) {
+ switch (Context.getTypeSizeInChars(ValType).getQuantity()) {
+ case 4: SizeIndex = 5; break;
+ case 8: SizeIndex = 6; break;
+ default:
+ Diag(DRE->getLocStart(), diag::err_nontemporal_builtin_pointer_size)
+ << AddressArg->getType() << AddressArg->getSourceRange();
+ return ExprError();
+ }
+ } else {
+ switch (Context.getTypeSizeInChars(ValType).getQuantity()) {
+ case 1: SizeIndex = 0; break;
+ case 2: SizeIndex = 1; break;
+ case 4: SizeIndex = 2; break;
+ case 8: SizeIndex = 3; break;
+ case 16: SizeIndex = 4; break;
+ default:
+ Diag(DRE->getLocStart(), diag::err_nontemporal_builtin_pointer_size)
+ << AddressArg->getType() << AddressArg->getSourceRange();
+ return ExprError();
+ }
+ }
+ // Get the decl for the concrete builtin from this, we can tell what the
+ // concrete integer type we should convert to is.
+ unsigned NewBuiltinID = BuiltinIndices[isStore ? 0 : 1][SizeIndex];
+ const char *NewBuiltinName = Context.BuiltinInfo.getName(NewBuiltinID);
+
+ // Perform builtin lookup to avoid redeclaring it.
+ DeclarationName DN(&Context.Idents.get(NewBuiltinName));
+ LookupResult Res(*this, DN, DRE->getLocStart(), LookupOrdinaryName);
+ LookupName(Res, TUScope, /*AllowBuiltinCreation=*/true);
+ assert(Res.getFoundDecl());
+ FunctionDecl *NewBuiltinDecl = dyn_cast<FunctionDecl>(Res.getFoundDecl());
+ if (!NewBuiltinDecl)
+ return ExprError();
+
+ // For stores convert the value (the first argument) to the type of the
+ // pointer (the second argument).
+ if (isStore) {
+ ExprResult Arg = TheCall->getArg(0);
+ InitializedEntity Entity = InitializedEntity::InitializeParameter(
+ Context, ValType, /*consume*/ false);
+ Arg = PerformCopyInitialization(Entity, SourceLocation(), Arg);
+ if (Arg.isInvalid())
+ return ExprError();
+
+ TheCall->setArg(0, Arg.get());
+ }
+
+ ASTContext &Context = this->getASTContext();
+
+ // Create a new DeclRefExpr to refer to the new decl.
+ DeclRefExpr *NewDRE = DeclRefExpr::Create(
+ Context, DRE->getQualifierLoc(), SourceLocation(), NewBuiltinDecl,
+ /*enclosing*/ false, DRE->getLocation(), Context.BuiltinFnTy,
+ DRE->getValueKind());
+
+ // Set the callee in the CallExpr.
+ // FIXME: This loses syntactic information.
+ QualType CalleePtrTy = Context.getPointerType(NewBuiltinDecl->getType());
+ ExprResult PromotedCall =
+ ImpCastExprToType(NewDRE, CalleePtrTy, CK_BuiltinFnToFnPtr);
+ TheCall->setCallee(PromotedCall.get());
+
+ // For loads change the result type of the call to match the original value
+ // type. This is arbitrary, but the codegen for these builtins ins design to
+ // handle it gracefully.
+ if (!isStore)
+ TheCall->setType(ValType);
+
+ return TheCallResult;
----------------
Instead of doing all of this, you can just (for stores) check the type of the
second parameter or (for loads) set the type of the call expression. (See
`CheckARMBuiltinExclusiveCall` for an example.)
http://reviews.llvm.org/D12313
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
