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

Phoebe Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 13 19:57:43 PST 2021


pengfei 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]
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115410



More information about the llvm-commits mailing list