This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Simplify eliminateFrameIndex in advance of reuse [nfc-ish]
ClosedPublic

Authored by reames on Nov 22 2022, 8:11 AM.

Details

Summary

The prior code intermixed several concerns - the actual materialization of the offset, the choice of destination register, and whether to prune the ADDI. This version factors the first part out, and then reasons only about the later two. My intention is to merge the adjustReg routine with the one from frame lowering, and then explore using the merged result to simplify frame setup and tear down.

This change is conceptually NFC, but since it results in slightly different vreg usage, the end result can change register allocation in minor ways.

Diff Detail

Event Timeline

reames created this revision.Nov 22 2022, 8:11 AM
reames requested review of this revision.Nov 22 2022, 8:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 22 2022, 8:11 AM
reames added inline comments.Nov 22 2022, 8:12 AM
llvm/test/CodeGen/RISCV/rvv/emergency-slot.mir
114–118

Note these are spills to the stack. The register allocator has picked a different register to use for the sequence below, and is thus save/restoring a different value here. That's correct, but confusing on first glance.

asb accepted this revision.Nov 23 2022, 10:25 AM

Nice refactoring - this seems much clearer to me.

This revision is now accepted and ready to land.Nov 23 2022, 10:25 AM
frasercrmck accepted this revision.Nov 24 2022, 2:05 AM

LGTM too, with some minor suggestions

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
164

Some comments/documentation about what adjustReg does would be handy, I think.

271

Since we're refactoring anyway, maybe we could add /*IsDef*/false, /*IsImp*/false, /*IsKill*/true? Might make things that bit easier to understand.

This revision was landed with ongoing or failed builds.Nov 28 2022, 10:09 AM
This revision was automatically updated to reflect the committed changes.