From f8a981a7ac07cd6a78940a3f22f85dcc9b348c3b Mon Sep 17 00:00:00 2001 From: Jay Foad Date: Mon, 18 Nov 2024 14:34:19 +0000 Subject: [PATCH 1/2] [IR] Improve check for target extension types disallowed in globals. For target extension types that are not allowed to be used as globals, also disallow them nested inside array and structure types. --- llvm/include/llvm/IR/DerivedTypes.h | 10 ++++- llvm/include/llvm/IR/Type.h | 6 +++ llvm/lib/IR/Type.cpp | 44 +++++++++++++++++++ llvm/lib/IR/Verifier.cpp | 23 +++++----- llvm/test/Assembler/target-type-properties.ll | 16 +++++-- 5 files changed, 83 insertions(+), 16 deletions(-) diff --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h index 19d351456d658e..65f9810776024e 100644 --- a/llvm/include/llvm/IR/DerivedTypes.h +++ b/llvm/include/llvm/IR/DerivedTypes.h @@ -225,7 +225,9 @@ class StructType : public Type { SCDB_IsLiteral = 4, SCDB_IsSized = 8, SCDB_ContainsScalableVector = 16, - SCDB_NotContainsScalableVector = 32 + SCDB_NotContainsScalableVector = 32, + SCDB_ContainsNonGlobalTargetExtType = 64, + SCDB_NotContainsNonGlobalTargetExtType = 128, }; /// For a named struct that actually has a name, this is a pointer to the @@ -294,6 +296,12 @@ class StructType : public Type { bool isScalableTy(SmallPtrSetImpl &Visited) const; using Type::isScalableTy; + /// Return true if this type is or contains a target extension type that + /// disallows being used as a global. + bool + containsNonGlobalTargetExtType(SmallPtrSetImpl &Visited) const; + using Type::containsNonGlobalTargetExtType; + /// Returns true if this struct contains homogeneous scalable vector types. /// Note that the definition of homogeneous scalable vector type is not /// recursive here. That means the following structure will return false diff --git a/llvm/include/llvm/IR/Type.h b/llvm/include/llvm/IR/Type.h index 7fa940ab347af6..000fdee45bb861 100644 --- a/llvm/include/llvm/IR/Type.h +++ b/llvm/include/llvm/IR/Type.h @@ -209,6 +209,12 @@ class Type { bool isScalableTy(SmallPtrSetImpl &Visited) const; bool isScalableTy() const; + /// Return true if this type is or contains a target extension type that + /// disallows being used as a global. + bool + containsNonGlobalTargetExtType(SmallPtrSetImpl &Visited) const; + bool containsNonGlobalTargetExtType() const; + /// Return true if this is a FP type or a vector of FP. bool isFPOrFPVectorTy() const { return getScalarType()->isFloatingPointTy(); } diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp index 88ede0d35fa3ee..6def8ac27c0fa4 100644 --- a/llvm/lib/IR/Type.cpp +++ b/llvm/lib/IR/Type.cpp @@ -72,6 +72,22 @@ bool Type::isScalableTy() const { return isScalableTy(Visited); } +bool Type::containsNonGlobalTargetExtType( + SmallPtrSetImpl &Visited) const { + if (const auto *ATy = dyn_cast(this)) + return ATy->getElementType()->containsNonGlobalTargetExtType(Visited); + if (const auto *STy = dyn_cast(this)) + return STy->containsNonGlobalTargetExtType(Visited); + if (auto *TT = dyn_cast(this)) + return !TT->hasProperty(TargetExtType::CanBeGlobal); + return false; +} + +bool Type::containsNonGlobalTargetExtType() const { + SmallPtrSet Visited; + return containsNonGlobalTargetExtType(Visited); +} + const fltSemantics &Type::getFltSemantics() const { switch (getTypeID()) { case HalfTyID: return APFloat::IEEEhalf(); @@ -425,6 +441,34 @@ bool StructType::isScalableTy(SmallPtrSetImpl &Visited) const { return false; } +bool StructType::containsNonGlobalTargetExtType( + SmallPtrSetImpl &Visited) const { + if ((getSubclassData() & SCDB_ContainsNonGlobalTargetExtType) != 0) + return true; + + if ((getSubclassData() & SCDB_NotContainsNonGlobalTargetExtType) != 0) + return false; + + if (!Visited.insert(this).second) + return false; + + for (Type *Ty : elements()) { + if (Ty->containsNonGlobalTargetExtType(Visited)) { + const_cast(this)->setSubclassData( + getSubclassData() | SCDB_ContainsNonGlobalTargetExtType); + return true; + } + } + + // For structures that are opaque, return false but do not set the + // SCDB_NotContainsNonGlobalTargetExtType flag since it may gain non-global + // target extension types when it becomes non-opaque. + if (!isOpaque()) + const_cast(this)->setSubclassData( + getSubclassData() | SCDB_NotContainsNonGlobalTargetExtType); + return false; +} + bool StructType::containsHomogeneousScalableVectorTypes() const { if (getNumElements() <= 0 || !isa(elements().front())) return false; diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 5cfcd21e508595..5c0ccf734cccbc 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -829,8 +829,10 @@ void Verifier::visitGlobalValue(const GlobalValue &GV) { } void Verifier::visitGlobalVariable(const GlobalVariable &GV) { + Type *GVType = GV.getValueType(); + if (GV.hasInitializer()) { - Check(GV.getInitializer()->getType() == GV.getValueType(), + Check(GV.getInitializer()->getType() == GVType, "Global variable initializer type does not match global " "variable type!", &GV); @@ -854,7 +856,7 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) { // Don't worry about emitting an error for it not being an array, // visitGlobalValue will complain on appending non-array. - if (ArrayType *ATy = dyn_cast(GV.getValueType())) { + if (ArrayType *ATy = dyn_cast(GVType)) { StructType *STy = dyn_cast(ATy->getElementType()); PointerType *FuncPtrTy = PointerType::get(Context, DL.getProgramAddressSpace()); @@ -878,7 +880,6 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) { Check(GV.materialized_use_empty(), "invalid uses of intrinsic global variable", &GV); - Type *GVType = GV.getValueType(); if (ArrayType *ATy = dyn_cast(GVType)) { PointerType *PTy = dyn_cast(ATy->getElementType()); Check(PTy, "wrong type for intrinsic global variable", &GV); @@ -912,15 +913,13 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) { // Scalable vectors cannot be global variables, since we don't know // the runtime size. - Check(!GV.getValueType()->isScalableTy(), - "Globals cannot contain scalable types", &GV); - - // Check if it's a target extension type that disallows being used as a - // global. - if (auto *TTy = dyn_cast(GV.getValueType())) - Check(TTy->hasProperty(TargetExtType::CanBeGlobal), - "Global @" + GV.getName() + " has illegal target extension type", - TTy); + Check(!GVType->isScalableTy(), "Globals cannot contain scalable types", &GV); + + // Check if it is or contains a target extension type that disallows being + // used as a global. + Check(!GVType->containsNonGlobalTargetExtType(), + "Global @" + GV.getName() + " has illegal target extension type", + GVType); if (!GV.hasInitializer()) { visitGlobalValue(GV); diff --git a/llvm/test/Assembler/target-type-properties.ll b/llvm/test/Assembler/target-type-properties.ll index 49c9d812f1cf4a..60790dbc5c17bb 100644 --- a/llvm/test/Assembler/target-type-properties.ll +++ b/llvm/test/Assembler/target-type-properties.ll @@ -1,6 +1,8 @@ ; RUN: split-file %s %t ; RUN: not llvm-as < %t/zeroinit-error.ll -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-ZEROINIT %s -; RUN: not llvm-as < %t/global-var.ll -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-GLOBALVAR %s +; RUN: not llvm-as < %t/global-var.ll -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-GLOBAL-VAR %s +; RUN: not llvm-as < %t/global-array.ll -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-GLOBAL-ARRAY %s +; RUN: not llvm-as < %t/global-struct.ll -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-GLOBAL-STRUCT %s ; Check target extension type properties are verified in the assembler. ;--- zeroinit-error.ll @@ -12,5 +14,13 @@ define void @foo() { } ;--- global-var.ll -@global = external global target("unknown_target_type") -; CHECK-GLOBALVAR: Global @global has illegal target extension type +@global_var = external global target("unknown_target_type") +; CHECK-GLOBAL-VAR: Global @global_var has illegal target extension type + +;--- global-array.ll +@global_array = external global [4 x target("unknown_target_type")] +; CHECK-GLOBAL-ARRAY: Global @global_array has illegal target extension type + +;--- global-struct.ll +@global_struct = external global {target("unknown_target_type")} +; CHECK-GLOBAL-STRUCT: Global @global_struct has illegal target extension type From ee6637fe9cc270aceb9f17980476579ee7caaad1 Mon Sep 17 00:00:00 2001 From: Jay Foad Date: Mon, 18 Nov 2024 17:13:39 +0000 Subject: [PATCH 2/2] Mark DirectX resource types as CanBeGlobal --- llvm/lib/IR/Type.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp index 6def8ac27c0fa4..75f4751ea4f148 100644 --- a/llvm/lib/IR/Type.cpp +++ b/llvm/lib/IR/Type.cpp @@ -947,7 +947,7 @@ static TargetTypeInfo getTargetTypeInfo(const TargetExtType *Ty) { // DirectX resources if (Name.starts_with("dx.")) - return TargetTypeInfo(PointerType::get(C, 0)); + return TargetTypeInfo(PointerType::get(C, 0), TargetExtType::CanBeGlobal); // Opaque types in the AMDGPU name space. if (Name == "amdgcn.named.barrier") {