This is an archive of the discontinued LLVM Phabricator instance.

[codeview] Don't use the type visitor to merge types
ClosedPublic

Authored by rnk on Jul 17 2017, 10:45 AM.

Details

Summary

This didn't do much to speed things up, but it implements a FIXME, and I
think it's a nice simplification. We don't need the record kind switch.
We're doing that ourselves.

Depends on D35394

Event Timeline

rnk created this revision.Jul 17 2017, 10:45 AM
ruiu edited edge metadata.Jul 17 2017, 10:50 AM

I agree that this is a nice simplification.

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
274

What is the meaning of this boolean varaible? Looks like you are not using its value.

rnk added inline comments.Jul 17 2017, 10:58 AM
llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
274

It indicates if all type indices were successfully remapped. This gets passed into writeIdRecord / writeRecord, and then we decide to either write the record or map this index to LF_NOTTRANSLATED. It could be renamed AllIndicesMapped, or something.

ruiu accepted this revision.Jul 17 2017, 12:22 PM

LGTM

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
274

AllIndicesMapped would be a good name because I expected you would eventually return Succcess from this function as you did below.

291

Can you replace auto with a concrete type?

This revision is now accepted and ready to land.Jul 17 2017, 12:22 PM
rnk updated this revision to Diff 106921.Jul 17 2017, 12:48 PM
  • comments
This revision was automatically updated to reflect the committed changes.