This is an archive of the discontinued LLVM Phabricator instance.

[BOLT][DWARF] Add ability to insert new entries in to DIE
ClosedPublic

Authored by ayermolo on Feb 11 2022, 12:35 PM.

Details

Summary

Added ability to append new entries to DIE. This is useful to standadize DWARF4
Split Dwarf, and simplify implementation of DWARF5.
Multiple DIEs can share an abbrev. So currently limitation is that only unique
Attributes can be added.

Diff Detail

Event Timeline

ayermolo created this revision.Feb 11 2022, 12:35 PM
ayermolo requested review of this revision.Feb 11 2022, 12:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2022, 12:35 PM
ayermolo updated this revision to Diff 408658.Feb 14 2022, 3:51 PM

Missed a place where we update range base.

maksfb added inline comments.Feb 14 2022, 11:17 PM
bolt/lib/Core/DebugData.cpp
902

nit: add braces for the if since the nested for is braced.

bolt/lib/Rewrite/DWARFRewriter.cpp
603

Accidental?

bolt/test/X86/insert-debug-info-entry.test
10

Could you add a description what the test is checking? If you are inserting a new attribute, perhaps you should check that it's introduced by BOLT and not by llc?

ayermolo marked 3 inline comments as done.Feb 15 2022, 1:52 PM
ayermolo updated this revision to Diff 409058.Feb 15 2022, 2:19 PM

addressing comments

ayermolo updated this revision to Diff 409062.Feb 15 2022, 2:32 PM

missed comment

maksfb added inline comments.Feb 15 2022, 3:28 PM
bolt/include/bolt/Core/DebugData.h
666
669
bolt/lib/Core/DebugData.cpp
422

We need a proper way to handle endianness.

yota9 added inline comments.Feb 15 2022, 3:37 PM
bolt/lib/Core/DebugData.cpp
422

You can take a look to support::endian::Writer, here is an example I'm using in golang support:

template <typename T>
static T writeEndian(BinaryContext &BC, T Val) {
  T ret;
  SmallVector<char, sizeof(T)> Tmp;
  raw_svector_ostream OS(Tmp);

  if (BC.AsmInfo->isLittleEndian())
    support::endian::Writer<support::little>(OS).write<T>(Val);
  else
    support::endian::Writer<support::big>(OS).write<T>(Val);

  memcpy(&ret, OS.str().data(), sizeof(T));
  return ret;
}
ayermolo added inline comments.Feb 15 2022, 4:36 PM
bolt/lib/Core/DebugData.cpp
422

I think it should be part of a different commit thought.

maksfb accepted this revision.Feb 15 2022, 6:01 PM
maksfb added inline comments.
bolt/lib/Core/DebugData.cpp
422

Since it's moved code, it makes sense.

This revision is now accepted and ready to land.Feb 15 2022, 6:01 PM