[PATCH] First set of patches for type uniquing

Eric Christopher echristo at gmail.com
Thu Aug 22 12:48:59 PDT 2013


Hi Manman,

I'm still looking at the patch but one comment that will affect a lot
of the text of the patch is that I think the unique name should be
passed into the type creation functions, not created inside the
backend. This means that each front end can use the definition of
"unique" that most appeals to them.

Also I'm not sure why you've got an actual hashing algorithm in there.
Can you explain?

Couple other things I noticed:

+  // If the consumer of DIBuilder does not provide a unique name, generate one
+  // here.
+  MDString *UniqueStr = UniqueName.empty() ?
+                        getUniqueTypeName(dwarf::DW_TAG_structure_type, Name) :
+                        MDString::get(VMContext, UniqueName);

I don't think we should do this. Have an example where we'd want to?

+        Map.insert(std::make_pair(
+                   std::make_pair(Ty.getUniqueName(), Ty.isForwardDecl()),
+                   Ty));

A pair of a pair is a bit icky here as well.

This should be enough to start discussion/revision of the patch.

-eric

On Wed, Jul 31, 2013 at 4:37 PM, Manman Ren <manman.ren at gmail.com> wrote:
>
> After cleaning up usage of the verify function and all the testing cases,
> the first set of patches are ready for review :)
>
> 3 patches are included: one for clang (tu_clang_07312013.patch), one for
> code change of llvm (tu_07312013.patch), and one for updating all the
> testing cases (tu_test_07312013.patch).
>
> The following are implemented:
> 1> DIType will have a unique name at field 9.
>      For DIBasicType, DICompositeType, DIDerivedType, the fields after will
> be shifted by 1.
> 2> I am going to update usage of DIType in phases:
>      At the first batch, getContainingType of DICompositeType and
> DISubprogram will be replaced with a MDString, as well as getClassType of
> DIDerivedType.
> 3> The verifier is updated according to 1 and 2.
> 4> DwarfDebug has a TypeMap.
>       It also has a helper function lookupType to find the corresponding
> DIType given a type name.
> 5> how to generate the unique name?
>       The clang frontend has two helper functions: getUniqueTagTypeName and
> getUniqueTypeName
>           The first one handles TagType and it uses ODR (One Definition
> Rule) for external c++ types. It is currently called for struct, class,
> union and enum.
>           The second one can be used for general types, it uses mangled name
> appended with CU's directory and file name to provide uniqueness across CUs.
> It is currently used to generate unique name for subroutine type. For other
> types, DIBuilder will provide a unique name. Clang can handle more types in
> follow-on patches if it can provide a better name than DIBuilder.
>       DIBuilder also has a helper function getUniqueTypeName, this is called
> when consumer of DIBuilder does not provide a unique type name. This is
> needed for dragonegg, before it provides a unique name (not sure whether it
> is needed).
> 6> All DITypes created by DIBuilder will be added to RetainTypes.
>
> Comments and feedback are welcome.
>
> Manman
>



More information about the llvm-commits mailing list