Dev/wwolff/llvm cleanup 20240710 #100
Draft
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
wwolff42
force-pushed
the
dev/wwolff/llvm_cleanup_20240710
branch
5 times, most recently
from
37c19fd to
4d7d03f
Compare
…king of change made
The idea is to generate correct SSA form in EmitInstrWithCustomInserter Currently, we introduce arith+comp+branch, which seems to be wrong for SSA ala LLVM Loads of passes during reg_alloc/phi_elim morph def to use in those instruction it also put some stuff after the terminator for stack frame management and potentially put COPY before the def So this is a wip to actually keep correct SSA form at this stage. Todo so, we do the actual arith+cmp+branch with simple instruction, and add enough information to keep together/adjacent those instructions as we know they will be merge back together at later stage. Try to use metadata and implement the tweaker for LLVM internal passes.
… generic and handle naturally all cases
I first moved the trick stuff from DPUMergeComboInstrPass to a new DPUPostRAFusion because I wanted to have possible optim between RA and PreEmit::MergeComboInstr be effective. Along the way, I simply discover that we lose consistency of our def/use during analyzeBranch process for those arith+comp+jump: insertBranch and buildConditionalBranch was changing the instruction in a bad way. So, in fact we may don't even need to tweak shouldSink and don't need DPUPostRAFusion. And now I believe we don't kill really the SSA form, we just wasn't doing the right way. It was probably just bad branch reconstruction ... We always learn :)
…propagation and other optimizer
…GE cleanup now ... While at it, I implemented some quick optimization. - Implemented `(1 << n) - 1` == `lslx lneg n` #9 - Moved some 64-bit operation earlier in the pipeline, from ResolveMacroInstrPass to expandPostRA. I tried even earlier, but SUBC is not well defined and get moved around during MergeSink for critical edge split. The computation is then wrong because require to be packed for the particular use I identified. I leave my experiment here. I will check to fully define it and move it preRA, or will leave it like that again for a while, fulfilling my other duty. - Fixed a few easy Def/Use when BuildMI - Fixed lose of MachineInstr correctness Our arithmetic+comp+branch was destroyed during analyzeBranch/removeBranch/insertBranch - temporarily removed fusion of any instruction + JUMPi in MergeComboInstr The problem is that at this stage (PreEmit): - machine CFG is done. - JUMPi is unconditional jump - arithmetic + cond + branch; with cond as True/False is conditional -- even if we know that cond with true/false is unconditional, the instruction have the -- property of being conditional by its definition. ---- To fix that, I will create other PseudoInstruction to have them set correctly. ---- Also, if those arith+cond+branch do have pattern, maybe they could be selected far earlier ---- and the machine CFG would be correctly formed at the first place probably. - issue with ThinLTO fixed -- some code construction ended up in SELECTrr, which is not common for us ---- this is lowered to TmpJcc ---- and TmpJcc is kind of wrong ------ and finally, MergeComboInstr was combining even more wrong. --> I removed TmpJcc, and use simply our well defined JEQrii - issue whith ThinLTO fixed -- another was present but undetected on Release build -- we use multiple address spacees (IRAM, WRAM, MRAM) -- there was an assertion with ThinLTO when populating GV out of multiple modules ---- it's fixed in llvm13, but we are on llvm12 ------ I reproduced the patch (not cherry-picked) llvm@60c60dd ------ just for now. will do that correctly when cleaning up ------- so when I will upgrade our LLVM, it will be mergeable easily
wwolff42
force-pushed
the
dev/wwolff/llvm_cleanup_20240710
branch
from
bd40fcb to
45abde9
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.
No description provided.