-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[MLIR][LLVM] Fix the import of LLVM IR metadata #170631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Change `getSupportedMetadata` to return `SmallVector<unsigned>` instead of `ArrayRef<unsigned>` and make the list non-static. This ensures metadata identifiers are correctly obtained per LLVM context, preventing incorrect import when multiple contexts are used (for metadata like vector hints or work group sizes which have non-static IDs).
|
@llvm/pr-subscribers-mlir Author: Tobias Gysi (gysit) ChangesChange Full diff: https://github.com/llvm/llvm-project/pull/170631.diff 2 Files Affected:
diff --git a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
index 6a42627e17e60..0e50fac7e85dd 100644
--- a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
+++ b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
@@ -84,10 +84,14 @@ class LLVMImportDialectInterface
/// Hook for derived dialect interfaces to publish the supported metadata
/// kinds. As every metadata kind has a unique integer identifier, the
- /// function returns the list of supported metadata identifiers. `ctx` can be
- /// used to obtain IDs of metadata kinds that do not have a fixed static one.
- virtual ArrayRef<unsigned>
- getSupportedMetadata(llvm::LLVMContext &ctx) const {
+ /// function returns the list of supported metadata identifiers. The
+ /// `llvmContext` parameter is used to obtain identifiers for metadata kinds
+ /// that do not have a fixed static identifier. Since different LLVM contexts
+ /// can assign different identifiers to these non-static metadata kinds, the
+ /// function must recompute the list of supported metadata identifiers on each
+ /// call.
+ virtual SmallVector<unsigned>
+ getSupportedMetadata(llvm::LLVMContext &llvmContext) const {
return {};
}
};
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
index 81c9da1d98c40..2d4a18cc4b145 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
@@ -80,8 +80,9 @@ static LogicalResult convertIntrinsicImpl(OpBuilder &odsBuilder,
/// Returns the list of LLVM IR metadata kinds that are convertible to MLIR LLVM
/// dialect attributes.
-static ArrayRef<unsigned> getSupportedMetadataImpl(llvm::LLVMContext &context) {
- static const SmallVector<unsigned> convertibleMetadata = {
+static SmallVector<unsigned>
+getSupportedMetadataImpl(llvm::LLVMContext &llvmContext) {
+ SmallVector<unsigned> convertibleMetadata = {
llvm::LLVMContext::MD_prof,
llvm::LLVMContext::MD_tbaa,
llvm::LLVMContext::MD_access_group,
@@ -91,10 +92,10 @@ static ArrayRef<unsigned> getSupportedMetadataImpl(llvm::LLVMContext &context) {
llvm::LLVMContext::MD_dereferenceable,
llvm::LLVMContext::MD_dereferenceable_or_null,
llvm::LLVMContext::MD_mmra,
- context.getMDKindID(vecTypeHintMDName),
- context.getMDKindID(workGroupSizeHintMDName),
- context.getMDKindID(reqdWorkGroupSizeMDName),
- context.getMDKindID(intelReqdSubGroupSizeMDName)};
+ llvmContext.getMDKindID(vecTypeHintMDName),
+ llvmContext.getMDKindID(workGroupSizeHintMDName),
+ llvmContext.getMDKindID(reqdWorkGroupSizeMDName),
+ llvmContext.getMDKindID(intelReqdSubGroupSizeMDName)};
return convertibleMetadata;
}
@@ -505,9 +506,9 @@ class LLVMDialectLLVMIRImportInterface : public LLVMImportDialectInterface {
/// Returns the list of LLVM IR metadata kinds that are convertible to MLIR
/// LLVM dialect attributes.
- ArrayRef<unsigned>
- getSupportedMetadata(llvm::LLVMContext &context) const final {
- return getSupportedMetadataImpl(context);
+ SmallVector<unsigned>
+ getSupportedMetadata(llvm::LLVMContext &llvmContext) const final {
+ return getSupportedMetadataImpl(llvmContext);
}
};
} // namespace
|
|
@llvm/pr-subscribers-mlir-llvm Author: Tobias Gysi (gysit) ChangesChange Full diff: https://github.com/llvm/llvm-project/pull/170631.diff 2 Files Affected:
diff --git a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
index 6a42627e17e60..0e50fac7e85dd 100644
--- a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
+++ b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
@@ -84,10 +84,14 @@ class LLVMImportDialectInterface
/// Hook for derived dialect interfaces to publish the supported metadata
/// kinds. As every metadata kind has a unique integer identifier, the
- /// function returns the list of supported metadata identifiers. `ctx` can be
- /// used to obtain IDs of metadata kinds that do not have a fixed static one.
- virtual ArrayRef<unsigned>
- getSupportedMetadata(llvm::LLVMContext &ctx) const {
+ /// function returns the list of supported metadata identifiers. The
+ /// `llvmContext` parameter is used to obtain identifiers for metadata kinds
+ /// that do not have a fixed static identifier. Since different LLVM contexts
+ /// can assign different identifiers to these non-static metadata kinds, the
+ /// function must recompute the list of supported metadata identifiers on each
+ /// call.
+ virtual SmallVector<unsigned>
+ getSupportedMetadata(llvm::LLVMContext &llvmContext) const {
return {};
}
};
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
index 81c9da1d98c40..2d4a18cc4b145 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
@@ -80,8 +80,9 @@ static LogicalResult convertIntrinsicImpl(OpBuilder &odsBuilder,
/// Returns the list of LLVM IR metadata kinds that are convertible to MLIR LLVM
/// dialect attributes.
-static ArrayRef<unsigned> getSupportedMetadataImpl(llvm::LLVMContext &context) {
- static const SmallVector<unsigned> convertibleMetadata = {
+static SmallVector<unsigned>
+getSupportedMetadataImpl(llvm::LLVMContext &llvmContext) {
+ SmallVector<unsigned> convertibleMetadata = {
llvm::LLVMContext::MD_prof,
llvm::LLVMContext::MD_tbaa,
llvm::LLVMContext::MD_access_group,
@@ -91,10 +92,10 @@ static ArrayRef<unsigned> getSupportedMetadataImpl(llvm::LLVMContext &context) {
llvm::LLVMContext::MD_dereferenceable,
llvm::LLVMContext::MD_dereferenceable_or_null,
llvm::LLVMContext::MD_mmra,
- context.getMDKindID(vecTypeHintMDName),
- context.getMDKindID(workGroupSizeHintMDName),
- context.getMDKindID(reqdWorkGroupSizeMDName),
- context.getMDKindID(intelReqdSubGroupSizeMDName)};
+ llvmContext.getMDKindID(vecTypeHintMDName),
+ llvmContext.getMDKindID(workGroupSizeHintMDName),
+ llvmContext.getMDKindID(reqdWorkGroupSizeMDName),
+ llvmContext.getMDKindID(intelReqdSubGroupSizeMDName)};
return convertibleMetadata;
}
@@ -505,9 +506,9 @@ class LLVMDialectLLVMIRImportInterface : public LLVMImportDialectInterface {
/// Returns the list of LLVM IR metadata kinds that are convertible to MLIR
/// LLVM dialect attributes.
- ArrayRef<unsigned>
- getSupportedMetadata(llvm::LLVMContext &context) const final {
- return getSupportedMetadataImpl(context);
+ SmallVector<unsigned>
+ getSupportedMetadata(llvm::LLVMContext &llvmContext) const final {
+ return getSupportedMetadataImpl(llvmContext);
}
};
} // namespace
|
Dinistro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Change `getSupportedMetadata` to return `SmallVector<unsigned>` instead of `ArrayRef<unsigned>` and make the list non-static. This ensures metadata identifiers are correctly obtained per LLVM context, preventing incorrect import when multiple contexts are used (for metadata like vector hints or work group sizes which have non-static IDs).
Change
getSupportedMetadatato returnSmallVector<unsigned>instead ofArrayRef<unsigned>and make the list non-static. This ensures metadata identifiers are correctly obtained per LLVM context, preventing incorrect import when multiple contexts are used (for metadata like vector hints or work group sizes which have non-static IDs).