r295935 - [CodeGen] Don't reemit expressions for pass_object_size params.
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 22 21:59:56 PST 2017
Author: gbiv
Date: Wed Feb 22 23:59:56 2017
New Revision: 295935
URL: http://llvm.org/viewvc/llvm-project?rev=295935&view=rev
Log:
[CodeGen] Don't reemit expressions for pass_object_size params.
This fixes an assertion failure in cases where we had expression
statements that declared variables nested inside of pass_object_size
args. Since we were emitting the same ExprStmt twice (once for the arg,
once for the @llvm.objectsize call), we were getting issues with
redefining locals.
This also means that we can be more lax about when we emit
@llvm.objectsize for pass_object_size args: since we're reusing the
arg's value itself, we don't have to care so much about side-effects.
Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGen/pass-object-size.c
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=295935&r1=295934&r2=295935&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Feb 22 23:59:56 2017
@@ -420,10 +420,11 @@ getDefaultBuiltinObjectSizeResult(unsign
llvm::Value *
CodeGenFunction::evaluateOrEmitBuiltinObjectSize(const Expr *E, unsigned Type,
- llvm::IntegerType *ResType) {
+ llvm::IntegerType *ResType,
+ llvm::Value *EmittedE) {
uint64_t ObjectSize;
if (!E->tryEvaluateObjectSize(ObjectSize, getContext(), Type))
- return emitBuiltinObjectSize(E, Type, ResType);
+ return emitBuiltinObjectSize(E, Type, ResType, EmittedE);
return ConstantInt::get(ResType, ObjectSize, /*isSigned=*/true);
}
@@ -432,9 +433,14 @@ CodeGenFunction::evaluateOrEmitBuiltinOb
/// - A llvm::Argument (if E is a param with the pass_object_size attribute on
/// it)
/// - A call to the @llvm.objectsize intrinsic
+///
+/// EmittedE is the result of emitting `E` as a scalar expr. If it's non-null
+/// and we wouldn't otherwise try to reference a pass_object_size parameter,
+/// we'll call @llvm.objectsize on EmittedE, rather than emitting E.
llvm::Value *
CodeGenFunction::emitBuiltinObjectSize(const Expr *E, unsigned Type,
- llvm::IntegerType *ResType) {
+ llvm::IntegerType *ResType,
+ llvm::Value *EmittedE) {
// We need to reference an argument if the pointer is a parameter with the
// pass_object_size attribute.
if (auto *D = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts())) {
@@ -457,10 +463,10 @@ CodeGenFunction::emitBuiltinObjectSize(c
// LLVM can't handle Type=3 appropriately, and __builtin_object_size shouldn't
// evaluate E for side-effects. In either case, we shouldn't lower to
// @llvm.objectsize.
- if (Type == 3 || E->HasSideEffects(getContext()))
+ if (Type == 3 || (!EmittedE && E->HasSideEffects(getContext())))
return getDefaultBuiltinObjectSizeResult(Type, ResType);
- Value *Ptr = EmitScalarExpr(E);
+ Value *Ptr = EmittedE ? EmittedE : EmitScalarExpr(E);
assert(Ptr->getType()->isPointerTy() &&
"Non-pointer passed to __builtin_object_size?");
@@ -965,7 +971,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(
// We pass this builtin onto the optimizer so that it can figure out the
// object size in more complex cases.
- return RValue::get(emitBuiltinObjectSize(E->getArg(0), Type, ResType));
+ return RValue::get(emitBuiltinObjectSize(E->getArg(0), Type, ResType,
+ /*EmittedE=*/nullptr));
}
case Builtin::BI__builtin_prefetch: {
Value *Locality, *RW, *Address = EmitScalarExpr(E->getArg(0));
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=295935&r1=295934&r2=295935&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Feb 22 23:59:56 2017
@@ -3243,7 +3243,18 @@ void CodeGenFunction::EmitCallArgs(
EvaluationOrder Order) {
assert((int)ArgTypes.size() == (ArgRange.end() - ArgRange.begin()));
- auto MaybeEmitImplicitObjectSize = [&](unsigned I, const Expr *Arg) {
+ // We *have* to evaluate arguments from right to left in the MS C++ ABI,
+ // because arguments are destroyed left to right in the callee. As a special
+ // case, there are certain language constructs that require left-to-right
+ // evaluation, and in those cases we consider the evaluation order requirement
+ // to trump the "destruction order is reverse construction order" guarantee.
+ bool LeftToRight =
+ CGM.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()
+ ? Order == EvaluationOrder::ForceLeftToRight
+ : Order != EvaluationOrder::ForceRightToLeft;
+
+ auto MaybeEmitImplicitObjectSize = [&](unsigned I, const Expr *Arg,
+ RValue EmittedArg) {
if (CalleeDecl == nullptr || I >= CalleeDecl->getNumParams())
return;
auto *PS = CalleeDecl->getParamDecl(I)->getAttr<PassObjectSizeAttr>();
@@ -3253,20 +3264,16 @@ void CodeGenFunction::EmitCallArgs(
const auto &Context = getContext();
auto SizeTy = Context.getSizeType();
auto T = Builder.getIntNTy(Context.getTypeSize(SizeTy));
- llvm::Value *V = evaluateOrEmitBuiltinObjectSize(Arg, PS->getType(), T);
+ assert(EmittedArg.getScalarVal() && "We emitted nothing for the arg?");
+ llvm::Value *V = evaluateOrEmitBuiltinObjectSize(Arg, PS->getType(), T,
+ EmittedArg.getScalarVal());
Args.add(RValue::get(V), SizeTy);
+ // If we're emitting args in reverse, be sure to do so with
+ // pass_object_size, as well.
+ if (!LeftToRight)
+ std::swap(Args.back(), *(&Args.back() - 1));
};
- // We *have* to evaluate arguments from right to left in the MS C++ ABI,
- // because arguments are destroyed left to right in the callee. As a special
- // case, there are certain language constructs that require left-to-right
- // evaluation, and in those cases we consider the evaluation order requirement
- // to trump the "destruction order is reverse construction order" guarantee.
- bool LeftToRight =
- CGM.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()
- ? Order == EvaluationOrder::ForceLeftToRight
- : Order != EvaluationOrder::ForceRightToLeft;
-
// Insert a stack save if we're going to need any inalloca args.
bool HasInAllocaArgs = false;
if (CGM.getTarget().getCXXABI().isMicrosoft()) {
@@ -3284,11 +3291,20 @@ void CodeGenFunction::EmitCallArgs(
for (unsigned I = 0, E = ArgTypes.size(); I != E; ++I) {
unsigned Idx = LeftToRight ? I : E - I - 1;
CallExpr::const_arg_iterator Arg = ArgRange.begin() + Idx;
- if (!LeftToRight) MaybeEmitImplicitObjectSize(Idx, *Arg);
+ unsigned InitialArgSize = Args.size();
EmitCallArg(Args, *Arg, ArgTypes[Idx]);
- EmitNonNullArgCheck(Args.back().RV, ArgTypes[Idx], (*Arg)->getExprLoc(),
- CalleeDecl, ParamsToSkip + Idx);
- if (LeftToRight) MaybeEmitImplicitObjectSize(Idx, *Arg);
+ // In particular, we depend on it being the last arg in Args, and the
+ // objectsize bits depend on there only being one arg if !LeftToRight.
+ assert(InitialArgSize + 1 == Args.size() &&
+ "The code below depends on only adding one arg per EmitCallArg");
+ (void)InitialArgSize;
+ RValue RVArg = Args.back().RV;
+ EmitNonNullArgCheck(RVArg, ArgTypes[Idx], (*Arg)->getExprLoc(), CalleeDecl,
+ ParamsToSkip + Idx);
+ // @llvm.objectsize should never have side-effects and shouldn't need
+ // destruction/cleanups, so we can safely "emit" it after its arg,
+ // regardless of right-to-leftness
+ MaybeEmitImplicitObjectSize(Idx, *Arg, RVArg);
}
if (!LeftToRight) {
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=295935&r1=295934&r2=295935&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Feb 22 23:59:56 2017
@@ -3510,14 +3510,18 @@ private:
/// \brief Attempts to statically evaluate the object size of E. If that
/// fails, emits code to figure the size of E out for us. This is
/// pass_object_size aware.
+ ///
+ /// If EmittedExpr is non-null, this will use that instead of re-emitting E.
llvm::Value *evaluateOrEmitBuiltinObjectSize(const Expr *E, unsigned Type,
- llvm::IntegerType *ResType);
+ llvm::IntegerType *ResType,
+ llvm::Value *EmittedE);
/// \brief Emits the size of E, as required by __builtin_object_size. This
/// function is aware of pass_object_size parameters, and will act accordingly
/// if E is a parameter with the pass_object_size attribute.
llvm::Value *emitBuiltinObjectSize(const Expr *E, unsigned Type,
- llvm::IntegerType *ResType);
+ llvm::IntegerType *ResType,
+ llvm::Value *EmittedE);
public:
#ifndef NDEBUG
Modified: cfe/trunk/test/CodeGen/pass-object-size.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pass-object-size.c?rev=295935&r1=295934&r2=295935&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/pass-object-size.c (original)
+++ cfe/trunk/test/CodeGen/pass-object-size.c Wed Feb 22 23:59:56 2017
@@ -343,16 +343,26 @@ void test12(void *const p __attribute__(
// CHECK-LABEL: define void @test13
void test13() {
- // Ensuring that we don't lower objectsize if the expression has side-effects
char c[10];
+ unsigned i = 0;
char *p = c;
// CHECK: @llvm.objectsize
ObjectSize0(p);
- // CHECK-NOT: @llvm.objectsize
- ObjectSize0(++p);
- ObjectSize0(p++);
+ // Allow side-effects, since they always need to happen anyway. Just make sure
+ // we don't perform them twice.
+ // CHECK: = add
+ // CHECK-NOT: = add
+ // CHECK: @llvm.objectsize
+ // CHECK: call i32 @ObjectSize0
+ ObjectSize0(p + ++i);
+
+ // CHECK: = add
+ // CHECK: @llvm.objectsize
+ // CHECK-NOT: = add
+ // CHECK: call i32 @ObjectSize0
+ ObjectSize0(p + i++);
}
// There was a bug where variadic functions with pass_object_size would cause
@@ -395,3 +405,16 @@ void test16(__attribute__((address_space
// CHECK: call void @pass_size_unsigned_as1
pass_size_unsigned_as1(I);
}
+
+// This used to cause assertion failures, since we'd try to emit the statement
+// expression (and definitions for `a`) twice.
+// CHECK-LABEL: define void @test17
+void test17(char *C) {
+ // Check for 65535 to see if we're emitting this pointer twice.
+ // CHECK: 65535
+ // CHECK-NOT: 65535
+ // CHECK: @llvm.objectsize.i64.p0i8(i8* [[PTR:%[^,]+]],
+ // CHECK-NOT: 65535
+ // CHECK: call i32 @ObjectSize0(i8* [[PTR]]
+ ObjectSize0(C + ({ int a = 65535; a; }));
+}
More information about the cfe-commits
mailing list