[PATCH] D130879: [AsmPrinter] Emit PCs into requested PCSections

[Marco Elver via Phabricator via llvm-commits]

melver marked 2 inline comments as done.
melver added subscribers: MaskRay, pcc.
melver added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:195
+  /// List of symbols to be inserted into PC sections.
+  DenseMap<const MDNode *, SmallVector<const MCSymbol *, 8>> PCSectionsSymbols;
+
----------------
dvyukov wrote:
> If metadata is used, is the SmallVector actually expected to be small (<=8)?
> I assume that SmallVector<8> will allocate space for 8 statically and then waste it if the vector is larger.
Depends on current usage. If almost all instructions in a functions end up with !pcsections, then it's likely >=8. But currently we're only attaching this to atomics, and I assume the number of atomic ops is low in a function.

But the fact that !pcsections is currently only attached to atomics is something that AsmPrinter really shouldn't care about.

So the choice of 8 is quite arbitrary. SmallVector documentation says to just leave out the small-size, and it'll choose a reasonable default. Let's try that.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1439
+      (CM == CodeModel::Medium || CM == CodeModel::Large) ? getPointerSize()
+                                                          : 4;
+
----------------
dvyukov wrote:
> Can the other models have pointer size 8?
> I assume something like Tiny/Small can have pointer size <4 (?).
> Maybe it makes sense to just use getPointerSize() always?
The code model doesn't have anything to do with target bitness.

Yes, tiny and small CMs can have 64-bit pointers (small is usually the default).
I don't think we have to worry about 16-bit architectures or 16-bit pointers.

The thing we're optimizing is saving space if we know that the code model doesn't allow addressing (relatively) code or data more than 4 GiB away - even on 64-bit architectures.

The different encoding for different CMs is documented in PCSectionsMetadata.rst.

(The idea to use 32-bit relative relocations was pointed out by @pcc. The fact we should distinguish code models was pointed out by @MaskRay. Adding both as reviewers.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130879