[PATCH] D31908: [AntiDepBreaker] Don't rename callee saved register restore instruction

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D31908#746050, @hfinkel wrote:

> In https://reviews.llvm.org/D31908#745958, @kparzysz wrote:
>
> > In https://reviews.llvm.org/D31908#745274, @MatzeB wrote:
> >
> > > Yeah I like that way of modeling things explicitely. Unfortunately that is not how most llvm targets work today.
> >
> >
> > Would it be reasonable to implement that for all targets?
>


I think it would work and I think it would make the representation a bit cleaner. If there happened to be a target with hundreds of CSRs that would be a lot of extra operands, on the other hand that target already has hundreds of entries in the CSR list anyway.

Changing this would simplify some code in LivePhysReg, RegScavenger, DepBreaker etc. where people iterate the CSRs today. It's a small thing though I don't expect it to have a huge impact one way or another...

> Could we check this in the verifier? One advantage this might have is that, for blocks ending in unreachable, we can easily realize that we don't need the restoration code.

Verifying it today would be easy. Though if we go that route it may be reasonable to just not model an explicit list of CSRs anymore after PrologEpilog inserter (which means we couldn't verify anymore, but I always like it if we can do the same with less concepts/complexity).

It would indeed fix some situations in which we cannot properly differentiate between return vs. unreachable/noreturn call anymore (imagine if-converting a return instructions that ends up in the middle of a basic block, which `MachineBasicBlock::isReturnBlock()` would miss).


https://reviews.llvm.org/D31908