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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

We should still strike

> Allow inlining of callbr

>From the commit description.

This was already done in
commit 639b29b1b59f ("[INLINER] allow inlining of blockaddresses if sole uses are callbrs")
(I wonder if it's now safe to revert 639b29b1b59f, too? Keeping the test case though.)

---

Patch LGTM; but please don't land until we've had more time to test this. It's the kind of change that has the potential to regress our kernel builds.  I'm tied up today, tomorrow, and most of Wednesday, but @nathanchance and I will hammer on this and get you a green light by EOW. Thanks for the work here!



================
Comment at: llvm/test/Verifier/callbr.ll:67-76
-;; Ensure you cannot specify the same label as both normal and indirect targets.
-; CHECK: Duplicate callbr destination!
-; CHECK-NEXT: #test5
-define i32 @test5() {
-entry:
-  %ret = callbr i32 asm sideeffect "#test5", "=r,i"(i8* blockaddress(@test5, %both)) to label %both [label %both]
-
----------------
I think we want to retain this test? The important part is that `to label %both [label %both]` not be modified.


================
Comment at: llvm/test/Verifier/inline-asm-label-before-outputs.ll:3
+
+; CHECK: error: invalid type for inline asm constraint string
+define void @label_constraint_after_clobbers() {
----------------
Does the assembler stop emitting diagnostics after the first? If not, consider adding this to llvm/test/Verifier/inline-asm-label-after-clobbers.ll and maybe renaming the test file.


================
Comment at: llvm/test/tools/llvm-diff/callbr.ll:24
-; CHECK-NEXT:    <   callbr void asm sideeffect "", "i,i,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return))
-; CHECK-NEXT:          to label %asm.fallthrough [label %return, label %t_no]
-
----------------
I was never really sure what the intent of this test was...


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

https://reviews.llvm.org/D129288