Skip to content

Commit e7e1d0c

Browse files
committed
Reapply "AsmPrinter: Change DIEValue to be stored by value"
This reverts commit r238350, effectively reapplying r238349 after fixing (all?) the problems, all somehow related to how I was using `AlignedArrayCharUnion<>` inside `DIEValue`: - MSVC can only handle `sizeof()` on types, not values. Change the assert. - GCC doesn't know the `is_trivially_copyable` type trait. Instead of asserting it, add destructors. - Call placement new even when constructing POD (i.e., the pointers). - Instead of copying the char buffer, copy the casted classes. I've left in a couple of `static_assert`s that I think both MSVC and GCC know how to handle. If the bots disagree with me, I'll remove them. - Check that the constructed type is either standard layout or a pointer. This protects against a programming error: we really want the "small" `DIEValue`s to be small and simple, so don't accidentally change them not to be. - Similarly, check that the size of the buffer is no bigger than a `uint64_t` or a pointer. (I thought checking against `sizeof(uint64_t)` would be good enough, but Chandler suggested that pointers might sometimes be bigger than that in the context of sanitizers.) I've also committed r238359 in the meantime, which introduces a DIEValue.def to simplify dispatching between the various types (thanks to a review comment by David Blaikie). Without that, this commit would be almost unintelligible. Here's the original commit message: -- Change `DIEValue` to be stored/passed/etc. by value, instead of reference. It's now a discriminated union, with a `Val` field storing the actual type. The classes that used to inherit from `DIEValue` no longer do. There are two categories of these: - Small values fit in a single pointer and are stored by value. - Large values require auxiliary storage, and are stored by reference. The only non-mechanical change is to tools/dsymutil/DwarfLinker.cpp. It was relying on `DIEInteger`s being passed around by reference, so I replaced that assumption with a `PatchLocation` type that stores a safe reference to where the `DIEInteger` lives instead. This commit causes a temporary regression in memory usage, since I've left merging `DIEAbbrevData` into `DIEValue` for a follow-up commit. I measured an increase from 845 MB to 879 MB, around 3.9%. The follow-up drops it lower than the starting point, and I've only recently brought the memory this low anyway, so I'm committing these changes separately to keep them incremental. (I also considered swapping the commits, but the other one first would cause a lot more code churn.) (I'm looking at `llc` memory usage on `verify-uselistorder.lto.opt.bc`; see r236629 for details.) -- llvm-svn: 238362
1 parent 27ab2b3 commit e7e1d0c

File tree

13 files changed

+575
-545
lines changed

13 files changed

+575
-545
lines changed

llvm/include/llvm/CodeGen/DIE.h

+205-158
Large diffs are not rendered by default.

llvm/include/llvm/CodeGen/DIEValue.def

+29-11
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,37 @@
1111
//
1212
//===----------------------------------------------------------------------===//
1313

14-
#if !(defined HANDLE_DIEVALUE)
14+
#if !(defined HANDLE_DIEVALUE || defined HANDLE_DIEVALUE_SMALL || \
15+
defined HANDLE_DIEVALUE_LARGE)
1516
#error "Missing macro definition of HANDLE_DIEVALUE"
1617
#endif
1718

18-
HANDLE_DIEVALUE(Integer)
19-
HANDLE_DIEVALUE(String)
20-
HANDLE_DIEVALUE(Expr)
21-
HANDLE_DIEVALUE(Label)
22-
HANDLE_DIEVALUE(Delta)
23-
HANDLE_DIEVALUE(Entry)
24-
HANDLE_DIEVALUE(TypeSignature)
25-
HANDLE_DIEVALUE(Block)
26-
HANDLE_DIEVALUE(Loc)
27-
HANDLE_DIEVALUE(LocList)
19+
// Handler for all values.
20+
#ifndef HANDLE_DIEVALUE
21+
#define HANDLE_DIEVALUE(T)
22+
#endif
23+
24+
// Handler for small values.
25+
#ifndef HANDLE_DIEVALUE_SMALL
26+
#define HANDLE_DIEVALUE_SMALL(T) HANDLE_DIEVALUE(T)
27+
#endif
28+
29+
// Handler for large values.
30+
#ifndef HANDLE_DIEVALUE_LARGE
31+
#define HANDLE_DIEVALUE_LARGE(T) HANDLE_DIEVALUE(T)
32+
#endif
33+
34+
HANDLE_DIEVALUE_SMALL(Integer)
35+
HANDLE_DIEVALUE_SMALL(String)
36+
HANDLE_DIEVALUE_SMALL(Expr)
37+
HANDLE_DIEVALUE_SMALL(Label)
38+
HANDLE_DIEVALUE_LARGE(Delta)
39+
HANDLE_DIEVALUE_SMALL(Entry)
40+
HANDLE_DIEVALUE_SMALL(TypeSignature)
41+
HANDLE_DIEVALUE_LARGE(Block)
42+
HANDLE_DIEVALUE_LARGE(Loc)
43+
HANDLE_DIEVALUE_SMALL(LocList)
2844

2945
#undef HANDLE_DIEVALUE
46+
#undef HANDLE_DIEVALUE_SMALL
47+
#undef HANDLE_DIEVALUE_LARGE

llvm/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ void AsmPrinter::emitDwarfDIE(const DIE &Die) const {
265265
dwarf::TagString(Abbrev.getTag()));
266266
EmitULEB128(Abbrev.getNumber());
267267

268-
const SmallVectorImpl<DIEValue *> &Values = Die.getValues();
268+
const SmallVectorImpl<DIEValue> &Values = Die.getValues();
269269
const SmallVectorImpl<DIEAbbrevData> &AbbrevData = Abbrev.getData();
270270

271271
// Emit the DIE attribute values.
@@ -277,12 +277,12 @@ void AsmPrinter::emitDwarfDIE(const DIE &Die) const {
277277
if (isVerbose()) {
278278
OutStreamer->AddComment(dwarf::AttributeString(Attr));
279279
if (Attr == dwarf::DW_AT_accessibility)
280-
OutStreamer->AddComment(dwarf::AccessibilityString(
281-
cast<DIEInteger>(Values[i])->getValue()));
280+
OutStreamer->AddComment(
281+
dwarf::AccessibilityString(Values[i].getDIEInteger().getValue()));
282282
}
283283

284284
// Emit an attribute using the defined form.
285-
Values[i]->EmitValue(this, Form);
285+
Values[i].EmitValue(this, Form);
286286
}
287287

288288
// Emit the DIE children if any.

0 commit comments

Comments
 (0)