[PATCH] D122533: [avr] Remove AVRRelaxMemOperations

Patryk27 created this revision. Herald added subscribers: Jim, hiraditya, mgorny, dylanmckay. Herald added a project: All. Patryk27 requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This commit contains a refactoring that me

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 updated this revision to Diff 418413. Patryk27 retitled this revision from "[avr] Remove AVRRelaxMemOperations" to "[AVR] Remove AVRRelaxMemOperations". Patryk27 added a comment. avr -> AVR Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 added a subscriber: benshi001. Patryk27 added a comment. cc @benshi001 (from https://reviews.llvm.org/D114611) 🙂 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122533/new/ https://reviews.llvm.org/D122533 __

[PATCH] D114611: [AVR] Expand STDWSPQRr & STDSPQRr, approach #2

Patryk27 marked 6 inline comments as done. Patryk27 added a comment. Thanks for the review! For the time being, I have extracted the first set of changes into: https://reviews.llvm.org/D122533 Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1234 + + MachineOperand &D

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 added a comment. Hmm, that's weird - I've just re-checked and everything's working correctly on my side; maybe you're testing it on an older LLVM revision? (for reference, my patch is based off of the current LLVM's main branch, which - at the time of writing this comment - is the d9ce

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 added inline comments. Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1174 - auto MIBLO = buildMI(MBB, MBBI, OpLo) - .addReg(DstReg) - .addImm(Imm) - .addReg(SrcLoReg, getKillRegState(SrcIsKill)); +bu

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 added a comment. I think I'm using `=Release` instead of `=Debug`, that would explain the difference, yeah; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122533/new/ https://reviews.llvm.org/D122533 __

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 updated this revision to Diff 418443. Patryk27 added a comment. SBCI -> SUBI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122533/new/ https://reviews.llvm.org/D122533 Files: clang/docs/tools/clang-formatted-files.txt llvm/lib/Target/

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 marked 2 inline comments as done. Patryk27 added inline comments. Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1174 - auto MIBLO = buildMI(MBB, MBBI, OpLo) - .addReg(DstReg) - .addImm(Imm) - .addReg(Src

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 added a comment. Ah, sorry - I've pushed my changes at the same time you created your comment; one moment, let me change to SUBIW, then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122533/new/ https://reviews.llvm.org/D122533 _

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 added a comment. Ah, nevermind - looks like I've already actually used `SUBIWRdK`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122533/new/ https://reviews.llvm.org/D122533 ___ cfe-commits mail

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 updated this revision to Diff 418460. Patryk27 added a comment. Add `-verify-machineinstrs` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122533/new/ https://reviews.llvm.org/D122533 Files: clang/docs/tools/clang-formatted-files.txt l

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 added a comment. Ok, I have added the switch; I think a separate patch that adds that switch to all the tests would be handy - what do you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122533/new/ https://reviews.llvm.org/D122533

[PATCH] D122533: [AVR] Remove AVRRelaxMemOperations

Patryk27 added a comment. In D122533#3410428 , @benshi001 wrote: > In D122533#3410259 , @Patryk27 > wrote: > >> Ok, I have added the switch; I think a separate patch that adds that switch >> to all of the AVR te

[PATCH] D114611: [AVR] Expand STDWSPQRr & STDSPQRr, approach #2

Patryk27 abandoned this revision. Patryk27 marked 2 inline comments as done. Patryk27 added a comment. Changes in this revision were split into separate revisions, so this one is not needed anymore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114

[PATCH] D153197: [AVR] Expand shifts during AVRISelLowering

Patryk27 added a comment. ping ping, @benshi001 👀 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153197/new/ https://reviews.llvm.org/D153197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D153197: [AVR] Expand shifts during AVRISelLowering

Patryk27 abandoned this revision. Patryk27 added a comment. Superseded by: - https://reviews.llvm.org/D154785 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153197/new/ https://reviews.llvm.org/D153197 _

[PATCH] D153197: [AVR] Expand shifts during AVRISelLowering

Patryk27 added inline comments. Comment at: llvm/lib/Target/AVR/AVRISelLowering.cpp:2207 + MF->push_back(LoopBB); + MachineBasicBlock *ExitBB = EntryBB->splitAt(MI, false); + Alright, this is wrong, after all - I've just tested it on a more elaborate code in r

[PATCH] D153197: [AVR] Expand shifts during AVRISelLowering

Patryk27 added a comment. Now that I think about it, there //might// be a simpler way! So, the underlying issue is that sometimes LLVM spawns new shifts during the instruction selection pass - for instance given this IR¹: define i64 @test(i64 %x, i32 %y) { start: %0 = or i32 %y, 38

[PATCH] D153197: [AVR] Expand shifts during AVRISelLowering

Patryk27 created this revision. Herald added subscribers: Jim, JDevlieghere, hiraditya, dylanmckay. Herald added a project: All. Patryk27 requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Some passes can introduce shifts aft

[PATCH] D153197: [AVR] Expand shifts during AVRISelLowering

Patryk27 added inline comments. Comment at: llvm/lib/Target/AVR/AVRISelLowering.cpp:2193 +// (the output registers are stored in this array on return). +static MachineBasicBlock *insertMultibyteShiftLoop( +MachineInstr &MI, MachineBasicBlock *BB, Register ShiftNum, --

[PATCH] D114611: [AVR] Expand STDWSPQRr & STDSPQRr, approach #2

Patryk27 updated this revision to Diff 402203. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114611/new/ https://reviews.llvm.org/D114611 Files: clang/docs/tools/clang-f

[PATCH] D114611: [AVR] Expand STDWSPQRr & STDSPQRr, approach #2

Patryk27 added inline comments. Comment at: llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp:1261 +if (!DstIsKill) { + buildMI(MBB, MBBI, AVR::POPWRd).addDef(Dst.getReg()); +} I'm not sure if that's correct, but the previous approach: ``` buildMI(MBB, MB

[PATCH] D114611: [AVR] Expand STDWSPQRr & STDSPQRr, approach #2

Patryk27 updated this revision to Diff 402213. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114611/new/ https://reviews.llvm.org/D114611 Files: clang/docs/tools/clang-formatted-files.txt llvm/lib/Target/AVR/AVR.h llvm/lib/Target/AVR/AVRExpand