[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

James Y Knight via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 16 15:05:56 PST 2021


jyknight added inline comments.


================
Comment at: llvm/test/tools/llvm-diff/callbr.ll:28-29
 entry:
-  callbr void asm sideeffect "", "X,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return))
+  callbr void asm sideeffect "", "i,i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return))
           to label %asm.fallthrough [label %return, label %t_no]
 
----------------
nickdesaulniers wrote:
> pengfei wrote:
> > jyknight wrote:
> > > pengfei wrote:
> > > > If my above assumption is true, I think we can't replace the `X` with `i` here.
> > > > Besides, I'm confused on the indirect labels list. Are they the labels of `bar` or `foo`?
> > > I don't see a a problem with using "i" everywhere -- all blockaddress are going to be immediate values no matter whether they're an indirect target or not.
> > > 
> > > The indirect labels list is only referring to labels in the current function.
> > > 
> > > This test is confusing, but, it is a test for llvm-diff, so that's okay or maybe even intended. (It can't actually possibly jump to @bar:%return or @bar:%t_no, because nothing ever gets the address of those labels. It does get the similarly-named labels in @foo, but it can't jump to those either, since they're in a different function.)
> > Thanks for the explanation. My point is the test3 above intended to use `X` to indicate the destination is not in the indirect labels list. For consistency, we should use `X` here too, since the @foo:%return etc. are not in the list either. Or we don't need to use `X` in test3.
> The "indirect destination list" for the `callbr` in `@bar` is the `[label %return, label %t_no]`. Both operands have corresponding `blockaddress` arguments. So they //should not// use `X` in this case.
I don't see why the correct constraint to use should be related at all to whether the blockaddress argument corresponds to a label in the indirect label list or not.

Using something other than "X" should probably always be preferred, since presumably the instruction you're emitting has requirements. (Unless of course you don't actually use the argument, or only use it in a comment, or something like that...in which case "X" is fine.)

But, FTR, in this test, the blockaddress is for a label in a //different// function ("@foo") than the function we're in ("@bar"), which is what pengfei was pointing out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115410



More information about the cfe-commits mailing list