[PATCH] D40348: [PowerPC] Follow-up to r318436 to get the missed CSE opportunities

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

In https://reviews.llvm.org/D40348#941119, @bogner wrote:

> This change is really three different changes. In general it's easier to review changes that only do one thing. The DAGCombiner change here looks correct, and I took a cursory glance at the PPC changes and didn't see anything wrong.


First off, thanks for the review Justin.

I agree that this kind of conflates multiple conceptual changes and am happy to split them out on the commit (or even just commit the first one now and post the other two as separate reviews).

1. The DAGCombiner change
2. Sign-extending the immediate for the wider constant to be stored
3. Improvements to materialization of constants

I felt that posting the DAGCombiner change in isolation wouldn't provide enough motivation so I wrapped it into this. Sorry about that.



================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:862-863
+  unsigned MaxTruncation = 0;
+  for (SDNode::use_iterator Use = N->use_begin(), UseEnd = N->use_end();
+       Use != UseEnd; ++Use) {
+    unsigned Opc =
----------------
bogner wrote:
> This can be simplified using range-for and N->uses().
Good point. I wonder why I did this. I think the original implementation needed to increment the iterator within the loop. In any case, it is now good for a range-for. Thanks for pointing it out.


Repository:
  rL LLVM

https://reviews.llvm.org/D40348