[llvm-branch-commits] [llvm] [LivePhysRegs] Add callee-saved regs from MFI in addLiveOutsNoPristines. (PR #73553)

Florian Hahn via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Nov 27 12:59:30 PST 2023


fhahn wrote:


> I still feel like I am missing something here, and it's been a while since I looked at this. But my impression is that LLVM modeling is "cheating" a bit in that technically all the callee-saves should be implicit-uses of the return instruction (and not live afterwards) but we don't model it that way and instead make them appear as live-outs of the return block? So doesn't seem like overestimating the liveness because of our current modeling?

Yep, the current modeling in general may overestimates the liveness. With overestimating I meant that with this patch we overestimate in more places, but that's by design.

> 
> If I'm reading your patch correctly it would mean we would start adding all pristine registers for the return block[1]. I am just confused so far because this is happening in a function called `addLiveOutsNoPristines`...
>

I am not sure what exactly the definition for pristines is (and not super familiar with this code in general), and maybe the function name needs to be changed. The main thing to note is that it won't add all pristines; `addPristines` adds all callee saved registers (via TRI) and removes the ones which are in he machine function's CalleeSavedInfo. The patch adds pristines, but *only* those that have been added to CalleeSavedInfo.

 
> [1] Pristine == "callee saved but happens to be unused and hence not saved/restored" which is what happens when you remove that `Info.isRestored()` check?


> Which code/pass is using LivePhysRegs that is causing you trouble here?

The issue is in `BranchFolding`, where after splitting, the liveness of `LR` isn't preserved in the new or merged blocks. It could be fixed locally there by iterating over the registers in CalleeSavedInfo and checking if they were live-in in the original block (see below), but I am worried that fixing this locally leaves us open for similar issues in other parts of the codebase.

https://github.com/llvm/llvm-project/pull/73553


More information about the llvm-branch-commits mailing list