erichkeane created this revision.
erichkeane added reviewers: eli.friedman, aaron.ballman, tahonermann.
Herald added a project: All.
erichkeane requested review of this revision.
Currently we emit an error in just about every case of conditionals
with a 'non simple' branch if treated as an LValue. This patch adds
support for the special case where this is an 'ignored' lvalue, which
permits the side effects from happening.
It also splits up the emit for conditional LValue in a way that should
be usable to handle simple assignment expressions in similar situations.
https://reviews.llvm.org/D123680
Files:
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp
Index: clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+struct S {
+ int field1 : 5;
+ int field2 : 6;
+};
+
+void use(bool cond, struct S s1, struct S s2, int val1, int val2) {
+ // CHECK: define {{.*}}use{{.*}}(
+ // CHECK: %[[S1:.+]] = alloca %struct.S
+ // CHECK: %[[S2:.+]] = alloca %struct.S
+ // CHECK: %[[COND:.+]] = alloca i8
+ // CHECK: %[[VAL1:.+]] = alloca i32
+ // CHECK: %[[VAL2:.+]] = alloca i32
+
+ cond ? s1.field1 = val1 : s1.field2 = val2;
+ // Condition setup, branch.
+ // CHECK: %[[CONDLD:.+]] = load i8, ptr %[[COND]]
+ // CHECK: %[[TO_BOOL:.+]] = trunc i8 %[[CONDLD]] to i1
+ // CHECK: br i1 %[[TO_BOOL]], label %[[TRUE:.+]], label %[[FALSE:.+]]
+
+ // 'True', branch set the BF, branch to 'end'.
+ // CHECK: [[TRUE]]:
+ // CHECK: %[[VAL1LD:.+]] = load i32, ptr %[[VAL1]]
+ // CHECK: %[[VAL1TRUNC:.+]] = trunc i32 %[[VAL1LD]] to i16
+ // CHECK: %[[BF_LOAD:.+]] = load i16, ptr %[[S1]]
+ // CHECK: %[[BF_VAL:.+]] = and i16 %[[VAL1TRUNC]], 31
+ // CHECK: %[[BF_CLEAR:.+]] = and i16 %[[BF_LOAD]], -32
+ // CHECK: %[[BF_SET:.+]] = or i16 %[[BF_CLEAR]], %[[BF_VAL]]
+ // CHECK: store i16 %[[BF_SET]], ptr %[[S1]]
+ // CHECK: br label %[[END:.+]]
+
+ // 'False', branch set the OTHER BF, branch to 'end'.
+ // CHECK: [[FALSE]]:
+ // CHECK: %[[VAL2LD:.+]] = load i32, ptr %[[VAL2]]
+ // CHECK: %[[VAL2TRUNC:.+]] = trunc i32 %[[VAL2LD]] to i16
+ // CHECK: %[[BF_LOAD:.+]] = load i16, ptr %[[S1]]
+ // CHECK: %[[BF_VAL:.+]] = and i16 %[[VAL2TRUNC]], 63
+ // CHECK: %[[BF_SHIFT:.+]] = shl i16 %[[BF_VAL]], 5
+ // CHECK: %[[BF_CLEAR:.+]] = and i16 %[[BF_LOAD]], -2017
+ // CHECK: %[[BF_SET:.+]] = or i16 %[[BF_CLEAR]], %[[BF_SHIFT]]
+ // CHECK: store i16 %[[BF_SET]], ptr %[[S1]]
+ // CHECK: br label %[[END:.+]]
+
+ // CHECK: [[END]]:
+ // There is nothing in the 'end' block associated with this, but it is the
+ // 'continuation' block for the rest of the function.
+
+ // Same test, has a no-op cast and parens.
+ (int)(cond ? s2.field1 = val1 : s2.field2 = val2);
+ // Condition setup, branch.
+ // CHECK: %[[CONDLD:.+]] = load i8, ptr %[[COND]]
+ // CHECK: %[[TO_BOOL:.+]] = trunc i8 %[[CONDLD]] to i1
+ // CHECK: br i1 %[[TO_BOOL]], label %[[TRUE:.+]], label %[[FALSE:.+]]
+
+ // 'True', branch set the BF, branch to 'end'.
+ // CHECK: [[TRUE]]:
+ // CHECK: %[[VAL1LD:.+]] = load i32, ptr %[[VAL1]]
+ // CHECK: %[[VAL1TRUNC:.+]] = trunc i32 %[[VAL1LD]] to i16
+ // CHECK: %[[BF_LOAD:.+]] = load i16, ptr %[[S2]]
+ // CHECK: %[[BF_VAL:.+]] = and i16 %[[VAL1TRUNC]], 31
+ // CHECK: %[[BF_CLEAR:.+]] = and i16 %[[BF_LOAD]], -32
+ // CHECK: %[[BF_SET:.+]] = or i16 %[[BF_CLEAR]], %[[BF_VAL]]
+ // CHECK: store i16 %[[BF_SET]], ptr %[[S2]]
+ // CHECK: br label %[[END:.+]]
+
+ // 'False', branch set the OTHER BF, branch to 'end'.
+ // CHECK: [[FALSE]]:
+ // CHECK: %[[VAL2LD:.+]] = load i32, ptr %[[VAL2]]
+ // CHECK: %[[VAL2TRUNC:.+]] = trunc i32 %[[VAL2LD]] to i16
+ // CHECK: %[[BF_LOAD:.+]] = load i16, ptr %[[S2]]
+ // CHECK: %[[BF_VAL:.+]] = and i16 %[[VAL2TRUNC]], 63
+ // CHECK: %[[BF_SHIFT:.+]] = shl i16 %[[BF_VAL]], 5
+ // CHECK: %[[BF_CLEAR:.+]] = and i16 %[[BF_LOAD]], -2017
+ // CHECK: %[[BF_SET:.+]] = or i16 %[[BF_CLEAR]], %[[BF_SHIFT]]
+ // CHECK: store i16 %[[BF_SET]], ptr %[[S2]]
+ // CHECK: br label %[[END:.+]]
+
+ // CHECK: [[END]]:
+ // There is nothing in the 'end' block associated with this, but it is the
+ // 'continuation' block for the rest of the function.
+
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3915,6 +3915,7 @@
LValue EmitObjCIsaExpr(const ObjCIsaExpr *E);
LValue EmitCompoundLiteralLValue(const CompoundLiteralExpr *E);
LValue EmitInitListLValue(const InitListExpr *E);
+ void EmitIgnoredConditionalOperator(const AbstractConditionalOperator *E);
LValue EmitConditionalOperatorLValue(const AbstractConditionalOperator *E);
LValue EmitCastLValue(const CastExpr *E);
LValue EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -189,7 +189,17 @@
/// ignoring the result.
void CodeGenFunction::EmitIgnoredExpr(const Expr *E) {
if (E->isPRValue())
- return (void) EmitAnyExpr(E, AggValueSlot::ignored(), true);
+ return (void)EmitAnyExpr(E, AggValueSlot::ignored(), true);
+
+ // if this is a bitfield-resulting conditional operator, we can special case
+ // emit this. The normal 'EmitLValue' version of this is particularly
+ // difficult to codegen for, since creating a single "LValue" for two
+ // different sized arguments here is not particularly doable.
+ if (const auto *CondOp = dyn_cast<AbstractConditionalOperator>(
+ E->IgnoreParenNoopCasts(getContext()))) {
+ if (CondOp->getObjectKind() == OK_BitField)
+ return EmitIgnoredConditionalOperator(CondOp);
+ }
// Just emit it as an l-value and drop the result.
EmitLValue(E);
@@ -4570,94 +4580,131 @@
return CGF.EmitLValue(Operand);
}
-LValue CodeGenFunction::
-EmitConditionalOperatorLValue(const AbstractConditionalOperator *expr) {
- if (!expr->isGLValue()) {
- // ?: here should be an aggregate.
- assert(hasAggregateEvaluationKind(expr->getType()) &&
- "Unexpected conditional operator!");
- return EmitAggExprToLValue(expr);
- }
-
- OpaqueValueMapping binding(*this, expr);
-
- const Expr *condExpr = expr->getCond();
+namespace {
+// Handle the case where the condition is a constant evaluatable simple integer,
+// which means we don't have to separately handle the true/false blocks.
+llvm::Optional<LValue> HandleConditionalOperatorLValueSimpleCase(
+ CodeGenFunction &CGF, const AbstractConditionalOperator *E) {
+ const Expr *condExpr = E->getCond();
bool CondExprBool;
- if (ConstantFoldsToSimpleInteger(condExpr, CondExprBool)) {
- const Expr *live = expr->getTrueExpr(), *dead = expr->getFalseExpr();
- if (!CondExprBool) std::swap(live, dead);
+ if (CGF.ConstantFoldsToSimpleInteger(condExpr, CondExprBool)) {
+ const Expr *Live = E->getTrueExpr(), *Dead = E->getFalseExpr();
+ if (!CondExprBool)
+ std::swap(Live, Dead);
- if (!ContainsLabel(dead)) {
+ if (!CGF.ContainsLabel(Dead)) {
// If the true case is live, we need to track its region.
if (CondExprBool)
- incrementProfileCounter(expr);
+ CGF.incrementProfileCounter(E);
// If a throw expression we emit it and return an undefined lvalue
// because it can't be used.
- if (auto *ThrowExpr = dyn_cast<CXXThrowExpr>(live->IgnoreParens())) {
- EmitCXXThrowExpr(ThrowExpr);
- llvm::Type *ElemTy = ConvertType(dead->getType());
+ if (auto *ThrowExpr = dyn_cast<CXXThrowExpr>(Live->IgnoreParens())) {
+ CGF.EmitCXXThrowExpr(ThrowExpr);
+ llvm::Type *ElemTy = CGF.ConvertType(Dead->getType());
llvm::Type *Ty = llvm::PointerType::getUnqual(ElemTy);
- return MakeAddrLValue(
+ return CGF.MakeAddrLValue(
Address(llvm::UndefValue::get(Ty), ElemTy, CharUnits::One()),
- dead->getType());
+ Dead->getType());
}
- return EmitLValue(live);
+ return CGF.EmitLValue(Live);
}
}
+ return llvm::None;
+}
+struct ConditionalInfo {
+ llvm::BasicBlock *lhsBlock, *rhsBlock;
+ Optional<LValue> LHS, RHS;
+};
- llvm::BasicBlock *lhsBlock = createBasicBlock("cond.true");
- llvm::BasicBlock *rhsBlock = createBasicBlock("cond.false");
- llvm::BasicBlock *contBlock = createBasicBlock("cond.end");
+// Create and generate the 3 blocks for a conditional operator.
+// Leaves the 'current block' in the continuation basic block.
+ConditionalInfo EmitConditionalBlocks(CodeGenFunction &CGF,
+ const AbstractConditionalOperator *E) {
+ ConditionalInfo Info{CGF.createBasicBlock("cond.true"),
+ CGF.createBasicBlock("cond.false")};
+ llvm::BasicBlock *endBlock = CGF.createBasicBlock("cond.end");
- ConditionalEvaluation eval(*this);
- EmitBranchOnBoolExpr(condExpr, lhsBlock, rhsBlock, getProfileCount(expr));
+ CodeGenFunction::ConditionalEvaluation eval(CGF);
+ CGF.EmitBranchOnBoolExpr(E->getCond(), Info.lhsBlock, Info.rhsBlock,
+ CGF.getProfileCount(E));
// Any temporaries created here are conditional.
- EmitBlock(lhsBlock);
- incrementProfileCounter(expr);
- eval.begin(*this);
- Optional<LValue> lhs =
- EmitLValueOrThrowExpression(*this, expr->getTrueExpr());
- eval.end(*this);
-
- if (lhs && !lhs->isSimple())
- return EmitUnsupportedLValue(expr, "conditional operator");
+ CGF.EmitBlock(Info.lhsBlock);
+ CGF.incrementProfileCounter(E);
+ eval.begin(CGF);
+ Info.LHS = EmitLValueOrThrowExpression(CGF, E->getTrueExpr());
+ eval.end(CGF);
+ Info.lhsBlock = CGF.Builder.GetInsertBlock();
- lhsBlock = Builder.GetInsertBlock();
- if (lhs)
- Builder.CreateBr(contBlock);
+ if (Info.LHS)
+ CGF.Builder.CreateBr(endBlock);
// Any temporaries created here are conditional.
- EmitBlock(rhsBlock);
- eval.begin(*this);
- Optional<LValue> rhs =
- EmitLValueOrThrowExpression(*this, expr->getFalseExpr());
- eval.end(*this);
- if (rhs && !rhs->isSimple())
- return EmitUnsupportedLValue(expr, "conditional operator");
- rhsBlock = Builder.GetInsertBlock();
+ CGF.EmitBlock(Info.rhsBlock);
+ eval.begin(CGF);
+ Info.RHS = EmitLValueOrThrowExpression(CGF, E->getFalseExpr());
+ eval.end(CGF);
+ Info.rhsBlock = CGF.Builder.GetInsertBlock();
+ CGF.EmitBlock(endBlock);
+
+ return Info;
+}
+} // namespace
- EmitBlock(contBlock);
+void CodeGenFunction::EmitIgnoredConditionalOperator(
+ const AbstractConditionalOperator *E) {
+ if (!E->isGLValue()) {
+ // ?: here should be an aggregate.
+ assert(hasAggregateEvaluationKind(E->getType()) &&
+ "Unexpected conditional operator!");
+ return (void)EmitAggExprToLValue(E);
+ }
+
+ OpaqueValueMapping binding(*this, E);
+ if (HandleConditionalOperatorLValueSimpleCase(*this, E))
+ return;
+
+ EmitConditionalBlocks(*this, E);
+}
+LValue CodeGenFunction::EmitConditionalOperatorLValue(
+ const AbstractConditionalOperator *expr) {
+ if (!expr->isGLValue()) {
+ // ?: here should be an aggregate.
+ assert(hasAggregateEvaluationKind(expr->getType()) &&
+ "Unexpected conditional operator!");
+ return EmitAggExprToLValue(expr);
+ }
+
+ OpaqueValueMapping binding(*this, expr);
+ if (llvm::Optional<LValue> Res =
+ HandleConditionalOperatorLValueSimpleCase(*this, expr))
+ return *Res;
+
+ ConditionalInfo Info = EmitConditionalBlocks(*this, expr);
+
+ if ((Info.LHS && !Info.LHS->isSimple()) ||
+ (Info.RHS && !Info.RHS->isSimple()))
+ return EmitUnsupportedLValue(expr, "conditional operator");
- if (lhs && rhs) {
- Address lhsAddr = lhs->getAddress(*this);
- Address rhsAddr = rhs->getAddress(*this);
+ if (Info.LHS && Info.RHS) {
+ Address lhsAddr = Info.LHS->getAddress(*this);
+ Address rhsAddr = Info.RHS->getAddress(*this);
llvm::PHINode *phi = Builder.CreatePHI(lhsAddr.getType(), 2, "cond-lvalue");
- phi->addIncoming(lhsAddr.getPointer(), lhsBlock);
- phi->addIncoming(rhsAddr.getPointer(), rhsBlock);
+ phi->addIncoming(lhsAddr.getPointer(), Info.lhsBlock);
+ phi->addIncoming(rhsAddr.getPointer(), Info.rhsBlock);
Address result(phi, lhsAddr.getElementType(),
std::min(lhsAddr.getAlignment(), rhsAddr.getAlignment()));
AlignmentSource alignSource =
- std::max(lhs->getBaseInfo().getAlignmentSource(),
- rhs->getBaseInfo().getAlignmentSource());
+ std::max(Info.LHS->getBaseInfo().getAlignmentSource(),
+ Info.RHS->getBaseInfo().getAlignmentSource());
TBAAAccessInfo TBAAInfo = CGM.mergeTBAAInfoForConditionalOperator(
- lhs->getTBAAInfo(), rhs->getTBAAInfo());
+ Info.LHS->getTBAAInfo(), Info.RHS->getTBAAInfo());
return MakeAddrLValue(result, expr->getType(), LValueBaseInfo(alignSource),
TBAAInfo);
} else {
- assert((lhs || rhs) &&
+ assert((Info.LHS || Info.RHS) &&
"both operands of glvalue conditional are throw-expressions?");
- return lhs ? *lhs : *rhs;
+ return Info.LHS ? *Info.LHS : *Info.RHS;
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
