r240522 - Move the special-case check from r240462 into ARM-specific code.
Bob Wilson
bob.wilson at apple.com
Tue Jun 23 23:05:20 PDT 2015
Author: bwilson
Date: Wed Jun 24 01:05:20 2015
New Revision: 240522
URL: http://llvm.org/viewvc/llvm-project?rev=240522&view=rev
Log:
Move the special-case check from r240462 into ARM-specific code.
This fixes a serious bug in r240462: checking the BuiltinID for
ARM::BI_MoveToCoprocessor* in EmitBuiltinExpr() ignores the fact that
each target has an overlapping range of the BuiltinID values. That check
can trigger for builtins from other targets, leading to very bad
behavior.
Part of the reason I did not implement r240462 this way to begin with is
the special handling of the last argument for Neon builtins. In this
change, I have factored out the check to see which builtins have that
extra argument into a new HasExtraNeonArgument() function. There is still
some awkwardness in having to check for those builtins in two separate
places, i.e., once to see if the extra argument is present and once to
generate the appropriate IR, but this seems much cleaner than my previous
patch.
Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=240522&r1=240521&r2=240522&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Jun 24 01:05:20 2015
@@ -1831,14 +1831,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(
Args.push_back(ArgValue);
}
- // The ARM _MoveToCoprocessor builtins put the input register value as
- // the first argument, but the LLVM intrinsic expects it as the third one.
- if (BuiltinID == ARM::BI_MoveToCoprocessor ||
- BuiltinID == ARM::BI_MoveToCoprocessor2) {
- return RValue::get(Builder.CreateCall(F, {Args[1], Args[2], Args[0],
- Args[3], Args[4], Args[5]}));
- }
-
Value *V = Builder.CreateCall(F, Args);
QualType BuiltinRetType = E->getType();
@@ -3347,6 +3339,42 @@ static Value *EmitSpecialRegisterBuiltin
return Builder.CreateCall(F, { Metadata, ArgValue });
}
+/// Return true if BuiltinID is an overloaded Neon intrinsic with an extra
+/// argument that specifies the vector type.
+static bool HasExtraNeonArgument(unsigned BuiltinID) {
+ switch (BuiltinID) {
+ default: break;
+ case NEON::BI__builtin_neon_vget_lane_i8:
+ case NEON::BI__builtin_neon_vget_lane_i16:
+ case NEON::BI__builtin_neon_vget_lane_i32:
+ case NEON::BI__builtin_neon_vget_lane_i64:
+ case NEON::BI__builtin_neon_vget_lane_f32:
+ case NEON::BI__builtin_neon_vgetq_lane_i8:
+ case NEON::BI__builtin_neon_vgetq_lane_i16:
+ case NEON::BI__builtin_neon_vgetq_lane_i32:
+ case NEON::BI__builtin_neon_vgetq_lane_i64:
+ case NEON::BI__builtin_neon_vgetq_lane_f32:
+ case NEON::BI__builtin_neon_vset_lane_i8:
+ case NEON::BI__builtin_neon_vset_lane_i16:
+ case NEON::BI__builtin_neon_vset_lane_i32:
+ case NEON::BI__builtin_neon_vset_lane_i64:
+ case NEON::BI__builtin_neon_vset_lane_f32:
+ case NEON::BI__builtin_neon_vsetq_lane_i8:
+ case NEON::BI__builtin_neon_vsetq_lane_i16:
+ case NEON::BI__builtin_neon_vsetq_lane_i32:
+ case NEON::BI__builtin_neon_vsetq_lane_i64:
+ case NEON::BI__builtin_neon_vsetq_lane_f32:
+ case NEON::BI__builtin_neon_vsha1h_u32:
+ case NEON::BI__builtin_neon_vsha1cq_u32:
+ case NEON::BI__builtin_neon_vsha1pq_u32:
+ case NEON::BI__builtin_neon_vsha1mq_u32:
+ case ARM::BI_MoveToCoprocessor:
+ case ARM::BI_MoveToCoprocessor2:
+ return false;
+ }
+ return true;
+}
+
Value *CodeGenFunction::EmitARMBuiltinExpr(unsigned BuiltinID,
const CallExpr *E) {
if (auto Hint = GetValueForARMHint(BuiltinID))
@@ -3599,7 +3627,9 @@ Value *CodeGenFunction::EmitARMBuiltinEx
SmallVector<Value*, 4> Ops;
llvm::Value *Align = nullptr;
- for (unsigned i = 0, e = E->getNumArgs() - 1; i != e; i++) {
+ bool HasExtraArg = HasExtraNeonArgument(BuiltinID);
+ unsigned NumArgs = E->getNumArgs() - (HasExtraArg ? 1 : 0);
+ for (unsigned i = 0, e = NumArgs; i != e; i++) {
if (i == 0) {
switch (BuiltinID) {
case NEON::BI__builtin_neon_vld1_v:
@@ -3674,8 +3704,7 @@ Value *CodeGenFunction::EmitARMBuiltinEx
switch (BuiltinID) {
default: break;
- // vget_lane and vset_lane are not overloaded and do not have an extra
- // argument that specifies the vector type.
+
case NEON::BI__builtin_neon_vget_lane_i8:
case NEON::BI__builtin_neon_vget_lane_i16:
case NEON::BI__builtin_neon_vget_lane_i32:
@@ -3686,8 +3715,8 @@ Value *CodeGenFunction::EmitARMBuiltinEx
case NEON::BI__builtin_neon_vgetq_lane_i32:
case NEON::BI__builtin_neon_vgetq_lane_i64:
case NEON::BI__builtin_neon_vgetq_lane_f32:
- return Builder.CreateExtractElement(Ops[0], EmitScalarExpr(E->getArg(1)),
- "vget_lane");
+ return Builder.CreateExtractElement(Ops[0], Ops[1], "vget_lane");
+
case NEON::BI__builtin_neon_vset_lane_i8:
case NEON::BI__builtin_neon_vset_lane_i16:
case NEON::BI__builtin_neon_vset_lane_i32:
@@ -3698,29 +3727,34 @@ Value *CodeGenFunction::EmitARMBuiltinEx
case NEON::BI__builtin_neon_vsetq_lane_i32:
case NEON::BI__builtin_neon_vsetq_lane_i64:
case NEON::BI__builtin_neon_vsetq_lane_f32:
- Ops.push_back(EmitScalarExpr(E->getArg(2)));
return Builder.CreateInsertElement(Ops[1], Ops[0], Ops[2], "vset_lane");
- // Non-polymorphic crypto instructions also not overloaded
case NEON::BI__builtin_neon_vsha1h_u32:
- Ops.push_back(EmitScalarExpr(E->getArg(0)));
return EmitNeonCall(CGM.getIntrinsic(Intrinsic::arm_neon_sha1h), Ops,
"vsha1h");
case NEON::BI__builtin_neon_vsha1cq_u32:
- Ops.push_back(EmitScalarExpr(E->getArg(2)));
return EmitNeonCall(CGM.getIntrinsic(Intrinsic::arm_neon_sha1c), Ops,
"vsha1h");
case NEON::BI__builtin_neon_vsha1pq_u32:
- Ops.push_back(EmitScalarExpr(E->getArg(2)));
return EmitNeonCall(CGM.getIntrinsic(Intrinsic::arm_neon_sha1p), Ops,
"vsha1h");
case NEON::BI__builtin_neon_vsha1mq_u32:
- Ops.push_back(EmitScalarExpr(E->getArg(2)));
return EmitNeonCall(CGM.getIntrinsic(Intrinsic::arm_neon_sha1m), Ops,
"vsha1h");
+
+ // The ARM _MoveToCoprocessor builtins put the input register value as
+ // the first argument, but the LLVM intrinsic expects it as the third one.
+ case ARM::BI_MoveToCoprocessor:
+ case ARM::BI_MoveToCoprocessor2: {
+ Function *F = CGM.getIntrinsic(BuiltinID == ARM::BI_MoveToCoprocessor ?
+ Intrinsic::arm_mcr : Intrinsic::arm_mcr2);
+ return Builder.CreateCall(F, {Ops[1], Ops[2], Ops[0],
+ Ops[3], Ops[4], Ops[5]});
+ }
}
// Get the last argument, which specifies the vector type.
+ assert(HasExtraArg);
llvm::APSInt Result;
const Expr *Arg = E->getArg(E->getNumArgs()-1);
if (!Arg->isIntegerConstantExpr(Result, getContext()))
More information about the cfe-commits
mailing list