Adding interfaces for global ops used by the util dialect and fixing SinkGlobalBufferLoads. #10089
Conversation
This file contains 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
benvanik
added
the
compiler/dialects
benvanik
force-pushed
the
benvanik-global-interface
branch
2 times, most recently
from
33c28aa
to
4733991
Compare
This allows us to share more code between dialects using globals and generalizes the Explorer infrastructure to support global ops in any dialect (not just util.global).
This handles multiple uniform and non-uniform global stores that can arise after rodata deduplication. By using the explorer it's also able to track the rodata across block boundaries/calls made from initializers.
ScottTodd
approved these changes
Aug 15, 2022
Comment on lines
+25
to
+29
| // TODO(benvanik): replace this entire pass with generic IPO - the rodata refs | ||
| // are kind of constant like and should be trivial to inline, though they can't | ||
| // be ConstantLike and will need a new interface so that IPO can materialize | ||
| // ops. It's also possible we could use the dialect interface for materializing | ||
| // constants to do that, though. |
There was a problem hiding this comment.
😂 new pass with "replace this entire pass" comment, nice
There was a problem hiding this comment.
🐢 🐢 🐢 🐢 🐢
🐢 🐢 🐢 🐢
🐢 🐢 🐢
🐢 🐢
🐢
|
Looks like the existing VM tests were relying on canonicalization to constant evaluate everything instead of actually testing the globals at runtime 😨 Test at head after canonicalization, what ends up in the vmfb that's tested: vm.export @test_global_load_f32 attributes {ordinal = 0 : i32}
vm.func @test_global_load_f32() attributes {ordinal = 0 : i32} {
vm.return
}With changes in here that remove that canonicalization (as it was unsafe): vm.global.f32 private @c42 {ordinal = 0 : i32} = 4.250000e+01 : f32
vm.export @test_global_load_f32 attributes {ordinal = 0 : i32}
vm.func @test_global_load_f32() attributes {ordinal = 0 : i32} {
%c9 = vm.const.i32 9
%0 = vm.const.f32 4.250000e+01
%c42 = vm.global.load.f32 @c42 : f32
%1 = vm.cmp.ne.f32.o %c42, %0 : f32
vm.cond_br %1, ^bb2(%c9 : i32), ^bb1
^bb1: // pred: ^bb0
vm.return
^bb2(%2: i32): // pred: ^bb0
vm.fail %2, "@c42 != 42.5"
}This won't run yet as it needs global initialization and such - will see what's all needed and verify these really run. |
|
Ah.... |
There's some larger reworking here required to make the `vm` compilation modes work a little better: we want them to not change the IR too much but do want some of the nice passes like global inits.
benvanik
force-pushed
the
benvanik-global-interface
branch
from
4733991
to
825ac61
Compare
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.
The interfaces allow us to share more code between dialects using globals
and generalizes the Explorer infrastructure to support global ops in any
dialect (not just util.global). We could add external models for ml_program
or iree_input to run the optimization passes there. In this first step we are
only adding them to VM ops to be able to reuse the util passes there. In the
future we could go back to having
flow.global/stream.globaland such buttoday that's not needed.
This allows for the existing SinkGlobalBufferLoads pass to be reworked to use
the Explorer to perform IR traversal and fix some issues it had with multiple stores
found during #9945 (which after inlining produces multiple duplicate rodata ops).
Future IPO passes will also now be able to function on the VM dialect after
conversion.