[PATCH] D129288: [IR] Don't use blockaddresses as callbr arguments

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 7 13:51:50 PDT 2022


nickdesaulniers added a comment.

In D129288#3636297 <https://reviews.llvm.org/D129288#3636297>, @jyknight wrote:

>> Why the poison placeholders? This makes it easy to satisfy the existing inlineasm infrastructure. It would be cleaner to omit the placeholder arguments and just say that trailing inlinasm constraints refer to the indirect target labels. InlineAsm itself doesn't have knowledge about those though, so it would not be possible to verify InlineAsm constraints in isolation.
>
> I'm really uncomfortable with this hack. If we're going to introduce a new kind of input which doesn't come from the arglist, I think it'd be better to introduce a new constraint modifier symbol which can be used with input constraints to indicate that...then, the parsed InlineAsm will have the expected count of those separately, and the the verifier can verify the count matches at the appropriate time (during call/callbr verification), and we can propagate this into SDag, etc...
>
> E.g., we could choose the symbol '!' for that, so you might have e.g.:
> `callbr void asm sideeffect "", "!i,r,!i,~{dirflag},~{fpsr},~{flags}"(i32 4) to label %1 [label %2, label %3]`

I like this suggestion because to me it is homogenous with outputs. Outputs also do not appear in the operand list, yet are represented in the constraint string.  As an example:

  ; Before:
  %res = callbr i8* asm "# $0\0A\09# ${2:l}", "=r,0,i,~{dirflag},~{fpsr},~{flags}"(i8* %0, i8* blockaddress(@test8, %foo))
  to label %asm.fallthrough [label %foo]
  ; After:
  %res = callbr i8* asm "# $0\0A\09# ${2:l}", "=r,0,!i,~{dirflag},~{fpsr},~{flags}"(i8* %0)
  to label %asm.fallthrough [label %foo]

(where `"=r"` applies to `%res` and isn't listed in the asm string, `0` refers to `%0` and also isn't listed in the asm string, then `!i` refers to `label %foo` and `${2:l}` in the asm string, just to be overly specific).

Does the langref mention any specific constraint string symbols?  Regardless, such a change should be documented very explicitly since it doesn't collide with any constraint string AFAIK today, but theoretically could in the future should the higher level languages implementing extended inline asm choose such a character as the constraint representation.


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

https://reviews.llvm.org/D129288



More information about the llvm-commits mailing list