[PATCH] D43286: [CodeGen] Generate DWARF v5 Accelerator Tables

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/AccelTable.cpp:469
+  switch (Form) {
+  case dwarf::DW_FORM_data1:
+    Size = 1;
----------------
JDevlieghere wrote:
> labath wrote:
> > JDevlieghere wrote:
> > > labath wrote:
> > > > aprantl wrote:
> > > > > Don't we have this somewhere else already and if not, should it be factored into a common utility function?
> > > > We have something in the DebugInfo library, but I haven't seen anything in CodeGen. I'll have another look tomorrow...
> > > I've seen similar code quite a bit the last few weeks but always in different parts which made it hard to reuse. Maybe it's time to put something as general as possible in Dwarf.def? The problem is usually that there's no fixed length for (U)LEB128 encoded forms.
> > It seems it should be fairly easy to move the DWARFFormParams class and the static DWARFFormValue::getFixedByteSize function somewhere into the BinaryFormat library. Jonas, can you give me pointers to the other places which do form size calculations?
> Looks like it's not as prominent as I thought, or maybe I'm not grep'ing effectively: 
>  - `DIEInteger::SizeOf`
>  - `DIEEntry::SizeOf`'
>  - `DWARFFormValue::getFixedByteSize` 
> 
> Theres's also `DW_EH_PE_udata4`/`DW_EH_PE_sdata4` variants that could probably benefit from a similar function:
>  - `getSizeForEncoding` in `MCDwarf`
>  - `DWARFDataExtractor::getEncodedPointer`
Actually, it looks like the DIEInteger class does exactly what I want here, so I was able to remove both instances of size computation by delegating to it.  There is still the opportunity to merge DWARFFormValue::getFixedByteSize and DIEInteger::SizeOf, which I will pursue separately.


Repository:
  rL LLVM

https://reviews.llvm.org/D43286