[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