[PATCH] D65304: [MC] Don't recreate a label if it's already used

[Bill Wendling via Phabricator via llvm-commits]

void added inline comments.


================
Comment at: lib/MC/MCContext.cpp:64
       Symbols(Allocator), UsedNames(Allocator),
+      InlineAsmUsedLabelNames(Allocator),
       CurrentDwarfLoc(0, 0, 0, DWARF2_FLAG_IS_STMT, 0, 0),
----------------
nickdesaulniers wrote:
> I think we may want to be invoking the `StringMap` constructor that additionally takes an `InitialSize`, which we'd set to `0U`, as it's highly unlikely that the `%l` modifier is used at all for any given piece of code.
> 
> IIUC, `StringMap`'s constructor that simply accepts an allocator will allocate room for 1 instance.  See the definition of `StringMapImpl::StringMapImpl(unsigned InitSize, unsigned itemSize)`.
> 
> I think that micro optimization would save many small allocations for the general case.
The c'tor that's called when we supply only an allocator is this one:

```
explicit StringMapImpl(unsigned itemSize)
      : ItemSize(itemSize) {}
```

If we supply an `InitSize`, it executes the `init()` function during creation, but only if it's non-zero. So the effect is the same whether we supply it or not.


================
Comment at: test/CodeGen/X86/callbr-asm-label-addr.ll:8
+; CHECK-LABEL: .Ltmp1:
+; CHECK-LABEL: .LBB0_1: # %bar
+; CHECK-NEXT:    callq foo
----------------
nickdesaulniers wrote:
> Maybe not necessary to solve in this patch, but it's curious to me why we emit BOTH `.Ltmp1` AND `.LBB0_1` for `%bar`, but only `.Ltmp0` for `%baz` (with `%bb.2` commented out).  Obviously related to `%bar` being the `callbr` jump target (while `%baz` is simply the address of a label), but something seems wrong to me to emit so many labels to the same statement.
> 
> From this case specifically, I feel like `.LBB0_1:` should not be emitted, and instead there should be a (commented out) `# %bb.1:    # %bar`.
Yeah, that's beyond the scope of this patch. It should *probably* emit a label only when needed, but that's probably infeasible with the current structure of MC...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65304