[PATCH] D140072: [dsymutil] Mark ODR canonical DIEs after processing the whole CU

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

DIE could be marked as ODR canonical candidate(and also kept) from the another compile unit. If we do markDIEsAsODRCanonical right after the current unit is processed then we will miss DIE marked from another compile unit. And then this DIE would not be deduplcated.

  CU1:
     0x10 DIE type "class1"   // will not be deduplicated and will be left inplace, after CU1 is processed this DIE is not marked as Kept and then is not marked as ODR canonical candidate
  CU2:
     DIE ref to 0x10               // 0x10 DIE is marked as Kept but is not marked as ODR canonical candidate
  CU3:
    0x40 DIE type "class1"   // will not be deduplicated and will be left inplace, after CU3 is processed this DIE is marked as Kept and is marked as ODR canonical candidate.
    DIE ref to 0x40 

If markDIEsAsODRCanonical would be called after all compile uints are processed:

  for (auto &CurrentUnit : OptContext.CompileUnits) {
    lookForDIEsToKeep(*OptContext.File.Addresses,
                      OptContext.File.Addresses->getValidAddressRanges(),
                      OptContext.CompileUnits,
                      CurrentUnit->getOrigUnit().getUnitDIE(),
                      OptContext.File, *CurrentUnit, 0);
  }
  for (auto &CurrentUnit : OptContext.CompileUnits) {
    markDIEsAsODRCanonical(CurrentUnit->getOrigUnit().getUnitDIE(),
                           *CurrentUnit);
  }

Then deduplication would not be done for all these compile units, since it is based on ODRCanonical information which is not ready while lookForDIEsToKeep is working:

  void DWARFLinker::lookForRefDIEsToKeep(
  ..
        // If the referenced DIE has a DeclContext that has already been
        // emitted, then do not keep the one in this CU. We'll link to
        // the canonical DIE in cloneDieReferenceAttribute.
        //
        // FIXME: compatibility with dsymutil-classic. UseODR shouldn't
        // be necessary and could be advantageously replaced by
        // ReferencedCU->hasODR() && CU.hasODR().
        //
        // FIXME: compatibility with dsymutil-classic. There is no
        // reason not to unique ref_addr references.
        if (AttrSpec.Form != dwarf::DW_FORM_ref_addr &&
            isODRAttribute(AttrSpec.Attr) && Info.Ctxt &&
            Info.Ctxt->hasCanonicalDIE())  // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
          continue;    
  ...


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

https://reviews.llvm.org/D140072