[PATCH] D29631: SystemZTargetTransformInfo cost functions and some common code changes

[Renato Golin via Phabricator via llvm-commits]

rengolin accepted this revision.
rengolin added a comment.
This revision is now accepted and ready to land.

Hi Jonas,

Really sorry for not getting back at this earlier. The generic changes are not intrusive enough to warrant a full refactoring, and they're mostly mechanical than existential.

However, I do spot a few problems with this approach, which can be fixed in following patches to the generic parts, since this is now holding a big change in SystemZ.

Right now, some cost functions receive the opcode only, thus not having the same conflict resolution power. Most don't need it, but the choice is currently based on one target's specific needs. Other targets may need to inspect operands and uses of other instructions, and having a different interface for the same kind of functions will confuse the hell out of anyone trying to add costs to their targets. :)

We need to make this more generic and actually pass the instruction instead of the opcode for all, which would clean up the whole mess. But that's for another day.

As is, Hal seems happy with the generic parts, so am I. Ulrich is happy with the System Z parts, so there's no reason not to approve this patch.

Thanks for the large contribution and the patience, and sorry it took so long. LGTM.

cheers,
--renato


https://reviews.llvm.org/D29631