[llvm] [IR] Fix undiagnosed cases of structs containing scalable vectors (PR #113455)
Jay Foad via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 23 06:13:04 PDT 2024
https://github.com/jayfoad created https://github.com/llvm/llvm-project/pull/113455
Type::isScalableTy and StructType::containsScalableVectorType failed to
detect some cases of structs containing scalable vectors because
containsScalableVectorType did not call back into isScalableTy to check
the element types. Fix this, which requires sharing the same Visited set
in both functions. Also change the external API so that callers are
never required to pass in a Visited set, and normalize the naming to
isScalableTy.
>From 83e3d8635a6b5e58184d8dcd94900007fd15b589 Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Wed, 23 Oct 2024 11:33:12 +0100
Subject: [PATCH 1/5] Change callers of StructType::containsScalableVectorType
to Type::isScalableTy
---
llvm/lib/AsmParser/LLParser.cpp | 2 +-
llvm/lib/IR/Verifier.cpp | 3 +--
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 2 +-
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 6a2372c9751408..8ddb2efb0e26c2 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -8525,7 +8525,7 @@ int LLParser::parseGetElementPtr(Instruction *&Inst, PerFunctionState &PFS) {
return error(Loc, "base element of getelementptr must be sized");
auto *STy = dyn_cast<StructType>(Ty);
- if (STy && STy->containsScalableVectorType())
+ if (STy && STy->isScalableTy())
return error(Loc, "getelementptr cannot target structure that contains "
"scalable vector type");
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index f34fe7594c8602..60e65392218dad 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -4107,8 +4107,7 @@ void Verifier::visitGetElementPtrInst(GetElementPtrInst &GEP) {
Check(GEP.getSourceElementType()->isSized(), "GEP into unsized type!", &GEP);
if (auto *STy = dyn_cast<StructType>(GEP.getSourceElementType())) {
- SmallPtrSet<Type *, 4> Visited;
- Check(!STy->containsScalableVectorType(&Visited),
+ Check(!STy->isScalableTy(),
"getelementptr cannot target structure that contains scalable vector"
"type",
&GEP);
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index c8b9f166b16020..971ace2a4f4716 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -4087,7 +4087,7 @@ Instruction *InstCombinerImpl::visitExtractValueInst(ExtractValueInst &EV) {
if (LoadInst *L = dyn_cast<LoadInst>(Agg)) {
// Bail out if the aggregate contains scalable vector type
if (auto *STy = dyn_cast<StructType>(Agg->getType());
- STy && STy->containsScalableVectorType())
+ STy && STy->isScalableTy())
return nullptr;
// If the (non-volatile) load only has one use, we can rewrite this to a
>From a4a26a106ce98966357e3b1072a942dbf2f09f19 Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Wed, 23 Oct 2024 11:44:28 +0100
Subject: [PATCH 2/5] Rename StructType::containsScalableVectorType to
isScalableTy
---
llvm/include/llvm/IR/DerivedTypes.h | 3 +--
llvm/lib/IR/Type.cpp | 7 +++----
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h
index a24801d8bdf834..6890ddcaf951de 100644
--- a/llvm/include/llvm/IR/DerivedTypes.h
+++ b/llvm/include/llvm/IR/DerivedTypes.h
@@ -290,8 +290,7 @@ class StructType : public Type {
bool isSized(SmallPtrSetImpl<Type *> *Visited = nullptr) const;
/// Returns true if this struct contains a scalable vector.
- bool
- containsScalableVectorType(SmallPtrSetImpl<Type *> *Visited = nullptr) const;
+ bool isScalableTy(SmallPtrSetImpl<Type *> *Visited = nullptr) const;
/// Returns true if this struct contains homogeneous scalable vector types.
/// Note that the definition of homogeneous scalable vector type is not
diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index f618263f79c313..833b6afeb6a682 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -63,7 +63,7 @@ bool Type::isScalableTy() const {
return ATy->getElementType()->isScalableTy();
if (const auto *STy = dyn_cast<StructType>(this)) {
SmallPtrSet<Type *, 4> Visited;
- return STy->containsScalableVectorType(&Visited);
+ return STy->isScalableTy(&Visited);
}
return getTypeID() == ScalableVectorTyID || isScalableTargetExtTy();
}
@@ -394,8 +394,7 @@ StructType *StructType::get(LLVMContext &Context, ArrayRef<Type*> ETypes,
return ST;
}
-bool StructType::containsScalableVectorType(
- SmallPtrSetImpl<Type *> *Visited) const {
+bool StructType::isScalableTy(SmallPtrSetImpl<Type *> *Visited) const {
if ((getSubclassData() & SCDB_ContainsScalableVector) != 0)
return true;
@@ -412,7 +411,7 @@ bool StructType::containsScalableVectorType(
return true;
}
if (auto *STy = dyn_cast<StructType>(Ty)) {
- if (STy->containsScalableVectorType(Visited)) {
+ if (STy->isScalableTy(Visited)) {
const_cast<StructType *>(this)->setSubclassData(
getSubclassData() | SCDB_ContainsScalableVector);
return true;
>From c4ffee405d95584517f1c5f8e87d79121734def9 Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Wed, 23 Oct 2024 12:10:43 +0100
Subject: [PATCH 3/5] Allow passing Visited set into Type::isScalableTy
---
llvm/include/llvm/IR/DerivedTypes.h | 3 ++-
llvm/include/llvm/IR/Type.h | 1 +
llvm/lib/IR/Type.cpp | 19 +++++++++++--------
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h
index 6890ddcaf951de..34a10874f51523 100644
--- a/llvm/include/llvm/IR/DerivedTypes.h
+++ b/llvm/include/llvm/IR/DerivedTypes.h
@@ -290,7 +290,8 @@ class StructType : public Type {
bool isSized(SmallPtrSetImpl<Type *> *Visited = nullptr) const;
/// Returns true if this struct contains a scalable vector.
- bool isScalableTy(SmallPtrSetImpl<Type *> *Visited = nullptr) const;
+ bool isScalableTy(SmallPtrSetImpl<Type *> &Visited) const;
+ using Type::isScalableTy;
/// Returns true if this struct contains homogeneous scalable vector types.
/// Note that the definition of homogeneous scalable vector type is not
diff --git a/llvm/include/llvm/IR/Type.h b/llvm/include/llvm/IR/Type.h
index 2f53197df19998..3a6ead5dc7db25 100644
--- a/llvm/include/llvm/IR/Type.h
+++ b/llvm/include/llvm/IR/Type.h
@@ -206,6 +206,7 @@ class Type {
bool isScalableTargetExtTy() const;
/// Return true if this is a type whose size is a known multiple of vscale.
+ bool isScalableTy(SmallPtrSetImpl<Type *> &Visited) const;
bool isScalableTy() const;
/// Return true if this is a FP type or a vector of FP.
diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index 833b6afeb6a682..b09ac12b8ae947 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -58,16 +58,19 @@ bool Type::isIntegerTy(unsigned Bitwidth) const {
return isIntegerTy() && cast<IntegerType>(this)->getBitWidth() == Bitwidth;
}
-bool Type::isScalableTy() const {
+bool Type::isScalableTy(SmallPtrSetImpl<Type *> &Visited) const {
if (const auto *ATy = dyn_cast<ArrayType>(this))
- return ATy->getElementType()->isScalableTy();
- if (const auto *STy = dyn_cast<StructType>(this)) {
- SmallPtrSet<Type *, 4> Visited;
- return STy->isScalableTy(&Visited);
- }
+ return ATy->getElementType()->isScalableTy(Visited);
+ if (const auto *STy = dyn_cast<StructType>(this))
+ return STy->isScalableTy(Visited);
return getTypeID() == ScalableVectorTyID || isScalableTargetExtTy();
}
+bool Type::isScalableTy() const {
+ SmallPtrSet<Type *, 4> Visited;
+ return isScalableTy(Visited);
+}
+
const fltSemantics &Type::getFltSemantics() const {
switch (getTypeID()) {
case HalfTyID: return APFloat::IEEEhalf();
@@ -394,14 +397,14 @@ StructType *StructType::get(LLVMContext &Context, ArrayRef<Type*> ETypes,
return ST;
}
-bool StructType::isScalableTy(SmallPtrSetImpl<Type *> *Visited) const {
+bool StructType::isScalableTy(SmallPtrSetImpl<Type *> &Visited) const {
if ((getSubclassData() & SCDB_ContainsScalableVector) != 0)
return true;
if ((getSubclassData() & SCDB_NotContainsScalableVector) != 0)
return false;
- if (Visited && !Visited->insert(const_cast<StructType *>(this)).second)
+ if (!Visited.insert(const_cast<StructType *>(this)).second)
return false;
for (Type *Ty : elements()) {
>From dd30c6ef679e93000e9d71526f5502f94198b07f Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Wed, 23 Oct 2024 13:42:14 +0100
Subject: [PATCH 4/5] Fix StructType::isScalableTy by calling back into
Type::isScalableTy
---
llvm/lib/IR/Type.cpp | 9 +--------
llvm/test/Verifier/scalable-global-vars.ll | 14 ++++++++++++++
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index b09ac12b8ae947..db2de663e03f13 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -408,18 +408,11 @@ bool StructType::isScalableTy(SmallPtrSetImpl<Type *> &Visited) const {
return false;
for (Type *Ty : elements()) {
- if (isa<ScalableVectorType>(Ty)) {
+ if (Ty->isScalableTy(Visited)) {
const_cast<StructType *>(this)->setSubclassData(
getSubclassData() | SCDB_ContainsScalableVector);
return true;
}
- if (auto *STy = dyn_cast<StructType>(Ty)) {
- if (STy->isScalableTy(Visited)) {
- const_cast<StructType *>(this)->setSubclassData(
- getSubclassData() | SCDB_ContainsScalableVector);
- return true;
- }
- }
}
// For structures that are opaque, return false but do not set the
diff --git a/llvm/test/Verifier/scalable-global-vars.ll b/llvm/test/Verifier/scalable-global-vars.ll
index 81882261e664ef..fb9a3067acba98 100644
--- a/llvm/test/Verifier/scalable-global-vars.ll
+++ b/llvm/test/Verifier/scalable-global-vars.ll
@@ -15,3 +15,17 @@
; CHECK-NEXT: ptr @ScalableVecStructGlobal
@ScalableVecStructGlobal = global { i32, <vscale x 4 x i32> } zeroinitializer
+; CHECK-NEXT: Globals cannot contain scalable types
+; CHECK-NEXT: ptr @StructTestGlobal
+%struct.test = type { <vscale x 1 x double>, <vscale x 1 x double> }
+ at StructTestGlobal = global %struct.test zeroinitializer
+
+; CHECK-NEXT: Globals cannot contain scalable types
+; CHECK-NEXT: ptr @StructArrayTestGlobal
+%struct.array.test = type { [2 x <vscale x 1 x double>] }
+ at StructArrayTestGlobal = global %struct.array.test zeroinitializer
+
+; CHECK-NEXT: Globals cannot contain scalable types
+; CHECK-NEXT: ptr @StructTargetTestGlobal
+%struct.target.test = type { target("aarch64.svcount"), target("aarch64.svcount") }
+ at StructTargetTestGlobal = global %struct.target.test zeroinitializer
>From 2a2bb1cb66fbdad8485841294a450b7a0a9d043d Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Wed, 23 Oct 2024 13:54:11 +0100
Subject: [PATCH 5/5] Remove the need for one const_cast
---
llvm/include/llvm/IR/DerivedTypes.h | 2 +-
llvm/include/llvm/IR/Type.h | 2 +-
llvm/lib/IR/Type.cpp | 8 ++++----
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h
index 34a10874f51523..820b5c0707df6c 100644
--- a/llvm/include/llvm/IR/DerivedTypes.h
+++ b/llvm/include/llvm/IR/DerivedTypes.h
@@ -290,7 +290,7 @@ class StructType : public Type {
bool isSized(SmallPtrSetImpl<Type *> *Visited = nullptr) const;
/// Returns true if this struct contains a scalable vector.
- bool isScalableTy(SmallPtrSetImpl<Type *> &Visited) const;
+ bool isScalableTy(SmallPtrSetImpl<const Type *> &Visited) const;
using Type::isScalableTy;
/// Returns true if this struct contains homogeneous scalable vector types.
diff --git a/llvm/include/llvm/IR/Type.h b/llvm/include/llvm/IR/Type.h
index 3a6ead5dc7db25..d563b25d600a0c 100644
--- a/llvm/include/llvm/IR/Type.h
+++ b/llvm/include/llvm/IR/Type.h
@@ -206,7 +206,7 @@ class Type {
bool isScalableTargetExtTy() const;
/// Return true if this is a type whose size is a known multiple of vscale.
- bool isScalableTy(SmallPtrSetImpl<Type *> &Visited) const;
+ bool isScalableTy(SmallPtrSetImpl<const Type *> &Visited) const;
bool isScalableTy() const;
/// Return true if this is a FP type or a vector of FP.
diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index db2de663e03f13..912b1a3960ef19 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -58,7 +58,7 @@ bool Type::isIntegerTy(unsigned Bitwidth) const {
return isIntegerTy() && cast<IntegerType>(this)->getBitWidth() == Bitwidth;
}
-bool Type::isScalableTy(SmallPtrSetImpl<Type *> &Visited) const {
+bool Type::isScalableTy(SmallPtrSetImpl<const Type *> &Visited) const {
if (const auto *ATy = dyn_cast<ArrayType>(this))
return ATy->getElementType()->isScalableTy(Visited);
if (const auto *STy = dyn_cast<StructType>(this))
@@ -67,7 +67,7 @@ bool Type::isScalableTy(SmallPtrSetImpl<Type *> &Visited) const {
}
bool Type::isScalableTy() const {
- SmallPtrSet<Type *, 4> Visited;
+ SmallPtrSet<const Type *, 4> Visited;
return isScalableTy(Visited);
}
@@ -397,14 +397,14 @@ StructType *StructType::get(LLVMContext &Context, ArrayRef<Type*> ETypes,
return ST;
}
-bool StructType::isScalableTy(SmallPtrSetImpl<Type *> &Visited) const {
+bool StructType::isScalableTy(SmallPtrSetImpl<const Type *> &Visited) const {
if ((getSubclassData() & SCDB_ContainsScalableVector) != 0)
return true;
if ((getSubclassData() & SCDB_NotContainsScalableVector) != 0)
return false;
- if (!Visited.insert(const_cast<StructType *>(this)).second)
+ if (!Visited.insert(this).second)
return false;
for (Type *Ty : elements()) {
More information about the llvm-commits
mailing list