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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
