-
Notifications
You must be signed in to change notification settings - Fork 17
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
[EraVM] Enable MachineCopyPropagation optimization #712
Conversation
8a612ce
to
66f8f98
Compare
|
66f8f98
to
b653cb6
Compare
One regression test is failing for 20c39f6 commit, since it introduces two parameters in |
Theoretically, we could use a pre-commit XFAIL and re-enable it later, but since we periodically update LLVM and rebase our commits on top of theirs, I recommend not worrying about it. We have admin privileges to bypass the CI if needed. |
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.
Could we add tests to
[EraVM][EVM] Add renamable bit to copyPhysReg
[EraVM] Enable MachineCopyPropagation optimization
.
Also links to upstream counterparts in the commit messages are very welcome for future LLVM updates.
This is the name of the PR, and there is no such commit. Did you have some other commit in your mind?
|
@vladimirradosavljevic Sorry, this one: |
Copy paste typo. Did you mean |
b653cb6
to
c9ab6d4
Compare
Added test for |
Once we modernize CopyInfo with default member initializations, Copies.insert({Unit, ...}) becomes equivalent to: Copies.try_emplace(Unit) which we can simplify further down to Copies[Unit].
Previously we wouldn't remove dead copies from basic blocks with successors. The comment said we didn't want to trust the live-in lists. The comment is very old so I'm not sure if that's still a concern today. This patch checks the live-in lists and removes copies from MaybeDeadCopies if they are referenced by any live-ins in any successors. We only do this if the tracksLiveness property is set. If that property is not set, we retain the old behavior.
Tail duplication will generate the redundant move before return. It is because the MachineCopyPropogation can't recognize COPY after post-RA pseudoExpand. This patch make MachineCopyPropogation recognize `%0 = ADDI %1, 0` as COPY
…EIMM Some optimizations like MachineCopyPropagation makes decisions based on the register flags, so in order for these optimizations to work correctly, we need to preserve flags after expansion of pseudo instructions. Signed-off-by: Vladimir Radosavljevic <[email protected]>
Even though in tests registers are marked with renamable flag, MachineOperand::isRenamable returns false for them, since AllowRegisterRenaming is 0 by default. Signed-off-by: Vladimir Radosavljevic <[email protected]>
MachineCopyPropagation optimization only works if registers are renamable, so we need to allow register renaming to enable this optimization for EraVM. Signed-off-by: Vladimir Radosavljevic <[email protected]>
The renamable flag is useful during MachineCopyPropagation but renamable flag will be dropped after lowerCopy in some case. This patch introduces extra arguments to pass the renamable flag to copyPhysReg.
Signed-off-by: Vladimir Radosavljevic <[email protected]>
…ore time Signed-off-by: Vladimir Radosavljevic <[email protected]>
Run MCP one more time after pseudo expansion, since we can produce copy like instructions (e.g. from PTR_TO_INT) that can be optimized. This patch implements isCopyInstrImpl target hook which is called from MCP to detect copy instructions after expansion from COPY is performed. Signed-off-by: Vladimir Radosavljevic <[email protected]>
…g forward propagation Signed-off-by: Vladimir Radosavljevic <[email protected]>
Before this patch, redundant COPY couldn't be removed for the following case: %reg1 = COPY %const-reg ... // No use of %reg1 but there is a def/use of %const-reg %reg2 = COPY killed %reg1 where this can be optimized to: ... // No use of %reg1 but there is a def/use of %const-reg %reg2 = COPY %const-reg This patch allows for such optimization by not invalidating constant registers. This is safe even where constant registers are defined, as architectures like AArch64 and RISCV replace a dead definition of a GPR with a zero constant register for certain instructions. Upstream PR: llvm/llvm-project#111129 Signed-off-by: Vladimir Radosavljevic <[email protected]>
…ng backward propagation Signed-off-by: Vladimir Radosavljevic <[email protected]>
Before this patch, redundant COPY couldn't be removed for the following case: $R0 = OP ... ... // Read of %R0 $R1 = COPY killed $R0 This patch adds support for tracking the users of the source register during backward propagation, so that we can remove the redundant COPY in the above case and optimize it to: $R1 = OP ... ... // Replace all uses of %R0 with $R1 Upstream PR: llvm/llvm-project#111130 Signed-off-by: Vladimir Radosavljevic <[email protected]>
c9ab6d4
to
6b0b014
Compare
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, thanks!
Will wait for the rest of the checkers and then merge.
This PR enables MachineCopyPropagation for EraVM (by adding
let AllowRegisterRenaming = 1
inEraVM.td
), cherry-picks some improvements and adds some more improvements.