[PATCH] D19075: mergefunc: avoid merge with loop metadata

[Hal Finkel via llvm-commits]

hfinkel added a comment.

In http://reviews.llvm.org/D19075#406978, @jfb wrote:

> In http://reviews.llvm.org/D19075#406966, @hfinkel wrote:
>
> > In http://reviews.llvm.org/D19075#406955, @jfb wrote:
> >
> > > In http://reviews.llvm.org/D19075#406907, @hfinkel wrote:
> > >
> > > > In http://reviews.llvm.org/D19075#406897, @jfb wrote:
> > > >
> > > > > In http://reviews.llvm.org/D19075#405855, @hfinkel wrote:
> > > > >
> > > > > > Do we not unqiue MDNodes? This seems like a lot of code for what I would have thought should be: Compare the pointer equality all MDNode operands except the first (because the first is the unique loop id).
> > > > >
> > > > >
> > > > > mergefunc wants an ordering for what it compares, so pure equality isn't sufficient (although `cmpLoopOperandMetadata` does start with `L == R`). It needs to know how to order in a stable way, so we also can't just order based on pointer value.
> > > >
> > > >
> > > > Do you not already have a way to do this for metadata in general?
> > >
> > >
> > > Not that I know, but I've been wrong before! I think a lot of the logic in mergefunc could be moved to the actual instruction (ordering as well as mergefunc's special hashing) and factored in a way that would reduce the risk of breakage (right now mergefunc breaks if not updated in sync).
> >
> >
> > This synchronization requirement is part of what worries me about embedding so much knowledge of the metadata format in this code.
> >
> > > I need to hack on things a bit more before I feel comfortable enough to propose a clean solution.
> >
> >
> > Okay. I suspect a general solution for metadata can be used here (minus the first operand for loop ids), once such a thing exists.
>
>
> Are you saying the current patch looks good with promise to generalize solution later, or are you asking for the generalized solution first?


I'm asking for the general solution first, specifically because it seems like it would be simpler than the current code, and more general. I really don't want to embed specific knowledge of the format of the loop metadata into this pass if not really necessary.


http://reviews.llvm.org/D19075