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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Wait a second;

  void *x(void) {
      void *p = &&foo;
      asm goto ("# %0\n\t# %l1":"+r"(p):::foo);
      foo:;
      return p;
  }

doesn't compile on gcc-11:

  <source>:3:5: error: invalid 'asm': '%l' operand isn't a label
      3 |     asm goto ("# %0\n\t# %l1":"+r"(p):::foo);
        |     ^~~

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.

If we fixed that incompatibility (not sure if it's a bug vs feature in GCC), then the approach in this patch would make more sense.  That said, I have made the changes that @efriedma described locally and they do seem like a good simplification.  They need more testing, but I'm cleaning them up and will hopefully post by EOD.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114895