[PATCH] D37121: [DivRemHoist] add a pass to move div/rem pairs into the same block (PR31028)

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 25 17:11:24 PDT 2017


dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/DivRemHoist.cpp:83
+        continue;
+      RemInst->moveBefore(DivInst);
+      NumHoisted++;
----------------
spatel wrote:
> dberlin wrote:
> > It should go after, to retain the relative ordering.
> Agreed, that feels better. But now I remember why I picked "moveBefore"...there is no "moveAfter". Is this an oversight of the API or is there something about 'after' that requires different logic? Looks like other places do:
>       I->removeFromParent();
>       I->insertAfter(OtherInst);
> 
This seems strange to me.  The Eli tends to be much better than me at explaining the good reason something is the way it is that i've missed, so let's see what he says.

MemorySSA also uses iplists and has moveAfter.

All it requires there is the equivalent of:
  moveBefore(*MovePos->getParent(), ++MovePos->getIterator());

(since splice inserts before)



https://reviews.llvm.org/D37121





More information about the llvm-commits mailing list