[llvm] r198196 - Use a pointer to keep track of the skeleton unit for each normal unit

[Eric Christopher]

>> @@ -1082,7 +1080,9 @@ void DwarfDebug::finalizeModuleInfo() {
>>      // Add CU specific attributes if we need to add any.
>>      if (TheU->getUnitDie()->getTag() == dwarf::DW_TAG_compile_unit) {
>>        // If we're splitting the dwarf out now that we've got the entire
>> -      // CU then construct a skeleton CU based upon it.
>> +      // CU then add the dwo id to it.
>> +      DwarfCompileUnit *SkCU =
>> +          static_cast<DwarfCompileUnit *>(TheU->getSkeleton());
>
>
> Potentially we could change the check above (== compile_unit) to use
> dyn_cast on the DwarfUnit itself (would need to add cast machinery to
> DwarfUnit, I suppose) and add covariant overrides to getSkeleton in
> Dwarf*Unit so that DwarfCompileUnit::getSkeleton returns a DwarfCompileUnit
> & you wouldn't need the cast here.
>

Hrm. We could...

> Actually - do you need anything from DwarfCompileUnit specifically here? Or
> could you just make "SkCU" a DwarfUnit? (self documenting code is a fine
> reason, just not sure if it's the only one here)
>

Self-documenting was one reason, the other was that the type unit
stuff is still in flux so I was trying to keep it as separated as
possible. Not sure if you'll want skeleton type units etc. I.e.
addLabelAddress is compile unit only and I wanted something similar
for DW_AT_low_pc.


>
> You could just make "U" here a DwarfUnit, since you don't need any
> DwarfCompileUnit-specific behavior below. Or if you did the cast stuff
> suggested above, TheU would be a DwarfCompileUnit already.
>
> (could do this half-way, too. Without adding cast machinery to DwarfUnit,
> you could keep the == compile_unit check, then create a local variable with
> the static_cast of TheU, then use that both at the first comment (if you
> added the covariant return overrides) and here)
>

Could do.

>> +/// addLocalLabelAddress - Add a dwarf label attribute data and value
>> using
>> +/// DW_FORM_addr.
>> +void DwarfCompileUnit::addLocalLabelAddress(DIE *Die,
>> +                                            dwarf::Attribute Attribute,
>> +                                            MCSymbol *Label) {
>
>
> Is this refactoring out some common code? I don't see this function being
> removed from somewhere else. Might've been easier to see the refactor if it
> were done separately.
>

No, though it arguably should have been. It was because I didn't want
an address indirection - even with split dwarf. It's following the
local string stuff, etc. I'll go refactor those now. For now these
functions have been removed in the current commit.

>>
>> +  if (Label)
>> +    DD->addArangeLabel(SymbolCU(this, Label));
>> +
>> +  if (Label != NULL) {
>
>
> The same if condition twice? Written in two different ways?
>

Cut and paste from above, I'll refactor it as well :)

I've reapplied in r198222.

-eric