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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Hmm...this still isn't as precise as it could be. Consider the following input:

  int x;
  void foo (void) {
      asm goto (
      "# %0\n\t"
      "# %1\n\t"
      "# %l2\n\t"
      : "=r"(x):"rm"(&&bar)::bar);
      bar:;
  }

and corresponding IR:

  %1 = callbr i32 asm "# $0\0A\09# $1\0A\09# ${2:l}\0A\09", "=r,rm,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %3), i8* blockaddress(@foo, %3))
            to label %2 [label %3]

(so, the indirect destination is //both// in the inputs //and// in the indirect destinations.  Given an argument number, we still can't tell whether the first or second `blockaddress` was the intended target.

Changing the above `=r` output constrain to `+r` produces:

  %2 = callbr i32 asm "# $0\0A\09# $1\0A\09# ${2:l}\0A\09", "=r,rm,X,0,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %4), i8* blockaddress(@foo, %4), i32 %1)
            to label %3 [label %4]

ie. we have inputs that occur both before and after the indirect dest (in this case, `"X"` is meant for the indirect dest).
(before: "rm" corresponds to the first non-target `blockaddress`. after: `0` is a "backreference" to the 0-th (first) constraint (`"=r"`; the output)

then changing `int x;` at the global scope to `register int x asm("edi");` at function scope produces:

  %1 = callbr i32 asm "# $0\0A\09# $1\0A\09# ${2:l}\0A\09", "={edi},rm,X,{edi},~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %3), i8* blockaddress(@foo, %3), i32 undef)
            to label %2 [label %3]

ie. we again have inputs that occur both before and after the indirect dest.
(before: "rm" corresponds to the first non-target `blockaddress`. after: `{edi}` for the input register local).

I don't see how we can try to re-construct or disambiguate which `blockaddress` argument is the intended target at this point.

Perhaps if we refined the Langref from:

> The constraints must always be given in that order: outputs first,
> then inputs, then clobbers. They cannot be intermingled.

to:

> The constraints must always be given in that order: outputs first,
> then inputs, then clobbers, then goto labels. They cannot be intermingled.

example 3 would change to:

  %1 = callbr i32 asm "# $0\0A\09# $1\0A\09# ${2:l}\0A\09", "={edi},rm,{edi},~{dirflag},~{fpsr},~{flags},X"(i8* blockaddress(@foo, %3), i32 undef,  i8* blockaddress(@foo, %3))
            to label %2 [label %3]

This would more closely match the existing order of parameters in inline asm statements.  Or perhaps even just requiring that goto labels always were the final inputs. ie. example 3 would change to:

  %1 = callbr i32 asm "# $0\0A\09# $1\0A\09# ${2:l}\0A\09", "={edi},rm,{edi},X~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %3), i32 undef, i8* blockaddress(@foo, %3))
            to label %2 [label %3]

Because if we didn't intersperse inputs with "goto labels", and we know we have 3 inputs (in the final case described above) and we know we have 1 goto label, then it's obvious that the argument numbers that are indirect dests are in the range `[arg_size() - CBR->numIndirectDests(), arg_size())`.

I would make such a change as a parent patch, then rebase this on top.  Or is there something more perhaps more obvious that I'm missing that won't require a Langref change (and test churn)?  Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114895