[PATCH] D16083: [bugpoint] Teach bugpoint to reduce MDNodes

[Duncan P. N. Exon Smith via llvm-commits]

+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.

> On 2016-Jan-11, at 13:33, Keno Fischer <kfischer at college.harvard.edu> wrote:
> 
> loladiro created this revision.
> loladiro added reviewers: dexonsmith, tejohnson.
> loladiro added a subscriber: llvm-commits.
> Herald added a reviewer: tstellarAMD.
> Herald added a subscriber: arsenm.
> 
> 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.

My intuition is that you're liable to change the crash (rather than
reduce it) when touching arbitrary metadata like this.  Particularly
with debug info, this stuff is fairly cross-referential, and a `null`
reference can have some non-trivial meaning.

I think we'd have more luck with something that understood the
structure of the graph, at least at a high-level.

I've heard Adrian and Pete's ideas on that in the past, maybe they
can share here?

> http://reviews.llvm.org/D16083
> 
> Files:
>  include/llvm/Transforms/Utils/Cloning.h
>  lib/ExecutionEngine/Orc/IndirectionUtils.cpp
>  lib/IR/Metadata.cpp
>  lib/Target/AMDGPU/AMDGPUOpenCLImageTypeLoweringPass.cpp
>  lib/Transforms/Utils/CloneFunction.cpp
>  lib/Transforms/Utils/CloneModule.cpp
>  test/BugPoint/metadata.ll
>  tools/bugpoint-passes/TestPasses.cpp
>  tools/bugpoint/CrashDebugger.cpp
>  tools/bugpoint/ListReducer.h
>  unittests/Transforms/Utils/Cloning.cpp
> 
> <D16083.44545.patch>