[clang] 6f20744 - Add support for ignored bitfield conditional codegen.

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 13 10:34:02 PDT 2022


Author: Erich Keane
Date: 2022-04-13T10:33:55-07:00
New Revision: 6f20744b7ff875bb5eb735d1e7636ad6a66d4526

URL: https://github.com/llvm/llvm-project/commit/6f20744b7ff875bb5eb735d1e7636ad6a66d4526
DIFF: https://github.com/llvm/llvm-project/commit/6f20744b7ff875bb5eb735d1e7636ad6a66d4526.diff

LOG: Add support for ignored bitfield conditional codegen.

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.

Differential Revision: https://reviews.llvm.org/D123680

Added: 
    clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp

Modified: 
    clang/lib/CodeGen/CGExpr.cpp
    clang/lib/CodeGen/CodeGenFunction.h

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 0c1b201037ff1..848c56dd61dd1 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -189,7 +189,17 @@ llvm::Value *CodeGenFunction::EvaluateExprAsBool(const Expr *E) {
 /// 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
+  // 
diff icult to codegen for, since creating a single "LValue" for two
+  // 
diff erent 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,139 @@ static Optional<LValue> EmitLValueOrThrowExpression(CodeGenFunction &CGF,
   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.
+template<typename FuncTy>
+ConditionalInfo EmitConditionalBlocks(CodeGenFunction &CGF,
+                                      const AbstractConditionalOperator *E,
+                                      const FuncTy &BranchGenFunc) {
+  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 = BranchGenFunc(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 = BranchGenFunc(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, [](CodeGenFunction &CGF, const Expr *E) {
+    CGF.EmitIgnoredExpr(E);
+    return LValue{};
+  });
+}
+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, [](CodeGenFunction &CGF, const Expr *E) {
+        return EmitLValueOrThrowExpression(CGF, E);
+      });
+
+  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;
   }
 }
 

diff  --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index bda447339d42d..3d3fabe60795b 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -3915,6 +3915,7 @@ class CodeGenFunction : public CodeGenTypeCache {
   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);

diff  --git a/clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp b/clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp
new file mode 100644
index 0000000000000..7700e97ae9d5c
--- /dev/null
+++ b/clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp
@@ -0,0 +1,147 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+struct S {
+  int field1 : 5;
+  int field2 : 6;
+  int field3 : 3;
+};
+
+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.
+  (void)(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]]:
+  // CHECK-NOT: phi
+  // There is nothing in the 'end' block associated with this, but it is the
+  // 'continuation' block for the rest of the function.
+
+}
+
+
+void use2(bool cond1, bool cond2, struct S s1, int val1, int val2, int val3) {
+  // CHECK: define {{.*}}use2{{.*}}(
+  // CHECK: %[[S1:.+]] = alloca %struct.S
+  // CHECK: %[[COND1:.+]] = alloca i8
+  // CHECK: %[[COND2:.+]] = alloca i8
+  // CHECK: %[[VAL1:.+]] = alloca i32
+  // CHECK: %[[VAL2:.+]] = alloca i32
+  // CHECK: %[[VAL3:.+]] = alloca i32
+
+  cond1 ? s1.field1 = val1 : cond2 ? s1.field2 = val2 : s1.field3 = val3;
+  // First Condition setup, branch.
+  // CHECK: %[[CONDLD:.+]] = load i8, ptr %[[COND1]]
+  // CHECK: %[[TO_BOOL:.+]] = trunc i8 %[[CONDLD]] to i1
+  // CHECK: br i1 %[[TO_BOOL]], label %[[TRUE:.+]], label %[[FALSE:.+]]
+
+  // First 'True' branch, sets field1 to val1.
+  // 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:.+]]
+
+  // First 'False' branch, starts second ignored expression.
+  // CHECK: [[FALSE]]:
+  // CHECK: %[[CONDLD:.+]] = load i8, ptr %[[COND2]]
+  // CHECK: %[[TO_BOOL:.+]] = trunc i8 %[[CONDLD]] to i1
+  // CHECK: br i1 %[[TO_BOOL]], label %[[TRUE2:.+]], label %[[FALSE2:.+]]
+
+  // Second 'True' branch, sets field2 to val2.
+  // CHECK: [[TRUE2]]:
+  // 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:.+]]
+
+  // Second 'False' branch, sets field3 to val3.
+  // CHECK: [[FALSE2]]:
+  // CHECK: %[[VAL3LD:.+]] = load i32, ptr %[[VAL3]]
+  // CHECK: %[[VAL3TRUNC:.+]] = trunc i32 %[[VAL3LD]] to i16
+  // CHECK: %[[BF_LOAD:.+]] = load i16, ptr %[[S1]]
+  // CHECK: %[[BF_VAL:.+]] = and i16 %[[VAL3TRUNC]], 7
+  // CHECK: %[[BF_SHIFT:.+]] = shl i16 %[[BF_VAL]], 11
+  // CHECK: %[[BF_CLEAR:.+]] = and i16 %[[BF_LOAD]], -14337
+  // CHECK: %[[BF_SET:.+]] = or i16 %[[BF_CLEAR]], %[[BF_SHIFT]]
+  // CHECK: store i16 %[[BF_SET]], ptr %[[S1]]
+  // CHECK: br label %[[END:.+]]
+
+  // CHECK[[END]]:
+  // CHECK-NOT: phi
+  // Nothing left to do here.
+}


        


More information about the cfe-commits mailing list