From f15bb20c68da32ecbb7b6dca4c31a9e6b6e8c443 Mon Sep 17 00:00:00 2001 From: Jessica Clarke Date: Tue, 4 Jun 2024 23:57:30 +0100 Subject: [PATCH] [IR][CodeGen] Add new SizedCapabilityType for use in CodeGen EVT::getTypeForEVT currently returns a PointerType for MVT::cN, but this has a couple of issues. The first issue is we have to hard-code the address space, though that's not such a big deal given we do that elsewhere too. The bigger issue is that, when we later pass that to MVT::getVT or EVT::getEVT, it doesn't know what the right size is, so returns MVT::cPTR instead, which is not a true value type, and is supposed to only be used by TableGen. This has been seen to confuse TargetLoweringBase::getTypeConversion, as when presented with a vector of capability pointers it can end up trying to recreate a smaller vector of the same type, but this trips up various assertions for the MVT::cPTR as both the IR methods and the code here are expecting to be dealing with actual value types. Borrowing the idea of TypedPointerType (DXILPointerTyID) a bit, introduce a new IR type, SizedCapabilityType, to represent a fixed-size capability during CodeGen, which allows lossless roundtripping from MVT to Type and back. This fixes building cheritest, which has crashed since the introduction of cPTR due to cPTR not being a value type, mirroring iPTR, unlike the old iFATPTRAny which was its own weird beast, but wouldn't have tripped up these assertions. It probably didn't do the most sensible things though. Fixes: 7aa7f2ec8f27 ("[CodeGen] Rework MVT representation of capabilities and add type inference") --- llvm/include/llvm/IR/DataLayout.h | 2 ++ llvm/include/llvm/IR/DerivedTypes.h | 35 +++++++++++++++++++ llvm/include/llvm/IR/Type.h | 10 ++++-- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 2 ++ llvm/lib/CodeGen/ValueTypes.cpp | 12 ++++--- llvm/lib/IR/AsmWriter.cpp | 3 ++ llvm/lib/IR/Core.cpp | 2 ++ llvm/lib/IR/DataLayout.cpp | 2 ++ llvm/lib/IR/LLVMContextImpl.h | 1 + llvm/lib/IR/Type.cpp | 31 +++++++++++++--- .../DirectX/DXILWriter/DXILBitcodeWriter.cpp | 1 + .../Hexagon/HexagonTargetObjectFile.cpp | 1 + .../SLPVectorizer/cheri-crash-cPTR-element.ll | 34 ++++++++++++++++++ 13 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 llvm/test/Transforms/SLPVectorizer/cheri-crash-cPTR-element.ll diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h index 16d3c6fe6dd9..790ce9d5c7b3 100644 --- a/llvm/include/llvm/IR/DataLayout.h +++ b/llvm/include/llvm/IR/DataLayout.h @@ -756,6 +756,8 @@ inline TypeSize DataLayout::getTypeSizeInBits(Type *Ty) const { getTypeSizeInBits(VTy->getElementType()).getFixedSize(); return TypeSize(MinBits, EltCnt.isScalable()); } + case Type::SizedCapabilityTyID: + return TypeSize::Fixed(cast(Ty)->getBitWidth()); default: llvm_unreachable("DataLayout::getTypeSizeInBits(): Unsupported type"); } diff --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h index c01ec93ff737..9728dd64d484 100644 --- a/llvm/include/llvm/IR/DerivedTypes.h +++ b/llvm/include/llvm/IR/DerivedTypes.h @@ -705,6 +705,41 @@ class PointerType : public Type { } }; +/// Class to represent fixed-size capability types during CodeGen. +class SizedCapabilityType : public Type { + friend class LLVMContextImpl; + +protected: + explicit SizedCapabilityType(LLVMContext &C, unsigned NumBits) + : Type(C, SizedCapabilityTyID) { + setSubclassData(NumBits); + } + +public: + /// This enum is just used to hold constants we need for SizedCapabilityType. + enum { + MIN_CAP_BITS = 64, ///< Minimum number of bits that can be specified + MAX_CAP_BITS = 256 ///< Maximum number of bits that can be specified + ///< Note that bit width is stored in the Type classes SubclassData field + ///< which has 24 bits. + }; + + /// This static method is the primary way of constructing an IntegerType. + /// If an IntegerType with the same NumBits value was previously instantiated, + /// that instance will be returned. Otherwise a new one will be created. Only + /// one instance with a given NumBits value is ever created. + /// Get or create an IntegerType instance. + static SizedCapabilityType *get(LLVMContext &C, unsigned NumBits); + + /// Get the number of bits in this IntegerType + unsigned getBitWidth() const { return getSubclassData(); } + + /// Methods for support type inquiry through isa, cast, and dyn_cast. + static bool classof(const Type *T) { + return T->getTypeID() == SizedCapabilityTyID; + } +}; + Type *Type::getExtendedType() const { assert( isIntOrIntVectorTy() && diff --git a/llvm/include/llvm/IR/Type.h b/llvm/include/llvm/IR/Type.h index a17184672e46..94a3995dee01 100644 --- a/llvm/include/llvm/IR/Type.h +++ b/llvm/include/llvm/IR/Type.h @@ -82,6 +82,7 @@ class Type { FixedVectorTyID, ///< Fixed width SIMD vector type ScalableVectorTyID, ///< Scalable SIMD vector type DXILPointerTyID, ///< DXIL typed pointer used by DirectX target + SizedCapabilityTyID,///< Fixed-size CHERI capability type }; private: @@ -225,6 +226,11 @@ class Type { /// True if this is an instance of PointerType. bool isPointerTy() const { return getTypeID() == PointerTyID; } + /// True if this is an instance of PointerType. + bool isSizedCapabilityTy() const { + return getTypeID() == SizedCapabilityTyID; + } + /// True if this is an instance of an opaque PointerType. bool isOpaquePointerTy() const; @@ -272,8 +278,8 @@ class Type { bool isSized(SmallPtrSetImpl *Visited = nullptr) const { // If it's a primitive, it is always sized. if (getTypeID() == IntegerTyID || isFloatingPointTy() || - getTypeID() == PointerTyID || getTypeID() == X86_MMXTyID || - getTypeID() == X86_AMXTyID) + getTypeID() == PointerTyID || getTypeID() == SizedCapabilityTyID || + getTypeID() == X86_MMXTyID || getTypeID() == X86_AMXTyID) return true; // If it is not something that can have a size (e.g. a function or label), // it doesn't have a size. diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index 383ab8b25641..36ca0d32e9a8 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -1037,6 +1037,8 @@ void ModuleBitcodeWriter::writeTypeTable() { } case Type::DXILPointerTyID: llvm_unreachable("DXIL pointers cannot be added to IR modules"); + case Type::SizedCapabilityTyID: + llvm_unreachable("Fixed-size capabilities cannot be added to IR modules"); } // Emit the finished record. diff --git a/llvm/lib/CodeGen/ValueTypes.cpp b/llvm/lib/CodeGen/ValueTypes.cpp index 0524003a1882..ad2be01bab4d 100644 --- a/llvm/lib/CodeGen/ValueTypes.cpp +++ b/llvm/lib/CodeGen/ValueTypes.cpp @@ -535,10 +535,11 @@ Type *EVT::getTypeForEVT(LLVMContext &Context) const { return ScalableVectorType::get(Type::getDoubleTy(Context), 8); case MVT::Metadata: return Type::getMetadataTy(Context); case MVT::c64: + return SizedCapabilityType::get(Context, 64); case MVT::c128: + return SizedCapabilityType::get(Context, 128); case MVT::c256: - // XXX: Hard-coded AS - return PointerType::get(Type::getInt8Ty(Context), 200); + return SizedCapabilityType::get(Context, 256); } // clang-format on } @@ -555,6 +556,8 @@ MVT MVT::getVT(Type *Ty, bool HandleUnknown){ return MVT::isVoid; case Type::IntegerTyID: return getIntegerVT(cast(Ty)->getBitWidth()); + case Type::SizedCapabilityTyID: + return getCapabilityVT(cast(Ty)->getBitWidth()); case Type::HalfTyID: return MVT(MVT::f16); case Type::BFloatTyID: return MVT(MVT::bf16); case Type::FloatTyID: return MVT(MVT::f32); @@ -565,9 +568,8 @@ MVT MVT::getVT(Type *Ty, bool HandleUnknown){ case Type::FP128TyID: return MVT(MVT::f128); case Type::PPC_FP128TyID: return MVT(MVT::ppcf128); case Type::PointerTyID: { - // FIXME: require a DataLayout here! - if (isCheriPointer(Ty, nullptr)) - return MVT(MVT::cPTR); + // NB: Removing this upstream, so ensure we haven't made it worse + assert(!isCheriPointer(Ty, nullptr)); return MVT(MVT::iPTR); } case Type::FixedVectorTyID: diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp index 18ed2c63f24a..6135abd73a7b 100644 --- a/llvm/lib/IR/AsmWriter.cpp +++ b/llvm/lib/IR/AsmWriter.cpp @@ -618,6 +618,9 @@ void TypePrinting::print(Type *Ty, raw_ostream &OS) { // extra dependencies we just print the pointer's address here. OS << "dxil-ptr (" << Ty << ")"; return; + case Type::SizedCapabilityTyID: + OS << 'c' << cast(Ty)->getBitWidth(); + return; } llvm_unreachable("Invalid TypeID"); } diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp index 08b7b0e1f956..397fbdfd101a 100644 --- a/llvm/lib/IR/Core.cpp +++ b/llvm/lib/IR/Core.cpp @@ -543,6 +543,8 @@ LLVMTypeKind LLVMGetTypeKind(LLVMTypeRef Ty) { return LLVMScalableVectorTypeKind; case Type::DXILPointerTyID: llvm_unreachable("DXIL pointers are unsupported via the C API"); + case Type::SizedCapabilityTyID: + llvm_unreachable("Fixed-size capabilities are unsupported via the C API"); } llvm_unreachable("Unhandled TypeID."); } diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp index e6f64dcec60d..7896c24c8dd1 100644 --- a/llvm/lib/IR/DataLayout.cpp +++ b/llvm/lib/IR/DataLayout.cpp @@ -843,6 +843,8 @@ Align DataLayout::getAlignment(Type *Ty, bool abi_or_pref) const { } case Type::X86_AMXTyID: return Align(64); + case Type::SizedCapabilityTyID: + return Align(cast(Ty)->getBitWidth() / 8); default: llvm_unreachable("Bad type for getAlignment!!!"); } diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h index 47add940f603..3166438778ca 100644 --- a/llvm/lib/IR/LLVMContextImpl.h +++ b/llvm/lib/IR/LLVMContextImpl.h @@ -1484,6 +1484,7 @@ class LLVMContextImpl { DenseMap, VectorType *> VectorTypes; DenseMap PointerTypes; // Pointers in AddrSpace = 0 DenseMap, PointerType *> ASPointerTypes; + SmallDenseMap SizedCapabilityTypes; /// ValueHandles - This map keeps track of all of the value handles that are /// watching a Value*. The Value::HasValueHandle bit is used to know diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp index 85b658c8a52f..a1039c1bae77 100644 --- a/llvm/lib/IR/Type.cpp +++ b/llvm/lib/IR/Type.cpp @@ -182,6 +182,8 @@ TypeSize Type::getPrimitiveSizeInBits() const { assert(!ETS.isScalable() && "Vector type should have fixed-width elements"); return {ETS.getFixedSize() * EC.getKnownMinValue(), EC.isScalable()}; } + case Type::SizedCapabilityTyID: + return TypeSize::Fixed(cast(this)->getBitWidth()); default: return TypeSize::Fixed(0); } } @@ -674,7 +676,7 @@ VectorType *VectorType::get(Type *ElementType, ElementCount EC) { bool VectorType::isValidElementType(Type *ElemTy) { return ElemTy->isIntegerTy() || ElemTy->isFloatingPointTy() || - ElemTy->isPointerTy(); + ElemTy->isPointerTy() || ElemTy->isSizedCapabilityTy(); } //===----------------------------------------------------------------------===// @@ -684,8 +686,9 @@ bool VectorType::isValidElementType(Type *ElemTy) { FixedVectorType *FixedVectorType::get(Type *ElementType, unsigned NumElts) { assert(NumElts > 0 && "#Elements of a VectorType must be greater than 0"); assert(isValidElementType(ElementType) && "Element type of a VectorType must " - "be an integer, floating point, or " - "pointer type."); + "be an integer, floating point, " + "pointer, or sized capability " + "type."); auto EC = ElementCount::getFixed(NumElts); @@ -706,8 +709,9 @@ ScalableVectorType *ScalableVectorType::get(Type *ElementType, unsigned MinNumElts) { assert(MinNumElts > 0 && "#Elements of a VectorType must be greater than 0"); assert(isValidElementType(ElementType) && "Element type of a VectorType must " - "be an integer, floating point, or " - "pointer type."); + "be an integer, floating point, " + "pointer, or sized capability " + "type."); auto EC = ElementCount::getScalable(MinNumElts); @@ -784,3 +788,20 @@ bool PointerType::isValidElementType(Type *ElemTy) { bool PointerType::isLoadableOrStorableType(Type *ElemTy) { return isValidElementType(ElemTy) && !ElemTy->isFunctionTy(); } + +//===----------------------------------------------------------------------===// +// SizedCapabilityType Implementation +//===----------------------------------------------------------------------===// + +SizedCapabilityType *SizedCapabilityType::get(LLVMContext &C, + unsigned NumBits) { + assert(NumBits >= MIN_CAP_BITS && "bitwidth too small"); + assert(NumBits <= MAX_CAP_BITS && "bitwidth too large"); + + SizedCapabilityType *&Entry = C.pImpl->SizedCapabilityTypes[NumBits]; + + if (!Entry) + Entry = new (C.pImpl->Alloc) SizedCapabilityType(C, NumBits); + + return Entry; +} diff --git a/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp b/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp index 869433613620..0126a0267ba8 100644 --- a/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp +++ b/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp @@ -1031,6 +1031,7 @@ void DXILBitcodeWriter::writeTypeTable() { case Type::BFloatTyID: case Type::X86_AMXTyID: case Type::TokenTyID: + case Type::SizedCapabilityTyID: llvm_unreachable("These should never be used!!!"); break; case Type::VoidTyID: diff --git a/llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp b/llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp index c83ed16f0272..b43285bec8a6 100644 --- a/llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp +++ b/llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp @@ -333,6 +333,7 @@ unsigned HexagonTargetObjectFile::getSmallestAddressableSize(const Type *Ty, case Type::X86_AMXTyID: case Type::TokenTyID: case Type::DXILPointerTyID: + case Type::SizedCapabilityTyID: return 0; } diff --git a/llvm/test/Transforms/SLPVectorizer/cheri-crash-cPTR-element.ll b/llvm/test/Transforms/SLPVectorizer/cheri-crash-cPTR-element.ll new file mode 100644 index 000000000000..9888cd2dd14d --- /dev/null +++ b/llvm/test/Transforms/SLPVectorizer/cheri-crash-cPTR-element.ll @@ -0,0 +1,34 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt -mattr=+cheri128 -mcpu=beri -passes="function(slp-vectorizer)" -S < %s | FileCheck %s + +target datalayout = "E-m:e-pf200:128:128:128:64-i8:8:32-i16:16:32-i64:64-n32:64-S128" +target triple = "mips64c128-unknown-freebsd" + +;; This previously crashed in TargetLoweringBase due to creating a vector which +;; gave an element type of MVT::cPTR when queried rather than MVT::c128. +define ptr @cmemcpy(ptr addrspace(200) %0, ptr addrspace(200) %1, i1 %tobool) { +; CHECK-LABEL: @cmemcpy( +; CHECK-NEXT: entry: +; CHECK-NEXT: br i1 [[TOBOOL:%.*]], label [[IF_END12:%.*]], label [[DO_BODY45:%.*]] +; CHECK: if.end12: +; CHECK-NEXT: br label [[DO_BODY45]] +; CHECK: do.body45: +; CHECK-NEXT: [[SRC2_0:%.*]] = phi ptr addrspace(200) [ [[TMP0:%.*]], [[IF_END12]] ], [ [[TMP1:%.*]], [[ENTRY:%.*]] ] +; CHECK-NEXT: [[DST1_0:%.*]] = phi ptr addrspace(200) [ [[TMP1]], [[IF_END12]] ], [ [[TMP0]], [[ENTRY]] ] +; CHECK-NEXT: [[TMP2:%.*]] = load i64, ptr addrspace(200) [[SRC2_0]], align 8 +; CHECK-NEXT: store i64 [[TMP2]], ptr addrspace(200) [[DST1_0]], align 8 +; CHECK-NEXT: ret ptr null +; +entry: + br i1 %tobool, label %if.end12, label %do.body45 + +if.end12: + br label %do.body45 + +do.body45: + %src2.0 = phi ptr addrspace(200) [ %0, %if.end12 ], [ %1, %entry ] + %dst1.0 = phi ptr addrspace(200) [ %1, %if.end12 ], [ %0, %entry ] + %2 = load i64, ptr addrspace(200) %src2.0, align 8 + store i64 %2, ptr addrspace(200) %dst1.0, align 8 + ret ptr null +}