[PATCH] D115325: [DWARF] Fix PR51087 Extraneous enum record in DWARF with type units

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D115325#3178860 <https://reviews.llvm.org/D115325#3178860>, @Orlando wrote:

> More info about the fix:
> I've moved most of the body of addDwarfTypeUnitType into a new function getOrCreateDwarfTypeUnit, which addDwarfTypeUnitType now wraps. getOrCreateDwarfTypeUnit only gets or creates a type unit, and addDwarfTypeUnitType maintains its previous behaviour which is: get or create a type unit and add the info to a DIE (passed in as a parameter) if successful, otherwise construct the type in that DIE directly.
>
> With the patch we call getOrCreateDwarfTypeUnit for each enum, which generates a type unit without adding a skeleton DIE to the CU. If the call fails because a type unit cannot be emitted then we fall back to the old behaviour. If a CU DIE is needed later on it is created via getOrCreateTypeDIE (unchanged with the patch) and the (unchanged) call to addDwarfTypeUnitType will find the already-created type unit signature to add to the DIE.
>
> Possible future improvement:
> I think there could be some further work to reduce redundancy with the retained types (see the loop just below the getEnumTypes loop). Firstly, retained types are not actually referenced in the CU, so I don't think there is any point generating a DW_TAG_pointer_type DIE in the CU with skeleton DIEs referencing a type unit for a type. If a type unit is emitted for the pointed-to retained type we could probably skip the CU emission entirely? Secondly, applying the same same fix as for enums (try to emit only the type unit) could remove additional redundancy, more so when coupled with the previous point.

under what conditions do we get this redundant pointer type in the CU you're referring to here?



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1454-1457
     for (auto *Ty : CUNode->getRetainedTypes())
       if (DIType *RT = dyn_cast<DIType>(Ty))
         // There is no point in force-emitting a forward declaration.
         CU->getOrCreateTypeDIE(RT);
----------------
Should we do the same thing for non-enum types too?


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

https://reviews.llvm.org/D115325