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

[Nikita Popov via Phabricator via llvm-commits]

nikic marked an inline comment as done.
nikic added a comment.

In D129288#3643331 <https://reviews.llvm.org/D129288#3643331>, @nickdesaulniers wrote:

> 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.)

Ah yes, I got confused by this check here: https://github.com/llvm/llvm-project/blob/f0cd5389850589fbf8a3ce77cea2539579bd8028/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1763 But that one guards against inlining a callbr call itself, rather than a call to a function that contains callbr.



================
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]
-
----------------
nickdesaulniers wrote:
> I think we want to retain this test? The important part is that `to label %both [label %both]` not be modified.
Do you mean as a CHECK-NOT test, to check that duplicate successors are allowed now?


================
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() {
----------------
nickdesaulniers wrote:
> 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.
Yeah, these errors are thrown as part of IR parsing rather than the verifier, and that one stops on first error. (As a side note, we should probably make InlineAsm::Verify return an Error so we can produce a more specific error message for different cases. The current one leaves something to be desired...)


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

https://reviews.llvm.org/D129288