[llvm] [IR] Check parameters of target extension types on construction (PR #107268)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 5 02:44:53 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Jay Foad (jayfoad)
<details>
<summary>Changes</summary>
Since IR Types are immutable it makes sense to check them on
construction instead of in the IR Verifier pass.
This patch checks that some TargetExtTypes are well-formed in the sense
that they have the expected number of type parameters and integer
parameters. When called from LLParser it gives a diagnostic message.
When called from anywhere else it just asserts that they are
well-formed.
---
Full diff: https://github.com/llvm/llvm-project/pull/107268.diff
4 Files Affected:
- (modified) llvm/include/llvm/IR/DerivedTypes.h (+14)
- (modified) llvm/lib/AsmParser/LLParser.cpp (+6-1)
- (modified) llvm/lib/IR/Type.cpp (+30-7)
- (added) llvm/test/Assembler/target-type-param-errors.ll (+12)
``````````diff
diff --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h
index 01f76d49327808..0c8cbe1921ac9b 100644
--- a/llvm/include/llvm/IR/DerivedTypes.h
+++ b/llvm/include/llvm/IR/DerivedTypes.h
@@ -32,6 +32,7 @@ namespace llvm {
class Value;
class APInt;
class LLVMContext;
+template <typename T> class Expected;
/// Class to represent integer types. Note that this class is also used to
/// represent the built-in integer types: Int1Ty, Int8Ty, Int16Ty, Int32Ty and
@@ -735,6 +736,19 @@ class TargetExtType : public Type {
ArrayRef<Type *> Types = std::nullopt,
ArrayRef<unsigned> Ints = std::nullopt);
+ /// Return a target extension type having the specified name and optional
+ /// type and integer parameters, or an appropriate Error if it fails the
+ /// parameters check.
+ static Expected<TargetExtType *>
+ getOrError(LLVMContext &Context, StringRef Name,
+ ArrayRef<Type *> Types = std::nullopt,
+ ArrayRef<unsigned> Ints = std::nullopt);
+
+ /// Check that a newly created target extension type has the expected number
+ /// of type parameters and integer parameters, returning the type itself if OK
+ /// or an appropriate Error if not.
+ static Expected<TargetExtType *> checkParams(TargetExtType *TTy);
+
/// Return the name for this target extension type. Two distinct target
/// extension types may have the same name if their type or integer parameters
/// differ.
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index f41907f0351257..93dc2bd241581b 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -3530,7 +3530,12 @@ bool LLParser::parseTargetExtType(Type *&Result) {
if (parseToken(lltok::rparen, "expected ')' in target extension type"))
return true;
- Result = TargetExtType::get(Context, TypeName, TypeParams, IntParams);
+ auto TTy =
+ TargetExtType::getOrError(Context, TypeName, TypeParams, IntParams);
+ if (auto E = TTy.takeError())
+ return tokError(toString(std::move(E)));
+
+ Result = *TTy;
return false;
}
diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index 753aa5fd2118e5..0bb9b282385ac7 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -22,6 +22,7 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Value.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/TypeSize.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/RISCVTargetParser.h"
@@ -792,6 +793,13 @@ TargetExtType::TargetExtType(LLVMContext &C, StringRef Name,
TargetExtType *TargetExtType::get(LLVMContext &C, StringRef Name,
ArrayRef<Type *> Types,
ArrayRef<unsigned> Ints) {
+ return cantFail(getOrError(C, Name, Types, Ints));
+}
+
+Expected<TargetExtType *> TargetExtType::getOrError(LLVMContext &C,
+ StringRef Name,
+ ArrayRef<Type *> Types,
+ ArrayRef<unsigned> Ints) {
const TargetExtTypeKeyInfo::KeyTy Key(Name, Types, Ints);
TargetExtType *TT;
// Since we only want to allocate a fresh target type in case none is found
@@ -799,8 +807,8 @@ TargetExtType *TargetExtType::get(LLVMContext &C, StringRef Name,
// one for inserting the newly allocated one), here we instead lookup based on
// Key and update the reference to the target type in-place to a newly
// allocated one if not found.
- auto Insertion = C.pImpl->TargetExtTypes.insert_as(nullptr, Key);
- if (Insertion.second) {
+ auto [Iter, Inserted] = C.pImpl->TargetExtTypes.insert_as(nullptr, Key);
+ if (Inserted) {
// The target type was not found. Allocate one and update TargetExtTypes
// in-place.
TT = (TargetExtType *)C.pImpl->Alloc.Allocate(
@@ -808,12 +816,27 @@ TargetExtType *TargetExtType::get(LLVMContext &C, StringRef Name,
sizeof(unsigned) * Ints.size(),
alignof(TargetExtType));
new (TT) TargetExtType(C, Name, Types, Ints);
- *Insertion.first = TT;
- } else {
- // The target type was found. Just return it.
- TT = *Insertion.first;
+ *Iter = TT;
+ return checkParams(TT);
}
- return TT;
+
+ // The target type was found. Just return it.
+ return *Iter;
+}
+
+Expected<TargetExtType *> TargetExtType::checkParams(TargetExtType *TTy) {
+ // Opaque types in the AArch64 name space.
+ if (TTy->Name == "aarch64.svcount" &&
+ (TTy->getNumTypeParameters() != 0 || TTy->getNumIntParameters() != 0))
+ return createStringError("Target type should have no parameters");
+
+ // Opaque types in the RISC-V name space.
+ if (TTy->Name == "riscv.vector.tuple" &&
+ (TTy->getNumTypeParameters() != 1 || TTy->getNumIntParameters() != 1))
+ return createStringError(
+ "Target type should have one type parameter and one integer parameter");
+
+ return TTy;
}
namespace {
diff --git a/llvm/test/Assembler/target-type-param-errors.ll b/llvm/test/Assembler/target-type-param-errors.ll
new file mode 100644
index 00000000000000..a356a0804fafa7
--- /dev/null
+++ b/llvm/test/Assembler/target-type-param-errors.ll
@@ -0,0 +1,12 @@
+; RUN: split-file %s %t
+; RUN: not llvm-as < %t/aarch64-svcount.ll -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-AARCH64-SVCOUNT %s
+; RUN: not llvm-as < %t/riscv-vector-tuple.ll -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-RISCV-VECTOR-TUPLE %s
+; Check target extension type properties are verified in the assembler.
+
+;--- aarch64-svcount.ll
+declare target("aarch64.svcount", i32) @aarch64_svcount()
+; CHECK-AARCH64-SVCOUNT: error: Target type should have no parameters
+
+;--- riscv-vector-tuple.ll
+declare target("riscv.vector.tuple", 99) @riscv_vector_tuple()
+; CHECK-RISCV-VECTOR-TUPLE: Target type should have one type parameter and one integer parameter
``````````
</details>
https://github.com/llvm/llvm-project/pull/107268
More information about the llvm-commits
mailing list