Page MenuHomePhabricator

[bugpoint] Teach bugpoint to reduce MDNodes
Needs ReviewPublic

Authored by loladiro on Jan 11 2016, 1:33 PM.



This is essentially pretty simple, it first walks through the the contents of
the module starting from instructions/named metadata to find all MDNodes and
then does a standard list reduction in order to cut down the list of which
metadata nodes to keep. To actually implement the MDNode removal, it creates
a temporary Node, inserts it into the VMap, and laters RAUWs that to null.
This works great, except that the verifier really doesn't like null values in
some MDTuples. Thus, we need some extra logic to also fix up any MDTuples,
which would otherwise contain a null.

The one ugly point here is that in order to do the above, we need to allow
temporary metadata during cloning. To make this possible, I had to modify
CloneModule and CloneFunctionInto, in order to allow passing RemapFlags
through. I think this API change is fine, but if there's a different way
to accomplish the same thing, I'd be happy to change that. Naturally, I can
split that part out into a separate review.

There's also a small fix to pass through AllowTemps recursively in
resolveCylces, which was needed to make the above work.

Diff Detail

Event Timeline

loladiro updated this revision to Diff 44545.Jan 11 2016, 1:33 PM
loladiro retitled this revision from to [bugpoint] Teach bugpoint to reduce MDNodes.
loladiro updated this object.
loladiro added reviewers: dexonsmith, tejohnson.
loladiro added a subscriber: llvm-commits.
dexonsmith edited edge metadata.Jan 11 2016, 1:52 PM
dexonsmith added a subscriber: dexonsmith.

+Pete and Adrian, from whom I've heard ideas about bugpointing metadata
(and debug info specifically) before.

This is great; I'm glad you're working on this.

loladiro updated this revision to Diff 44630.Jan 12 2016, 6:01 AM
loladiro edited edge metadata.

Rebased and add switch to disable MD reduction.

loladiro updated this revision to Diff 44792.Jan 13 2016, 2:28 PM

Add two debuginfo specific passes to speed convergence of the reduction:

  • Canonicalize all DIFiles into a single node
  • Resolve All type references by string into explicit references

These two passes work around the two most commonly encountered verifier
failures when nulling out nodes, significantly speeding up the
convergence of the MD reducer (down to seconds from several minutes
on an LTO build of clang).

loladiro updated this revision to Diff 44793.Jan 13 2016, 2:29 PM

Remove accidentally added second commit

loladiro updated this revision to Diff 44932.Jan 14 2016, 3:10 PM

Recollect all MDNodes to exclude unreachable ones, as we can't be sure they're valid (since the verifier didn't get to them) and won't assert when we look at them.

slarin added a subscriber: slarin.Jan 15 2016, 9:52 AM

Sorry to come in cold on the discussion, but let me ask a naive question - Why can't we teach CloneModule to drop unnecessary metadata in the same way we use a selector function for GlobalVariables (ShouldCloneDefinition) - why copy it in the first place?

Dropping metadata is already implemented in the ValueMaterializer interface. However, I don't think the other things I'm doing are doable with that interface. I suppose we could extend it? @dexonsmith, thoughts on that?

Sorry - I told you I am starting cold :)

Maybe I am talking about a different issue here - I refer to this loop in CloneModule.cpp:

158 // And named metadata....
159 for (Module::const_named_metadata_iterator I = M->named_metadata_begin(),
160 E = M->named_metadata_end(); I != E; ++I) {
161 const NamedMDNode &NMD = *I;
162 NamedMDNode *NewNMD = New->getOrInsertNamedMetadata(NMD.getName());
163 for (unsigned i = 0, e = NMD.getNumOperands(); i != e; ++i)
164 NewNMD->addOperand(MapMetadata(NMD.getOperand(i), VMap));
165 }

It seems to me that it indiscriminately will copy all content of if one is present - at least this is exactly what it does in my case...

Sorry again if this is unrelated...

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Ah, that's for named metadata. I think it would be reasonable to call isMetadataNeeded there.

Got it. Thanks...

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

dexonsmith resigned from this revision.Oct 6 2020, 3:45 PM