[NFC]Update a ThinLTO test case #117584
Merged
[NFC]Update a ThinLTO test case #117584
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-lto Author: Mingming Liu (mingmingl-llvm) ChangesThe ThinLTO indexing step can import a function alias without importing its aliasee, making This patch updates the existing tests to reflect this. Full diff: https://github.com/llvm/llvm-project/pull/117584.diff 2 Files Affected:
diff --git a/llvm/test/ThinLTO/X86/distributed_indexes.ll b/llvm/test/ThinLTO/X86/distributed_indexes.ll
index 4f2662b1b34e1b..b70342fd0c96f5 100644
--- a/llvm/test/ThinLTO/X86/distributed_indexes.ll
+++ b/llvm/test/ThinLTO/X86/distributed_indexes.ll
@@ -48,6 +48,10 @@
; RUN: llvm-dis %t1.bc.thinlto.bc -o - | FileCheck %s --check-prefix=DIS
; DIS: aliasee: null
+; function-import pass crashed when alias is imported but aliasee doesn't.
+; TODO: Import both alias and aliasee, or neither of them.
+; RUN: not --crash opt -passes=function-import -summary-file=%t1.bc.thinlto.bc %t1.bc -o /dev/null 2>&1
+
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
declare void @g(...)
diff --git a/llvm/test/ThinLTO/X86/import_callee_declaration.ll b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
index 246920e5db0dc8..96257214c0a1af 100644
--- a/llvm/test/ThinLTO/X86/import_callee_declaration.ll
+++ b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
@@ -62,6 +62,10 @@
; these two GUIDs are declaration.
;
; RUN: llvm-dis main.bc.thinlto.bc -o - | FileCheck %s --check-prefix=MAIN-DIS
+
+; function-import pass crashed when alias is imported but aliasee doesn't.
+; TODO: Import both alias and aliasee, or neither of them.
+; RUN: not --crash opt -passes=function-import -summary-file=main.bc.thinlto.bc main.bc -o /dev/null 2>&1
;
; MAIN-DIS: [[LIBMOD:\^[0-9]+]] = module: (path: "lib.bc", hash: (0, 0, 0, 0, 0))
; MAIN-DIS: gv: (guid: 2418497564662708935, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
|
I'm confused as to what is happening here. The below crash is when we are computing imports, not after importing. The function importer should import an alias as a copy of its aliasee when it actually does the IR importing, but the below crash seems earlier. Hmm, it looks like you are trying to run the function import pass, which is just for testing, using the distributed ThinLTO index files. We shouldn't use these for computing imports. What are you trying to do here?
|
Ah, I assume you are trying to simulate a ThinLTO distributed backend. Add "-import-all-index" to your opt invocation. This will do what a ThinLTO distributed backend will do and blindly follow the importing decisions encoded in the sharded index file, rather than trying to recompute the import decisions. See for example the invocations in llvm/test/ThinLTO/X86/distributed_import.ll.
|
…dexes.ll. Revert changes to import_callee_declaration.ll
Yes, this is exactly what
Thanks! I added |
Sort of, it only simulates a distributed ThinLTO backend with "-import-all-index". Without that it is expecting a combined index, and makes importing decisions on the original index.
|
| @@ -48,6 +48,13 @@ | |||
| ; RUN: llvm-dis %t1.bc.thinlto.bc -o - | FileCheck %s --check-prefix=DIS | |||
| ; DIS: aliasee: null | |||
|
|
|||
| ; function-import pass crashed when alias is imported but aliasee doesn't. | |||
There was a problem hiding this comment.
I think the comment and TODO can now be removed?
There was a problem hiding this comment.
Thanks for the catch! Removed the outdated comments.
teresajohnson
approved these changes
Dec 3, 2024
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.
Run
function-importpass in a similar way that it runs in a ThinLTO distributed backend compile and checks that function alias is imported as a definition. More accurately, the function alias is a clone of the aliasee function.