Index: lib/Transforms/Scalar/GVNHoist.cpp =================================================================== --- lib/Transforms/Scalar/GVNHoist.cpp +++ lib/Transforms/Scalar/GVNHoist.cpp @@ -193,14 +193,11 @@ VN.setAliasAnalysis(AA); VN.setMemDep(MD); bool Res = false; + MemorySSA M(F, AA, DT); + MSSA = &M; // FIXME: use lazy evaluation of VN to avoid the fix-point computation. while (1) { - // FIXME: only compute MemorySSA once. We need to update the analysis in - // the same time as transforming the code. - MemorySSA M(F, AA, DT); - MSSA = &M; - // Perform DFS Numbering of instructions. unsigned I = 0; for (const BasicBlock *BB : depth_first(&F.getEntryBlock())) @@ -208,15 +205,15 @@ DFSNumber.insert({&Inst, ++I}); auto HoistStat = hoistExpressions(F); - if (HoistStat.first + HoistStat.second == 0) { + if (HoistStat.first + HoistStat.second == 0) return Res; - } - if (HoistStat.second > 0) { + + if (HoistStat.second > 0) // To address a limitation of the current GVN, we need to rerun the - // hoisting after we hoisted loads in order to be able to hoist all - // scalars dependent on the hoisted loads. Same for stores. + // hoisting after we hoisted loads or stores in order to be able to + // hoist all scalars dependent on the hoisted ld/st. VN.clear(); - } + Res = true; // DFS numbers change when instructions are hoisted: clear and recompute. @@ -725,6 +722,9 @@ if (!Repl || firstInBB(I, Repl)) Repl = I; + MemoryAccess *NewMemAcc = nullptr; + Instruction *ClonedRepl = nullptr; + if (Repl) { // Repl is already in HoistPt: it remains in place. assert(allOperandsAvailable(Repl, HoistPt) && @@ -742,7 +742,37 @@ !makeGepOperandsAvailable(Repl, HoistPt, InstructionsToHoist)) continue; - Repl->moveBefore(HoistPt->getTerminator()); + // Before moving the instruction, get its MSSA access. + MemoryUseOrDef *OldMemAcc = nullptr; + if (MemoryAccess *MA = MSSA->getMemoryAccess(Repl)) + OldMemAcc = dyn_cast(MA); + + if (!OldMemAcc) { + // Move the instruction at the end of HoistPt. + Repl->moveBefore(HoistPt->getTerminator()); + } else { + // As MSSA does not support moving instructions around, clone the + // instruction to be able to create a new MemoryAccess. + ClonedRepl = Repl->clone(); + ClonedRepl->insertBefore(HoistPt->getTerminator()); + + // Conservatively discard any optimization hints, they may differ on + // the other paths. + ClonedRepl->dropUnknownNonDebugMetadata(); + ClonedRepl->intersectOptionalDataWith(Repl); + combineKnownMetadata(ClonedRepl, Repl); + + // Replace uses of Repl with ClonedRepl. + Repl->replaceAllUsesWith(ClonedRepl); + + // The definition of this ld/st will not change: ld/st hoisting is + // legal when the ld/st is not moved past its current definition. + MemoryAccess *Def = OldMemAcc->getDefiningAccess(); + NewMemAcc = MSSA->createMemoryAccessInBB(ClonedRepl, Def, HoistPt, + MemorySSA::End); + OldMemAcc->replaceAllUsesWith(NewMemAcc); + MSSA->removeMemoryAccess(OldMemAcc); + } } if (isa(Repl)) @@ -775,11 +805,26 @@ } else if (isa(Repl)) { ++NumCallsRemoved; } - Repl->intersectOptionalDataWith(I); - combineKnownMetadata(Repl, I); - I->replaceAllUsesWith(Repl); + + if (ClonedRepl) { + // Update the uses of the old MSSA access with NewMemAcc. + MemoryAccess *OldMA = MSSA->getMemoryAccess(I); + OldMA->replaceAllUsesWith(NewMemAcc); + MSSA->removeMemoryAccess(OldMA); + ClonedRepl->intersectOptionalDataWith(I); + combineKnownMetadata(ClonedRepl, I); + I->replaceAllUsesWith(ClonedRepl); + } else { + Repl->intersectOptionalDataWith(I); + combineKnownMetadata(Repl, I); + I->replaceAllUsesWith(Repl); + } I->eraseFromParent(); } + + // Remove the instruction that we cloned. + if (ClonedRepl) + Repl->eraseFromParent(); } NumHoisted += NL + NS + NC + NI;