[PATCH] D139565: [IR] add new callbrpad instruction

Thu Dec 8 12:59:02 PST 2022

nickdesaulniers added a comment.

Thanks for the feedback!

In D139565#3982246 <https://reviews.llvm.org/D139565#3982246>, @nhaehnle wrote:

> but it may also produce values that are only available in some subset of successors.

Not precisely. All output values should be available in all successors as a result of this feature.

> and the "direct" successor may have a callbrpad.

Yes, it would be possible to require all edges have a landing pad, regardless of direct vs indirect.  Perhaps an ergonomics decision.

> This makes me dislike the name callbrpad

My heart isn't set on the name, but it sounds like @efriedma thinks that a new instruction is too heavyweight a solution to this problem.  I look forward to @efriedma 's thoughts on my latest idea. https://reviews.llvm.org/D139565#3980574 before I proceed further with ANY approach.

> (Side note: I don't understand why the diff puts so much emphasis on physical registers in the description of `callbrpad`. I would expect the issue to exist regardless of how the inlineasm constrains the output.)

I tried to allude to this in the RFC <https://discourse.llvm.org/uploads/short-url/Atx6Vibbl3XeIDQ0A1W5kOgjn1H.pdf> but TL;DR:

1. MIR doesn't allow for phi's to have physregs, only virtregs. We need to COPY physregs (which the inline asm constraints might explicitly require) to virtregs then use those in phis.
2. It's possible to express two `callbr`'s with mutually exclusive phyreg constraints for the same output variable which have the same indirect destination. So we must have a phi, which can't have physregs, so we need copies. Those copies need to go somewhere, and it's not correct/appropriate to put them in the predecessor or successor for such critical edges.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139565/new/

https://reviews.llvm.org/D139565