[mlir][llvm] Align linkage enum order with LLVM (NFC) #118484
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-mlir-llvm Author: Henrich Lauko (xlauko) ChangesFull diff: https://github.com/llvm/llvm-project/pull/118484.diff 2 Files Affected:
diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h
index 0e6434073437a5..ed9b23c343150c 100644
--- a/mlir/include/mlir-c/Dialect/LLVM.h
+++ b/mlir/include/mlir-c/Dialect/LLVM.h
@@ -175,17 +175,17 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMComdatAttrGet(MlirContext ctx,
MlirLLVMComdat comdat);
enum MlirLLVMLinkage {
- MlirLLVMLinkagePrivate = 0,
- MlirLLVMLinkageInternal = 1,
- MlirLLVMLinkageAvailableExternally = 2,
- MlirLLVMLinkageLinkonce = 3,
+ MlirLLVMLinkageExternal = 0,
+ MlirLLVMLinkageAvailableExternally = 1,
+ MlirLLVMLinkageLinkonce = 2,
+ MlirLLVMLinkageLinkonceODR = 3,
MlirLLVMLinkageWeak = 4,
- MlirLLVMLinkageCommon = 5,
+ MlirLLVMLinkageWeakODR = 5,
MlirLLVMLinkageAppending = 6,
- MlirLLVMLinkageExternWeak = 7,
- MlirLLVMLinkageLinkonceODR = 8,
- MlirLLVMLinkageWeakODR = 9,
- MlirLLVMLinkageExternal = 10,
+ MlirLLVMLinkageInternal = 7,
+ MlirLLVMLinkagePrivate = 8,
+ MlirLLVMLinkageExternWeak = 9,
+ MlirLLVMLinkageCommon = 10,
};
typedef enum MlirLLVMLinkage MlirLLVMLinkage;
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
index 4a43c16903394f..c08b75de036473 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
@@ -615,40 +615,40 @@ def ICmpPredicate : LLVM_EnumAttr<
//===----------------------------------------------------------------------===//
// Linkage attribute is used on functions and globals. The order follows that of
-// https://llvm.org/docs/LangRef.html#linkage-types. The names are equivalent to
-// visible names in the IR rather than to enum values names in llvm::GlobalValue
-// since the latter is easier to change.
-def LinkagePrivate
- : LLVM_EnumAttrCase<"Private", "private", "PrivateLinkage", 0>;
-def LinkageInternal
- : LLVM_EnumAttrCase<"Internal", "internal", "InternalLinkage", 1>;
+// llvm::GlobalValue::LinkageTypes from llvm/IR/GlobalValue.h. The names are
+// equivalent to visible names in the IR rather than to enum values names in
+// llvm::GlobalValue since the latter is easier to change.
+def LinkageExternal
+ : LLVM_EnumAttrCase<"External", "external", "ExternalLinkage", 0>;
def LinkageAvailableExternally
: LLVM_EnumAttrCase<"AvailableExternally", "available_externally",
- "AvailableExternallyLinkage", 2>;
+ "AvailableExternallyLinkage", 1>;
def LinkageLinkonce
- : LLVM_EnumAttrCase<"Linkonce", "linkonce", "LinkOnceAnyLinkage", 3>;
+ : LLVM_EnumAttrCase<"Linkonce", "linkonce", "LinkOnceAnyLinkage", 2>;
+def LinkageLinkonceODR
+ : LLVM_EnumAttrCase<"LinkonceODR", "linkonce_odr", "LinkOnceODRLinkage", 3>;
def LinkageWeak
: LLVM_EnumAttrCase<"Weak", "weak", "WeakAnyLinkage", 4>;
-def LinkageCommon
- : LLVM_EnumAttrCase<"Common", "common", "CommonLinkage", 5>;
+def LinkageWeakODR
+ : LLVM_EnumAttrCase<"WeakODR", "weak_odr", "WeakODRLinkage", 5>;
def LinkageAppending
: LLVM_EnumAttrCase<"Appending", "appending", "AppendingLinkage", 6>;
+def LinkageInternal
+ : LLVM_EnumAttrCase<"Internal", "internal", "InternalLinkage", 7>;
+def LinkagePrivate
+ : LLVM_EnumAttrCase<"Private", "private", "PrivateLinkage", 8>;
def LinkageExternWeak
- : LLVM_EnumAttrCase<"ExternWeak", "extern_weak", "ExternalWeakLinkage", 7>;
-def LinkageLinkonceODR
- : LLVM_EnumAttrCase<"LinkonceODR", "linkonce_odr", "LinkOnceODRLinkage", 8>;
-def LinkageWeakODR
- : LLVM_EnumAttrCase<"WeakODR", "weak_odr", "WeakODRLinkage", 9>;
-def LinkageExternal
- : LLVM_EnumAttrCase<"External", "external", "ExternalLinkage", 10>;
+ : LLVM_EnumAttrCase<"ExternWeak", "extern_weak", "ExternalWeakLinkage", 9>;
+def LinkageCommon
+ : LLVM_EnumAttrCase<"Common", "common", "CommonLinkage", 10>;
def LinkageEnum : LLVM_EnumAttr<
"Linkage",
"::llvm::GlobalValue::LinkageTypes",
"LLVM linkage types",
- [LinkagePrivate, LinkageInternal, LinkageAvailableExternally,
- LinkageLinkonce, LinkageWeak, LinkageCommon, LinkageAppending,
- LinkageExternWeak, LinkageLinkonceODR, LinkageWeakODR, LinkageExternal]> {
+ [LinkageExternal, LinkageAvailableExternally, LinkageLinkonce,
+ LinkageLinkonceODR, LinkageWeak, LinkageWeakODR, LinkageAppending,
+ LinkageInternal, LinkagePrivate, LinkageExternWeak, LinkageCommon]> {
let cppNamespace = "::mlir::LLVM::linkage";
}
|
|
@llvm/pr-subscribers-mlir Author: Henrich Lauko (xlauko) ChangesFull diff: https://github.com/llvm/llvm-project/pull/118484.diff 2 Files Affected:
diff --git a/mlir/include/mlir-c/Dialect/LLVM.h b/mlir/include/mlir-c/Dialect/LLVM.h
index 0e6434073437a5..ed9b23c343150c 100644
--- a/mlir/include/mlir-c/Dialect/LLVM.h
+++ b/mlir/include/mlir-c/Dialect/LLVM.h
@@ -175,17 +175,17 @@ MLIR_CAPI_EXPORTED MlirAttribute mlirLLVMComdatAttrGet(MlirContext ctx,
MlirLLVMComdat comdat);
enum MlirLLVMLinkage {
- MlirLLVMLinkagePrivate = 0,
- MlirLLVMLinkageInternal = 1,
- MlirLLVMLinkageAvailableExternally = 2,
- MlirLLVMLinkageLinkonce = 3,
+ MlirLLVMLinkageExternal = 0,
+ MlirLLVMLinkageAvailableExternally = 1,
+ MlirLLVMLinkageLinkonce = 2,
+ MlirLLVMLinkageLinkonceODR = 3,
MlirLLVMLinkageWeak = 4,
- MlirLLVMLinkageCommon = 5,
+ MlirLLVMLinkageWeakODR = 5,
MlirLLVMLinkageAppending = 6,
- MlirLLVMLinkageExternWeak = 7,
- MlirLLVMLinkageLinkonceODR = 8,
- MlirLLVMLinkageWeakODR = 9,
- MlirLLVMLinkageExternal = 10,
+ MlirLLVMLinkageInternal = 7,
+ MlirLLVMLinkagePrivate = 8,
+ MlirLLVMLinkageExternWeak = 9,
+ MlirLLVMLinkageCommon = 10,
};
typedef enum MlirLLVMLinkage MlirLLVMLinkage;
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
index 4a43c16903394f..c08b75de036473 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
@@ -615,40 +615,40 @@ def ICmpPredicate : LLVM_EnumAttr<
//===----------------------------------------------------------------------===//
// Linkage attribute is used on functions and globals. The order follows that of
-// https://llvm.org/docs/LangRef.html#linkage-types. The names are equivalent to
-// visible names in the IR rather than to enum values names in llvm::GlobalValue
-// since the latter is easier to change.
-def LinkagePrivate
- : LLVM_EnumAttrCase<"Private", "private", "PrivateLinkage", 0>;
-def LinkageInternal
- : LLVM_EnumAttrCase<"Internal", "internal", "InternalLinkage", 1>;
+// llvm::GlobalValue::LinkageTypes from llvm/IR/GlobalValue.h. The names are
+// equivalent to visible names in the IR rather than to enum values names in
+// llvm::GlobalValue since the latter is easier to change.
+def LinkageExternal
+ : LLVM_EnumAttrCase<"External", "external", "ExternalLinkage", 0>;
def LinkageAvailableExternally
: LLVM_EnumAttrCase<"AvailableExternally", "available_externally",
- "AvailableExternallyLinkage", 2>;
+ "AvailableExternallyLinkage", 1>;
def LinkageLinkonce
- : LLVM_EnumAttrCase<"Linkonce", "linkonce", "LinkOnceAnyLinkage", 3>;
+ : LLVM_EnumAttrCase<"Linkonce", "linkonce", "LinkOnceAnyLinkage", 2>;
+def LinkageLinkonceODR
+ : LLVM_EnumAttrCase<"LinkonceODR", "linkonce_odr", "LinkOnceODRLinkage", 3>;
def LinkageWeak
: LLVM_EnumAttrCase<"Weak", "weak", "WeakAnyLinkage", 4>;
-def LinkageCommon
- : LLVM_EnumAttrCase<"Common", "common", "CommonLinkage", 5>;
+def LinkageWeakODR
+ : LLVM_EnumAttrCase<"WeakODR", "weak_odr", "WeakODRLinkage", 5>;
def LinkageAppending
: LLVM_EnumAttrCase<"Appending", "appending", "AppendingLinkage", 6>;
+def LinkageInternal
+ : LLVM_EnumAttrCase<"Internal", "internal", "InternalLinkage", 7>;
+def LinkagePrivate
+ : LLVM_EnumAttrCase<"Private", "private", "PrivateLinkage", 8>;
def LinkageExternWeak
- : LLVM_EnumAttrCase<"ExternWeak", "extern_weak", "ExternalWeakLinkage", 7>;
-def LinkageLinkonceODR
- : LLVM_EnumAttrCase<"LinkonceODR", "linkonce_odr", "LinkOnceODRLinkage", 8>;
-def LinkageWeakODR
- : LLVM_EnumAttrCase<"WeakODR", "weak_odr", "WeakODRLinkage", 9>;
-def LinkageExternal
- : LLVM_EnumAttrCase<"External", "external", "ExternalLinkage", 10>;
+ : LLVM_EnumAttrCase<"ExternWeak", "extern_weak", "ExternalWeakLinkage", 9>;
+def LinkageCommon
+ : LLVM_EnumAttrCase<"Common", "common", "CommonLinkage", 10>;
def LinkageEnum : LLVM_EnumAttr<
"Linkage",
"::llvm::GlobalValue::LinkageTypes",
"LLVM linkage types",
- [LinkagePrivate, LinkageInternal, LinkageAvailableExternally,
- LinkageLinkonce, LinkageWeak, LinkageCommon, LinkageAppending,
- LinkageExternWeak, LinkageLinkonceODR, LinkageWeakODR, LinkageExternal]> {
+ [LinkageExternal, LinkageAvailableExternally, LinkageLinkonce,
+ LinkageLinkonceODR, LinkageWeak, LinkageWeakODR, LinkageAppending,
+ LinkageInternal, LinkagePrivate, LinkageExternWeak, LinkageCommon]> {
let cppNamespace = "::mlir::LLVM::linkage";
}
|
|
@gysit can you review and merge this? |
|
@Dinistro I'm refactoring the linkage code in preparation for future |
gysit
approved these changes
Dec 3, 2024
There was a problem hiding this comment.
Thanks LGTM.
We usually use a commit message of the form:
[mlir][llvm] Fix linkage enum order
To clarify that the change is related to MLIR's LLVM dialect. Also the title of the commit should not exceed 72 characters. The commit message can then further detail what the revision is changing. I guess the main argument here is to have the same enum order as in LLVM proper.
xlauko
changed the title
xlauko
force-pushed
the
xlauko/mlir-llvmir-linkage-order
branch
from
b6e7122 to
fda07bd
Compare
|
@gysit fixed it |
I would instead use "NFC" and avoid the word "fix" here, otherwise I'd want a test. Maybe:
|
This change doesn't introduce any functional differences but aligns the MLIR Linkage EnumAttr with LLVM's representation order. Previously, the code generated a lookup table to map MLIR enums to LLVM enums due to the lack of one-to-one correspondence. With this refactoring, the generated code now casts directly from one enum to another.
xlauko
force-pushed
the
xlauko/mlir-llvmir-linkage-order
branch
from
fda07bd to
6a91c64
Compare
xlauko
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change doesn't introduce any functional differences but aligns the implementation more closely with LLVM's representation. Previously, the code generated a lookup table to map MLIR enums to LLVM enums due to the lack of one-to-one correspondence. With this refactoring, the generated code now casts directly from one enum to another.