llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Adam Smith (adams381) <details> <summary>Changes</summary> Bool vectors are now sized one bit per element, as clang does, and are passed and returned the way classic CodeGen passes them, including in records. Assisted-by: Cursor / Claude Opus 5.5 --- Patch is 39.89 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/230280.diff 13 Files Affected: - (modified) clang/include/clang/CIR/Dialect/IR/CIRTypes.td (+11) - (modified) clang/lib/CIR/CodeGen/CIRGenCall.cpp (+12-3) - (modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+24) - (modified) clang/lib/CIR/Dialect/IR/CIRTypes.cpp (+12) - (modified) clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp (+12-9) - (modified) clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp (+74-52) - (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+4-4) - (modified) clang/lib/CIR/Lowering/LoweringHelpers.cpp (+2-3) - (added) clang/test/CIR/CodeGen/bool-vector-record-storage-nyi.c (+40) - (added) clang/test/CIR/CodeGen/call-conv-lowering-x86_64-bool-vector-return.cpp (+38) - (added) clang/test/CIR/CodeGen/call-conv-lowering-x86_64-bool-vector.c (+246) - (modified) clang/test/CIR/CodeGenBuiltins/builtins-elementwise-bool-nyi.c (+1-3) - (modified) clang/test/CIR/Transforms/abi-lowering/x86_64-vector.cir (+28) ``````````diff diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td index fd092c3ebb7056..2b76d1cb6928db 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td @@ -509,6 +509,11 @@ def CIR_VectorType : CIR_Type<"Vector", "vector", [ !cir.vector<2 x !cir.float> ``` + A fixed-length vector of `!cir.bool` is packed one bit per element, and its + size is rounded up to a power of two of at least one byte, so + `!cir.vector<4 x !cir.bool>` occupies one byte and + `!cir.vector<17 x !cir.bool>` occupies four. + Scalable vectors are indicated by enclosing size in square brackets. Example: @@ -532,6 +537,12 @@ def CIR_VectorType : CIR_Type<"Vector", "vector", [ }]>, ]; + let extraClassDeclaration = [{ + /// The width of the integer a vector of `!cir.bool` is stored as: one bit + /// per element, at least a byte. + uint64_t getBoolStorageWidth() const; + }]; + let extraClassDefinition = [{ bool $cppClass::isSized() const { return ::cir::isSized(getElementType()); diff --git a/clang/lib/CIR/CodeGen/CIRGenCall.cpp b/clang/lib/CIR/CodeGen/CIRGenCall.cpp index 4f4e10aa218be9..ea900b00413777 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCall.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCall.cpp @@ -606,10 +606,19 @@ static bool determineNoUndef(QualType clangTy, CIRGenTypes &types, // bits from the perspective of LLVM IR. return false; + // Classic CodeGen stores a packed bool vector as an integer with one bit per + // element, at least a byte wide, so the check above drops noundef when that + // integer is not a whole number of bytes. The CIR vector type is sized to a + // power of two of at least a byte, which always passes that check, so test + // the storage integer's width directly. + if (clangTy->isPackedVectorBoolType(types.getASTContext()) && + mlir::cast<cir::VectorType>(ty).getBoolStorageWidth() % 8) + return false; + assert(!cir::MissingFeatures::opCallCallConv()); - // TODO(cir): The calling convention code needs to figure if the - // coerced-to-type is larger than the actual type, and remove the noundef - // attribute. Classic compiler did it here. + // The coerced type is not known until CallConvLowering, which drops noundef + // when it is wider than the value's memory type, the check classic + // DetermineNoUndef makes at this point. if (clangTy->isBitIntType()) return true; if (clangTy->isReferenceType()) diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp index 4bb483fc57a962..1d324a23fcee1e 100644 --- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp @@ -249,9 +249,33 @@ struct CIRRecordLowering final { cirGenTypes.getCGModule().errorNYI(recordDecl->getSourceRange(), "getStorageType for bitfields"); } + if (hasBoolVectorStorageAlignMismatch(type)) + cirGenTypes.getCGModule().errorNYI( + fieldDecl->getSourceRange(), + "getStorageType for bool vector whose storage integer is aligned " + "differently from the vector"); return type; } + /// Whether \p type, or the innermost element type of an array \p type, is a + /// bool vector whose storage integer is aligned differently from the + /// vector. A bool vector is stored as an integer with one bit per element, + /// at least a byte wide. The vector is aligned to its own size, so an + /// integer with the same alignment also has the same allocation size. + /// Classic CodeGen lays a mismatched member out as that integer and fills + /// the difference with padding bytes, which this layout does not do yet. + bool hasBoolVectorStorageAlignMismatch(mlir::Type type) { + while (auto arrTy = mlir::dyn_cast<cir::ArrayType>(type)) + type = arrTy.getElementType(); + auto vecTy = mlir::dyn_cast<cir::VectorType>(type); + if (!vecTy || !mlir::isa<cir::BoolType>(vecTy.getElementType())) + return false; + auto storageTy = + mlir::IntegerType::get(type.getContext(), vecTy.getBoolStorageWidth()); + return dataLayout.layout.getTypeABIAlignment(storageTy) != + dataLayout.layout.getTypeABIAlignment(vecTy); + } + uint64_t getFieldBitOffset(const FieldDecl *fieldDecl) { return astRecordLayout.getFieldOffset(fieldDecl->getFieldIndex()); } diff --git a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp index fe3f41a226e00e..75677d05796e62 100644 --- a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp @@ -1494,10 +1494,22 @@ BitFieldType::getABIAlignment(const mlir::DataLayout &dataLayout, llvm::TypeSize cir::VectorType::getTypeSizeInBits( const ::mlir::DataLayout &dataLayout, ::mlir::DataLayoutEntryListRef params) const { + // Clang packs a fixed-length bool vector one bit per element and rounds the + // vector's size up to a power of two of at least a byte. + if (mlir::isa<cir::BoolType>(getElementType()) && !getIsScalable()) { + assert(!cir::MissingFeatures::hlsl()); + return llvm::TypeSize::getFixed(llvm::PowerOf2Ceil(getBoolStorageWidth())); + } return llvm::TypeSize::getFixed( getSize() * dataLayout.getTypeSizeInBits(getElementType())); } +uint64_t cir::VectorType::getBoolStorageWidth() const { + assert(mlir::isa<cir::BoolType>(getElementType()) && + "only a bool vector is stored as an integer"); + return std::max<uint64_t>(getSize(), 8); +} + uint64_t cir::VectorType::getABIAlignment(const ::mlir::DataLayout &dataLayout, ::mlir::DataLayoutEntryListRef params) const { diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 402cd09afd6370..c924529945c01e 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -205,11 +205,10 @@ static bool isSupportedType(mlir::Type ty, const DataLayout &dl) { // x86_64 has no calling convention for one. if (vecTy.getIsScalable()) return false; - // The classifier sizes a vector as element count times element width, so - // an element is only usable where that width is the one clang gives it. - // It is not for bool (a bit to clang, a byte here), for a _BitInt narrower - // than a byte (clang rounds to the storage container), or for x87 long - // double (80 bits here against clang's 128). + // mapCIRType maps a bool as a one-bit integer, so a bool vector is sized + // one bit per element, as clang packs it. Vectors of a _BitInt whose + // width is not a whole number of bytes, or of x87 long double, are not + // handled yet. mlir::Type elemTy = vecTy.getElementType(); if (auto elemInt = dyn_cast<cir::IntType>(elemTy)) { if (elemInt.getWidth() % 8) @@ -217,7 +216,7 @@ static bool isSupportedType(mlir::Type ty, const DataLayout &dl) { } else if (auto elemFp = dyn_cast<cir::FPTypeInterface>(elemTy)) { if (&elemFp.getFloatSemantics() == &llvm::APFloat::x87DoubleExtended()) return false; - } else { + } else if (!isa<cir::BoolType>(elemTy)) { return false; } // Clang also rounds the vector's own width up to a power of two, and the @@ -308,7 +307,10 @@ static mlir::Type abiTypeToCIR(const llvm::abi::Type *ty, MLIRContext *ctx) { return llvm::TypeSwitch<const llvm::abi::Type *, mlir::Type>(ty) .Case( [&](const llvm::abi::VoidType *) { return cir::VoidType::get(ctx); }) - .Case([&](const llvm::abi::IntegerType *intTy) { + .Case([&](const llvm::abi::IntegerType *intTy) -> mlir::Type { + // mapCIRType maps a bool as a one-bit integer. + if (intTy->isBool()) + return cir::BoolType::get(ctx); return cir::IntType::get(ctx, intTy->getSizeInBits().getFixedValue(), intTy->isSigned(), intTy->isBitInt()); }) @@ -373,8 +375,9 @@ static const llvm::abi::Type *mapCIRType(mlir::Type type, llvm::Align(dl.getTypeABIAlignment(type))); }) .Case([&](cir::BoolType) { - return tb.getIntegerType(dl.getTypeSizeInBits(type), - llvm::Align(dl.getTypeABIAlignment(type)), + // A bool is a one-bit integer to the classifier, as QualTypeMapper + // maps it, so a bool vector is sized one bit per element. + return tb.getIntegerType(1, llvm::Align(dl.getTypeABIAlignment(type)), /*Signed=*/false); }) .Case([&](cir::VoidType) { return tb.getVoidType(); }) diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp index f79a8535355a85..75eaec4d169866 100644 --- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp +++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp @@ -185,9 +185,36 @@ mlir::Value createIgnoredValue(mlir::OpBuilder &builder, mlir::Location loc, return cir::ConstantOp::create(builder, loc, ty, cir::PoisonAttr::get(ty)); } +/// Whether \p coercedTy is wider than the type a value of type \p origTy has +/// in memory, so the coerced value passes bits the value does not define and +/// cannot be noundef. This is the comparison classic CodeGen makes. A bool +/// vector's memory type is its storage integer, one bit per element and at +/// least a byte wide, not its power-of-two size. +static bool isCoercionWiderThanMemory(mlir::Type origTy, mlir::Type coercedTy, + const mlir::DataLayout &dl) { + uint64_t memoryBits = dl.getTypeSizeInBits(origTy).getFixedValue(); + if (auto vecTy = mlir::dyn_cast<cir::VectorType>(origTy); + vecTy && mlir::isa<cir::BoolType>(vecTy.getElementType())) + memoryBits = vecTy.getBoolStorageWidth(); + return dl.getTypeSizeInBits(coercedTy).getFixedValue() > memoryBits; +} + +/// Whether updateArgAttrs has anything to change: an arg that is Ignore (its +/// slot is dropped), Extend (needs llvm.signext / llvm.zeroext), Indirect +/// (needs the pointer attributes), Expand (changes the argument count), or a +/// coerced Direct, which may be flattened or may lose llvm.noundef. +static bool needsArgAttrUpdate(const FunctionClassification &fc) { + return llvm::any_of(fc.argInfos, [](const ArgClassification &ac) { + return ac.kind == ArgKind::Ignore || ac.kind == ArgKind::Extend || + ac.kind == ArgKind::Indirect || ac.kind == ArgKind::Expand || + (ac.kind == ArgKind::Direct && ac.coercedType); + }); +} + /// Build an updated arg_attrs ArrayAttr that drops Ignore'd args, adds -/// llvm.signext / llvm.zeroext on Extend args, and adds the pointer -/// attributes for Indirect args. Preserves any existing arg attributes on +/// llvm.signext / llvm.zeroext on Extend args, adds the pointer attributes +/// for Indirect args, and drops llvm.noundef from a coerced Direct arg that +/// passes undefined bits. Preserves any other existing arg attributes on /// retained arg slots. \p origArgTypes provides the pre-rewrite type for /// each arg slot. mlir::ArrayAttr updateArgAttrs(mlir::MLIRContext *ctx, @@ -255,6 +282,12 @@ mlir::ArrayAttr updateArgAttrs(mlir::MLIRContext *ctx, dl.getTypeSize(pointeeTy).getFixedValue())); } newArgAttrs.push_back(attrs.getDictionary(ctx)); + } else if (ac.kind == ArgKind::Direct && ac.coercedType && + isCoercionWiderThanMemory(origArgTypes[oldIdx], ac.coercedType, + dl)) { + mlir::NamedAttrList attrs(existing); + attrs.erase(mlir::LLVM::LLVMDialect::getNoUndefAttrName()); + newArgAttrs.push_back(attrs.getDictionary(ctx)); } else { newArgAttrs.push_back(existing); } @@ -264,22 +297,31 @@ mlir::ArrayAttr updateArgAttrs(mlir::MLIRContext *ctx, /// Build an updated res_attrs ArrayAttr (single entry, since CIR funcs have /// at most one result) that adds llvm.signext / llvm.zeroext on an Extend -/// return. Preserves any existing res attributes. +/// return and drops llvm.noundef from a coerced return that passes undefined +/// bits. Preserves any other existing res attributes. Returns +/// \p existingResAttrs unchanged, which may be null, when there is nothing to +/// add or drop. mlir::ArrayAttr updateResAttrs(mlir::MLIRContext *ctx, mlir::ArrayAttr existingResAttrs, - const ArgClassification &retInfo) { - if (retInfo.kind != ArgKind::Extend) + const ArgClassification &retInfo, + mlir::Type origRetTy, + const mlir::DataLayout &dl) { + bool needsNoUndefDrop = + retInfo.kind == ArgKind::Direct && retInfo.coercedType && + isCoercionWiderThanMemory(origRetTy, retInfo.coercedType, dl); + if (retInfo.kind != ArgKind::Extend && !needsNoUndefDrop) return existingResAttrs; - SmallVector<mlir::NamedAttribute> attrs; + mlir::NamedAttrList attrs; if (existingResAttrs && !existingResAttrs.empty()) - for (mlir::NamedAttribute na : - mlir::cast<mlir::DictionaryAttr>(existingResAttrs[0])) - attrs.push_back(na); - StringRef attrName = retInfo.signExtend ? "llvm.signext" : "llvm.zeroext"; - attrs.push_back(mlir::NamedAttribute(mlir::StringAttr::get(ctx, attrName), - mlir::UnitAttr::get(ctx))); - return mlir::ArrayAttr::get(ctx, {mlir::DictionaryAttr::get(ctx, attrs)}); + attrs.append( + mlir::cast<mlir::DictionaryAttr>(existingResAttrs[0]).getValue()); + if (retInfo.kind == ArgKind::Extend) + attrs.set(retInfo.signExtend ? "llvm.signext" : "llvm.zeroext", + mlir::UnitAttr::get(ctx)); + if (needsNoUndefDrop) + attrs.erase(mlir::LLVM::LLVMDialect::getNoUndefAttrName()); + return mlir::ArrayAttr::get(ctx, {attrs.getDictionary(ctx)}); } /// The number of bytes a coercion memory slot needs to hold a value of type @@ -1259,19 +1301,11 @@ void rewriteIndirectReturnCall(cir::CallOp call, copyCallAttributes(call, newCall); newCall->removeAttr("res_attrs"); - // Shape the per-argument attrs exactly as the non-sret path does - // (signext / zeroext for Extend, drop Ignore slots, byval / align for - // Indirect, flatten for Expand and Direct+canFlatten) before prepending the - // sret slot, so sret composes correctly with Extend / Ignore / Indirect / - // Expand / Direct+canFlatten args. + // Shape the per-argument attrs exactly as the non-sret path does, through + // updateArgAttrs, before prepending the sret slot, so sret composes with + // every argument classification. mlir::ArrayAttr argAttrs = call->getAttrOfType<mlir::ArrayAttr>("arg_attrs"); - bool needsArgAttrUpdate = - llvm::any_of(fc.argInfos, [](const ArgClassification &ac) { - return ac.kind == ArgKind::Ignore || ac.kind == ArgKind::Extend || - ac.kind == ArgKind::Indirect || ac.kind == ArgKind::Expand || - getFlattenedCoercedType(ac); - }); - if (needsArgAttrUpdate) + if (needsArgAttrUpdate(fc)) argAttrs = updateArgAttrs(ctx, origCallArgTypes, argAttrs, fc, dl); applySretSlotAttrs(newCall, argAttrs, origRetTy, sretAlign, builder); @@ -1609,17 +1643,8 @@ mlir::LogicalResult CIRABIRewriteContext::rewriteFunctionDefinition( funcOp.setFunctionTypeAttr(mlir::TypeAttr::get(newFnTy)); // Rebuild arg_attrs when the function has an sret slot (slot 0 needs the - // sret attribute set) or any arg is Ignore (dropped from the output array), - // Extend (needs llvm.signext / llvm.zeroext), Indirect (gains the pointer - // attributes updateArgAttrs applies), Expand or Direct+canFlatten (both - // change the argument count). - bool needsArgAttrUpdate = - hasSRet || llvm::any_of(fc.argInfos, [](const ArgClassification &ac) { - return ac.kind == ArgKind::Ignore || ac.kind == ArgKind::Extend || - ac.kind == ArgKind::Indirect || ac.kind == ArgKind::Expand || - getFlattenedCoercedType(ac); - }); - if (needsArgAttrUpdate) { + // sret attribute set) or any arg needs updateArgAttrs. + if (hasSRet || needsArgAttrUpdate(fc)) { auto existing = funcOp->getAttrOfType<mlir::ArrayAttr>("arg_attrs"); mlir::ArrayAttr updated = updateArgAttrs(ctx, oldArgTypes, existing, fc, dl); @@ -1641,10 +1666,12 @@ mlir::LogicalResult CIRABIRewriteContext::rewriteFunctionDefinition( if (mlir::isa<cir::VoidType>(newRetTy)) { funcOp->removeAttr("res_attrs"); - } else if (fc.returnInfo.kind == ArgKind::Extend) { - // Layer llvm.signext / llvm.zeroext onto an Extend return. + } else if (fc.returnInfo.kind == ArgKind::Extend || + fc.returnInfo.kind == ArgKind::Direct) { auto existing = funcOp->getAttrOfType<mlir::ArrayAttr>("res_attrs"); - funcOp->setAttr("res_attrs", updateResAttrs(ctx, existing, fc.returnInfo)); + if (mlir::ArrayAttr updated = + updateResAttrs(ctx, existing, fc.returnInfo, origRetTy, dl)) + funcOp->setAttr("res_attrs", updated); } return mlir::success(); @@ -1872,24 +1899,19 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, call.getResult().replaceAllUsesWith(coercedBack); } - // Layer llvm.signext / llvm.zeroext onto the new call's arg_attrs and - // res_attrs for Extend args/return. Ignore args require a rebuild because - // their slots are dropped; Indirect args need llvm.byval / llvm.align; - // Expand and Direct+canFlatten args change the argument count. - bool needsArgAttrUpdate = - llvm::any_of(fc.argInfos, [](const ArgClassification &ac) { - return ac.kind == ArgKind::Ignore || ac.kind == ArgKind::Extend || - ac.kind == ArgKind::Indirect || ac.kind == ArgKind::Expand || - getFlattenedCoercedType(ac); - }); - if (needsArgAttrUpdate) { + if (needsArgAttrUpdate(fc)) { auto existing = call->getAttrOfType<mlir::ArrayAttr>("arg_attrs"); newCall->setAttr("arg_attrs", updateArgAttrs(ctx, origCallArgTypes, existing, fc, dl)); } - if (fc.returnInfo.kind == ArgKind::Extend) { + // Add llvm.signext / llvm.zeroext to an Extend return, and drop llvm.noundef + // from a coerced Direct return that passes undefined bits. + if (fc.returnInfo.kind == ArgKind::Extend || + (hasResult && fc.returnInfo.kind == ArgKind::Direct)) { auto existing = call->getAttrOfType<mlir::ArrayAttr>("res_attrs"); - newCall->setAttr("res_attrs", updateResAttrs(ctx, existing, fc.returnInfo)); + if (mlir::ArrayAttr updated = + updateResAttrs(ctx, existing, fc.returnInfo, origRetTy, dl)) + newCall->setAttr("res_attrs", updated); } else if (hasResult && mlir::isa<cir::VoidType>(callRetTy)) { newCall->removeAttr("res_attrs"); } diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp index a254d798362290..4a49b252515496 100644 --- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp +++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp @@ -220,10 +220,10 @@ static mlir::Value emitToMemory(mlir::ConversionPatternRewriter &rewriter, resultTy = mlir::VectorType::get( vecTy.getSize(), vecTy.getElementType(), vecTy.getIsScalable()); else { - uint64_t bytePadded = std::max<uint64_t>(vecTy.getSize(), 8); - resultTy = mlir::IntegerType::get(origType.getContext(), bytePadded); - value = emitBoolVecConversion( - rewriter, value, dyn_cast<mlir::IntegerType>(resultTy).getWidth()); + resultTy = mlir::IntegerType::get(origType.getContext(), + vecTy.getBoolStorageWidth()); + value = + emitBoolVecConversion(rewriter, value, vecTy.getBoolStorageWidth()); } return mlir::LLVM::BitcastOp::create(rewriter, value.getLoc(), resultTy, value); diff --git a/clang/lib/CIR/Lowering/LoweringHelpers.... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/230280 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
