[clang] [CIR] Fix bitfield post-increment return value (PR #201723)

via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 4 17:10:55 PDT 2026


llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangir

Author: adams381

<details>
<summary>Changes</summary>

In \`emitScalarPrePostIncDec\`, the bitfield branch called \`emitStoreThroughBitfieldLValue\` and returned its result directly, bypassing the \`return e->isPrefix() ? value : input\` logic that selects old vs new for post vs pre-increment.  For post-increment on a bitfield, this returned the new (incremented) value instead of the old one, so \`if (s->nRefs++)\` always evaluated true even when \`nRefs\` was zero on entry.

Fix by assigning the store result to \`value\` and falling through to the existing prefix/postfix selector, mirroring the non-bitfield path.

Found via the SPEC CPU 2026 abc_r benchmark: \`Cnf_ManScanMapping_rec\` uses \`if (pObj->nRefs++)\` on a 26-bit bitfield to detect already-visited AIG nodes.  With the bug, every call returned true (already visited) immediately, producing an empty CNF mapping and a spuriously satisfiable SAT problem (133 variables instead of 3018).

---
Full diff: https://github.com/llvm/llvm-project/pull/201723.diff


4 Files Affected:

- (modified) clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp (+1-1) 
- (modified) clang/lib/CIR/CodeGen/CIRGenFunction.cpp (+12-3) 
- (added) clang/test/CIR/CodeGen/bitfield-postinc.cpp (+66) 
- (added) clang/test/CIR/CodeGen/member-pointer-null-init.cpp (+47) 


``````````diff
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 47dedd4e4cf0c..eae82369cbf1b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -725,7 +725,7 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
 
     // Store the updated result through the lvalue
     if (lv.isBitField())
-      return cgf.emitStoreThroughBitfieldLValue(RValue::get(value), lv);
+      value = cgf.emitStoreThroughBitfieldLValue(RValue::get(value), lv);
     else
       cgf.emitStoreThroughLValue(RValue::get(value), lv);
 
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 4ecb47a864146..f16a5e6eb1574 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -1302,10 +1302,19 @@ void CIRGenFunction::emitNullInitialization(mlir::Location loc, Address destPtr,
 
   // If the type contains a pointer to data member we can't memset it to zero.
   // Instead, create a null constant and copy it to the destination.
-  // TODO: there are other patterns besides zero that we can usefully memset,
-  // like -1, which happens to be the pattern used by member-pointers.
+  // Member pointers use -1 as the null value, so a plain zero store would be
+  // incorrect; emitNullConstant produces the right per-field pattern.
   if (!cgm.getTypes().isZeroInitializable(ty)) {
-    cgm.errorNYI(loc, "type is not zero initializable");
+    // emitNullConstant does not yet handle types with virtual bases.
+    if (const auto *rd = ty->getAsCXXRecordDecl(); rd && rd->getNumVBases()) {
+      cgm.errorNYI(loc,
+                   "emitNullInitialization: non-zero-init type with virtual "
+                   "bases");
+      return;
+    }
+    mlir::Value nullVal = cgm.emitNullConstant(ty, loc);
+    builder.createStore(loc, nullVal, destPtr);
+    return;
   }
 
   // In LLVM Codegen: otherwise, just memset the whole thing to zero using
diff --git a/clang/test/CIR/CodeGen/bitfield-postinc.cpp b/clang/test/CIR/CodeGen/bitfield-postinc.cpp
new file mode 100644
index 0000000000000..f9aea864f2119
--- /dev/null
+++ b/clang/test/CIR/CodeGen/bitfield-postinc.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll %s --check-prefix=LLVM-CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+// Post-increment on a bitfield: `s->nRefs++` should return the PRE-increment
+// value (old value before the increment).
+// Pre-increment on a bitfield: `++s->nRefs` should return the POST-increment
+// value (new value after the increment).
+
+struct S { unsigned type : 6; unsigned nRefs : 26; };
+
+// Post-increment: returns OLD value, stores OLD+1.
+int postinc(S *s) {
+  return s->nRefs++;
+}
+
+// Pre-increment: returns NEW value, stores OLD+1.
+int preinc(S *s) {
+  return ++s->nRefs;
+}
+
+// CIR-LABEL: @_Z7postincP1S
+// CIR:         %[[OLD:.*]] = cir.get_bitfield
+// CIR:         %[[INC:.*]] = cir.inc %[[OLD]]
+// CIR:         cir.set_bitfield {{.*}} %[[INC]]
+// CIR:         cir.cast integral %[[OLD]]
+// CIR-NOT:     cir.cast integral %[[INC]]
+
+// CIR-LABEL: @_Z6preincP1S
+// CIR:         %[[OLD2:.*]] = cir.get_bitfield
+// CIR:         %[[INC2:.*]] = cir.inc %[[OLD2]]
+// CIR:         %[[STORED:.*]] = cir.set_bitfield {{.*}} %[[INC2]]
+// CIR:         cir.cast integral %[[STORED]]
+
+// Postinc: old nRefs (lshr result, before add) goes into retval slot.
+// LLVM-CIR-LABEL: @_Z7postincP1S
+// LLVM-CIR:         %[[WORD:.*]] = load i32
+// LLVM-CIR:         %[[OLD:.*]] = lshr i32 %[[WORD]], 6
+// LLVM-CIR:         add i32 %[[OLD]], 1
+// LLVM-CIR:         store i32 %[[OLD]], ptr
+// LLVM-CIR:         ret i32
+
+// Preinc: new nRefs (after add+mask) goes into retval slot.
+// LLVM-CIR-LABEL: @_Z6preincP1S
+// LLVM-CIR:         %[[WORD2:.*]] = load i32
+// LLVM-CIR:         %[[OLD2:.*]] = lshr i32 %[[WORD2]], 6
+// LLVM-CIR:         %[[NEW2:.*]] = add i32 %[[OLD2]], 1
+// LLVM-CIR:         %[[MASKED:.*]] = and i32 %[[NEW2]], 67108863
+// LLVM-CIR:         store i32 %[[MASKED]], ptr {{.*}}, align 4
+// LLVM-CIR:         ret i32
+
+// OGCG-LABEL: @_Z7postincP1S
+// OGCG:         %[[LOAD:.*]] = load i32
+// OGCG:         %[[LSHR:.*]] = lshr i32 %[[LOAD]], 6
+// OGCG:         %[[INC:.*]] = add i32 %[[LSHR]], 1
+// OGCG:         ret i32 %[[LSHR]]
+
+// OGCG-LABEL: @_Z6preincP1S
+// OGCG:         %[[LOAD2:.*]] = load i32
+// OGCG:         %[[LSHR2:.*]] = lshr i32 %[[LOAD2]], 6
+// OGCG:         %[[INC2:.*]] = add i32 %[[LSHR2]], 1
+// OGCG:         %[[MASKED2:.*]] = and i32 %[[INC2]], 67108863
+// OGCG:         ret i32 %[[MASKED2]]
diff --git a/clang/test/CIR/CodeGen/member-pointer-null-init.cpp b/clang/test/CIR/CodeGen/member-pointer-null-init.cpp
new file mode 100644
index 0000000000000..74ee344c6f407
--- /dev/null
+++ b/clang/test/CIR/CodeGen/member-pointer-null-init.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++17 -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++17 -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --check-prefix=LLVM,LLVMCIR --input-file=%t-cir.ll %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++17 -emit-llvm %s -o %t.ll
+// RUN: FileCheck --check-prefix=LLVM,OGCG --input-file=%t.ll %s
+
+struct Inner {
+  int Inner::*p;
+};
+
+struct Outer {
+  Inner a;
+  int b;
+};
+
+// Value-init of a heap-allocated struct containing a pointer-to-data-member.
+// The member pointer is null (-1), so the stored constant must carry -1.
+
+// CIR-LABEL: cir.func {{.*}}@_Z8make_newv
+// CIR:         [[NULL:%.*]] = cir.const #cir.const_record<{#cir.int<-1> : !s64i}> : !rec_Inner
+// CIR:         cir.store align(8) [[NULL]], {{%.*}} : !rec_Inner, !cir.ptr<!rec_Inner>
+
+// LLVMCIR-LABEL: define {{.*}} ptr @_Z8make_newv
+// LLVMCIR:         call {{.*}} @_Znwm
+// LLVMCIR:         store %struct.Inner { i64 -1 }, ptr %{{.*}}, align 8
+
+// OGCG: @{{.*}} = private constant %struct.Inner { i64 -1 }
+// OGCG-LABEL: define {{.*}} ptr @_Z8make_newv
+// OGCG:         call {{.*}} @llvm.memcpy{{.*}}i64 8
+
+Inner *make_new() { return new Inner(); }
+
+// Partial aggregate init: Inner subobject 'a' is value-initialized because
+// it has no designated initializer.
+
+// CIR-LABEL: cir.func {{.*}}@_Z11runtime_aggi
+// CIR:         cir.const #cir.int<-1> : !s64i
+// CIR:         cir.store align(8) {{%.*}}, {{%.*}} : !s64i
+
+// LLVM-LABEL: define {{.*}} void @_Z11runtime_aggi
+// LLVM:          store i64 -1, ptr %{{.*}}, align 8
+
+void runtime_agg(int x) {
+  Outer o = {.b = x};
+  (void)o;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/201723


More information about the cfe-commits mailing list