[CodeGen] Require a name for a block addr target

[void]

Summary

A block address may be used in inline assembly. In which case it
requires a name so that the asm parser has something to parse. Creating
a name for every block address is a large hammer, but is necessary
because at the point when a temp symbol is created we don't necessarily
know if it's used in inline asm. This ensures that it exists regardless.

[void]

Friendly ping. :-)

[nickdesaulniers]

Creating a name for every block address is a large hammer, but is necessary because at the point when a temp symbol is created we don't necessarily know if it's used in inline asm.

Sure we do; in this case if llvm::any_of the Uses of the BasicBlocks isa<CallBrInst> then we need to name the label. Right?

[void]

Ah! Good point. Use "any_of" instead of creating a symbol for all block addresses.

[void]

Ugh! The last update combined D65304 with this...:-/

[void]

Remove accidentally added files.

[void]

It's not enough to just check that a BB's address is used by a callbr, since
the value doesn't have to be directly used by it. E.g., it could be stored into
memory and then loaded.

[nickdesaulniers]

test/CodeGen/X86/callbr-asm-obj-file.ll

1 ↗

(On Diff #214045)

When I run this test case locally, I get:

$ llc < callbr-asm-obj-file.ll -filetype=obj -mtriple=x86_64-linux-gnu
<unknown>:0: error: Undefined temporary symbol

Am I holding it wrong?

[nathanchance]

test/CodeGen/X86/callbr-asm-obj-file.ll

1 ↗

(On Diff #214045)

Do you have D65304 as well? With just this patch, I see that same error but with both patches, this test case works as expected.

[nickdesaulniers]

test/CodeGen/X86/callbr-asm-obj-file.ll

1 ↗

(On Diff #214045)

Ah, I missed the parent revision relationship between the two patches. Does phabricator UI show this relation anywhere?

[nathanchance]

test/CodeGen/X86/callbr-asm-obj-file.ll

1 ↗

(On Diff #214045)

I see it right above my subscription to this revision.

void added a parent revision: D65304: [MC] Don't recreate a label if it's already used.

[void]

In the "Revision Contents" box, there's a "Stack" tab. It looks like that's what shows the relationship.

[nickdesaulniers]

I think the comment in LLVMTargetMachine::addAsmPrinter about // Don't waste memory on names of temp labels. should be amended in this patch to say ... unless it's address is taken or something to that effect.

[void]

In D65352#1622024, @nickdesaulniers wrote:

I think the comment in LLVMTargetMachine::addAsmPrinter about // Don't waste memory on names of temp labels. should be amended in this patch to say ... unless it's address is taken or something to that effect.

I'm not sure about that. It might be confusing to people, as we aren't technically wasting memory if the block address is taken. Also, the boolean being set here seems to have slightly different semantics from the change I made.

llvm/trunk/lib/CodeGen/MachineModuleInfo.cpp

ArrayRef<MCSymbol *> MMIAddrLabelMap::getAddrLabelSymbolToEmit(BasicBlock *BB) {
   }
 
   // Otherwise, this is a new entry, create a new symbol for it and add an
   // entry to BBCallbacks so we can be notified if the BB is deleted or RAUWd.
   BBCallbacks.emplace_back(BB);
   BBCallbacks.back().setMap(this);
   Entry.Index = BBCallbacks.size() - 1;
   Entry.Fn = BB->getParent();
-  Entry.Symbols.push_back(Context.createTempSymbol());
+  Entry.Symbols.push_back(Context.createTempSymbol(!BB->hasAddressTaken()));
   return Entry.Symbols;
 }
 
 /// If we have any deleted symbols for F, return them.
 void MMIAddrLabelMap::
 takeDeletedSymbolsForFunction(Function *F, std::vector<MCSymbol*> &Result) {
   DenseMap<AssertingVH<Function>, std::vector<MCSymbol*>>::iterator I =
     DeletedAddrLabelsNeedingEmission.find(F);

llvm/trunk/test/CodeGen/AArch64/callbr-asm-obj-file.ll

+; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -filetype=obj -o - \
+; RUN:  | llvm-objdump -triple aarch64-unknown-linux-gnu -d - \
+; RUN:  | FileCheck %s
+
+%struct.c = type { i1 (...)* }
+
+@l = common hidden local_unnamed_addr global i32 0, align 4
+
+; CHECK-LABEL: 0000000000000000 test1:
+; CHECK-LABEL: 0000000000000018 $d.1:
+; CHECK-LABEL: 0000000000000020 $x.2:
+; CHECK-NEXT:    b #16 <$x.4+0x4>
+; CHECK-LABEL: 000000000000002c $x.4:
+; CHECK-NEXT:    b #4 <$x.4+0x4>
+; CHECK-NEXT:    mov w0, wzr
+; CHECK-NEXT:    ldr x30, [sp], #16
+; CHECK-NEXT:    ret
+define hidden i32 @test1() {
+  %1 = tail call i32 bitcast (i32 (...)* @g to i32 ()*)()
+  %2 = icmp eq i32 %1, 0
+  br i1 %2, label %3, label %5
+
+3:                                                ; preds = %0
+  callbr void asm sideeffect "1: nop\0A\09.quad a\0A\09b ${1:l}\0A\09.quad ${0:c}", "i,X"(i32* null, i8* blockaddress(@test1, %7))
+          to label %4 [label %7]
+
+4:                                                ; preds = %3
+  br label %7
+
+5:                                                ; preds = %0
+  %6 = tail call i32 bitcast (i32 (...)* @i to i32 ()*)()
+  br label %7
+
+7:                                                ; preds = %3, %4, %5
+  %8 = phi i32 [ %6, %5 ], [ 0, %4 ], [ 0, %3 ]
+  ret i32 %8
+}
+
+declare dso_local i32 @g(...) local_unnamed_addr
+
+declare dso_local i32 @i(...) local_unnamed_addr
+
+; CHECK-LABEL: 000000000000003c test2:
+; CHECK:         bl #0 <test2+0x18>
+; CHECK-LABEL: 0000000000000064 $d.5:
+; CHECK-LABEL: 000000000000006c $x.6:
+; CHECK-NEXT:    b #-24 <test2+0x18>
+define hidden i32 @test2() local_unnamed_addr {
+  %1 = load i32, i32* @l, align 4
+  %2 = icmp eq i32 %1, 0
+  br i1 %2, label %10, label %3
+
+3:                                                ; preds = %0
+  %4 = tail call i32 bitcast (i32 (...)* @g to i32 ()*)()
+  %5 = icmp eq i32 %4, 0
+  br i1 %5, label %6, label %7
+
+6:                                                ; preds = %3
+  callbr void asm sideeffect "1: nop\0A\09.quad b\0A\09b ${1:l}\0A\09.quad ${0:c}", "i,X"(i32* null, i8* blockaddress(@test2, %7))
+          to label %10 [label %9]
+
+7:                                                ; preds = %3
+  %8 = tail call i32 bitcast (i32 (...)* @i to i32 ()*)()
+  br label %10
+
+9:                                                ; preds = %6
+  br label %10
+
+10:                                               ; preds = %7, %0, %6, %9
+  ret i32 undef
+}
+
+; CHECK-LABEL: 0000000000000084 test3:
+; CHECK-LABEL: 00000000000000a8 $d.9:
+; CHECK-LABEL: 00000000000000b0 $x.10:
+; CHECK-NEXT:    b #20 <$x.12+0x8>
+; CHECK-LABEL: 00000000000000bc $x.12:
+; CHECK-NEXT:    b #4 <$x.12+0x4>
+; CHECK-NEXT:    mov w0, wzr
+; CHECK-NEXT:    ldr x30, [sp], #16
+; CHECK-NEXT:    ret
+define internal i1 @test3() {
+  %1 = tail call i32 bitcast (i32 (...)* @g to i32 ()*)()
+  %2 = icmp eq i32 %1, 0
+  br i1 %2, label %3, label %5
+
+3:                                                ; preds = %0
+  callbr void asm sideeffect "1: nop\0A\09.quad c\0A\09b ${1:l}\0A\09.quad ${0:c}", "i,X"(i32* null, i8* blockaddress(@test3, %8))
+          to label %4 [label %8]
+
+4:                                                ; preds = %3
+  br label %8
+
+5:                                                ; preds = %0
+  %6 = tail call i32 bitcast (i32 (...)* @i to i32 ()*)()
+  %7 = icmp ne i32 %6, 0
+  br label %8
+
+8:                                                ; preds = %3, %4, %5
+  %9 = phi i1 [ %7, %5 ], [ false, %4 ], [ false, %3 ]
+  ret i1 %9
+}

llvm/trunk/test/CodeGen/X86/callbr-asm-obj-file.ll

+; RUN: llc < %s -mtriple=x86_64-linux-gnu -filetype=obj -o - \
+; RUN:  | llvm-objdump -triple x86_64-linux-gnu -d - \
+; RUN:  | FileCheck %s
+
+; CHECK: 0000000000000000 test1:
+; CHECK-NEXT:   0: 74 00 je 0 <test1+0x2>
+; CHECK-NEXT:   2: c3    retq
+
+define void @test1() {
+entry:
+  callbr void asm sideeffect "je ${0:l}", "X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@test1, %a.b.normal.jump))
+          to label %asm.fallthrough [label %a.b.normal.jump]
+
+asm.fallthrough:
+  ret void
+
+a.b.normal.jump:
+  ret void
+}