This repository was archived by the owner on Jun 26, 2020. It is now read-only.
Legalize i64.const by breaking it into two i32.const#957
Merged
bnjbvr merged 2 commits intobytecodealliance:masterfrom Sep 10, 2019
Merged
Legalize i64.const by breaking it into two i32.const#957bnjbvr merged 2 commits intobytecodealliance:masterfrom
bnjbvr merged 2 commits intobytecodealliance:masterfrom
Conversation
bjorn3
approved these changes
Aug 30, 2019
Contributor
LLVM does that for both f32 and f64. black_box(0.1f64);[...]
movsd .LCPI10_0(%rip), %xmm0
callq core::hint::black_box
[...] |
Member
Author
|
Remaining to do: don't simplify operations into immediate operations if the ctrl value type is bigger than the native pointer type. |
abrown
approved these changes
Sep 10, 2019
…ry legalization; Converting something like iadd.i64 on a 32-bits architecture into a iadd_imm.i64 will result in the instruction being legalized back to an iadd.i64 later on, creating unnecessary churn. This commit implements avoid doing so, and changes the target ISA to a 64-bits platform for tests than ran into this, as well as making sure this won't happen on 32-bits platforms.
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
So my current thinking about isplit/iconcat is that both should not appear post-legalization, and every isplit must have an iconcat input, so the iconcat results are used instead. This seems to make sense if isplit/iconcat are just type system artifacts.
So trying a very simple program on x86 32-bits, that is generating an i64 constant, I implemented this legalization. This could be done in the meta language if there was literal support in the legalize patterns (cc @abrown).
Now it's funny because it interacts with f64const generation on 32 bits (so the test will fail, hence the WIP status of this PR). f64const are generated by reproducing the bit pattern in an i64, then bitcasting this to a f64 register. This is wrong on 32 bits platforms, where instead we should probably legalize it into two GPRs and then bitcast this to a f64. Or implement real constant pools for f32/f64, which would be even better.