[PATCH] D114895: [SelectionDagBuilder] improve CallBrInst BlockAddress constraint handling

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

In D114895#3181302 <https://reviews.llvm.org/D114895#3181302>, @nickdesaulniers wrote:

> if you change the asm string to use `%l2` rather than `%l1`, it compiles (and emits the expected code).  I think this is another incompatibility that is likely to harm us in the future.  I'm not sure yet whether this is orthogonal to this issue. (I'm not sure whether this is a bug or a feature in GCC; it means that the %0,%1,%2 references in the asm string are ordered: outputs, non-tied-inputs, inputs-tied-to-outputs, then goto-labels.  Clang today emits them as outputs, non-tied-inputs, goto-labels, then inputs-tied-to-outputs.

Ouch.  We probably need to fix that...

> Finally, I had a thought last night that I've always felt that `callbr` had a minor design issue that never quite sat right with me. https://reviews.llvm.org/D74947 demonstrates how methods like `User::replaceUsesOfWith` can subtly break `callbr`. Since `callbr` duplicates it indirect destinations twice in its operand list, once for the arg list to the `call asm` and again just cause (ie. the labels used as indirect destinations show up twice in the actual IR statement), replacing operands is brittle because the duplicate operands need to stay in sync.  If the indirect list at the end used placeholders more akin to `std::placeholders`, then we would know precisely which argument are intended to be indirect destinations, and avoid brittle duplication that breaks when calling `User::replaceUsesOfWith`.  This would be extensive surgery to `callbr`, but I'd sleep better at night knowing that `callbr` was less brittle.

The current design allows you to use blockaddress constants in code outside the asm goto to compute the destination of an asm goto, I think?  At least, the IR definition looks like it allows that; not sure how well it works in practice.

If we're willing to abandon this, we could use an alternate design where we don't use blockaddress at all.  Instead, asm callbr has implicit arguments: we just allow the asm to refer to the destinations labels directly.  So for a callbr with no explicit arguments, "$0" refers to the first destination.  The primary benefit here is that we can remove a bunch of special cases for callbr control flow at the IR level.  (I expect that in/after SelectionDAG, we'd keep the current representation?  At least, we can consider that separately.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114895