<div dir="ltr">This change is broken somehow, i'm debugging.<div>I have a suspicion it's related to the interaction between how the walker works and the def_chain iterator change.<br><div>You can see the cache hands back different results and if you turn on expensive checks, it'll assert.</div></div><div>If it's a simple matter of not caching, i'll fix it with the caching removal, otherwise, i'm going to see what's up and probably revert this.</div><div><br></div><div>(Nobody other than the patch i have for newgvn refines stores, so nobody else is broken, or i'd just revert now)</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Apr 1, 2017 at 10:09 PM, Daniel Berlin via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dannyb<br>
Date: Sun Apr 2 00:09:15 2017<br>
New Revision: 299322<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=299322&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=299322&view=rev</a><br>
Log:<br>
MemorySSA: Add support for caching clobbering access in stores<br>
<br>
Summary:<br>
This enables us to cache the clobbering access for stores, despite the<br>
fact that we can't rewrite the use-def chains themselves.<br>
<br>
Early testing shows that, after this change, for larger testcases, it will be a significant net positive (memory and time) to remove the walker caching.<br>
<br>
Reviewers: george.burgess.iv, davide<br>
<br>
Subscribers: Prazek, llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D31567" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D31567</a><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/<wbr>Transforms/Utils/MemorySSA.h<br>
llvm/trunk/lib/Transforms/<wbr>Utils/MemorySSA.cpp<br>
llvm/trunk/lib/Transforms/<wbr>Utils/MemorySSAUpdater.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/<wbr>Transforms/Utils/MemorySSA.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h?rev=299322&r1=299321&r2=299322&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/Transforms/Utils/<wbr>MemorySSA.h?rev=299322&r1=<wbr>299321&r2=299322&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/include/llvm/<wbr>Transforms/Utils/MemorySSA.h (original)<br>
+++ llvm/trunk/include/llvm/<wbr>Transforms/Utils/MemorySSA.h Sun Apr 2 00:09:15 2017<br>
@@ -246,6 +246,16 @@ public:<br>
return MA->getValueID() == MemoryUseVal || MA->getValueID() == MemoryDefVal;<br>
}<br>
<br>
+ // Sadly, these have to be public because they are needed in some of the iterators.<br>
+ virtual bool isOptimized() const = 0;<br>
+ virtual MemoryAccess *getOptimized() const = 0;<br>
+ virtual void setOptimized(MemoryAccess *) = 0;<br>
+<br>
+ /// \brief Reset the ID of what this MemoryUse was optimized to, causing it to<br>
+ /// be rewalked by the walker if necessary.<br>
+ /// This really should only be called by tests.<br>
+ virtual void resetOptimized() = 0;<br>
+<br>
protected:<br>
friend class MemorySSA;<br>
friend class MemorySSAUpdater;<br>
@@ -254,8 +264,12 @@ protected:<br>
: MemoryAccess(C, Vty, BB, 1), MemoryInst(MI) {<br>
setDefiningAccess(DMA);<br>
}<br>
-<br>
- void setDefiningAccess(MemoryAccess *DMA) { setOperand(0, DMA); }<br>
+ void setDefiningAccess(MemoryAccess *DMA, bool Optimized = false) {<br>
+ setOperand(0, DMA);<br>
+ if (!Optimized)<br>
+ return;<br>
+ setOptimized(DMA);<br>
+ }<br>
<br>
private:<br>
Instruction *MemoryInst;<br>
@@ -288,20 +302,18 @@ public:<br>
<br>
void print(raw_ostream &OS) const override;<br>
<br>
- void setDefiningAccess(MemoryAccess *DMA, bool Optimized = false) {<br>
- if (Optimized)<br>
- OptimizedID = DMA->getID();<br>
- MemoryUseOrDef::<wbr>setDefiningAccess(DMA);<br>
+ virtual void setOptimized(MemoryAccess *DMA) override {<br>
+ OptimizedID = DMA->getID();<br>
}<br>
<br>
- bool isOptimized() const {<br>
+ virtual bool isOptimized() const override {<br>
return getDefiningAccess() && OptimizedID == getDefiningAccess()->getID();<br>
}<br>
<br>
- /// \brief Reset the ID of what this MemoryUse was optimized to, causing it to<br>
- /// be rewalked by the walker if necessary.<br>
- /// This really should only be called by tests.<br>
- void resetOptimized() { OptimizedID = INVALID_MEMORYACCESS_ID; }<br>
+ virtual MemoryAccess *getOptimized() const override {<br>
+ return getDefiningAccess();<br>
+ }<br>
+ virtual void resetOptimized() override { OptimizedID = INVALID_MEMORYACCESS_ID; }<br>
<br>
protected:<br>
friend class MemorySSA;<br>
@@ -333,7 +345,8 @@ public:<br>
<br>
MemoryDef(LLVMContext &C, MemoryAccess *DMA, Instruction *MI, BasicBlock *BB,<br>
unsigned Ver)<br>
- : MemoryUseOrDef(C, DMA, MemoryDefVal, MI, BB), ID(Ver) {}<br>
+ : MemoryUseOrDef(C, DMA, MemoryDefVal, MI, BB), ID(Ver),<br>
+ Optimized(nullptr), OptimizedID(INVALID_<wbr>MEMORYACCESS_ID) {}<br>
<br>
// allocate space for exactly one operand<br>
void *operator new(size_t s) { return User::operator new(s, 1); }<br>
@@ -343,6 +356,17 @@ public:<br>
return MA->getValueID() == MemoryDefVal;<br>
}<br>
<br>
+ virtual void setOptimized(MemoryAccess *MA) override {<br>
+ Optimized = MA;<br>
+ OptimizedID = getDefiningAccess()->getID();<br>
+ }<br>
+ virtual MemoryAccess *getOptimized() const override { return Optimized; }<br>
+ virtual bool isOptimized() const override {<br>
+ return getOptimized() && OptimizedID == getDefiningAccess()->getID();<br>
+ }<br>
+ virtual void resetOptimized() override { OptimizedID = INVALID_MEMORYACCESS_ID; }<br>
+<br>
+<br>
void print(raw_ostream &OS) const override;<br>
<br>
protected:<br>
@@ -352,6 +376,8 @@ protected:<br>
<br>
private:<br>
const unsigned ID;<br>
+ MemoryAccess *Optimized;<br>
+ unsigned int OptimizedID;<br>
};<br>
<br>
template <><br>
@@ -1075,10 +1101,15 @@ struct def_chain_iterator<br>
<br>
def_chain_iterator &operator++() {<br>
// N.B. liveOnEntry has a null defining access.<br>
- if (auto *MUD = dyn_cast<MemoryUseOrDef>(MA))<br>
- MA = MUD->getDefiningAccess();<br>
- else<br>
+ if (auto *MUD = dyn_cast<MemoryUseOrDef>(MA)) {<br>
+ if (MUD->isOptimized())<br>
+ MA = MUD->getOptimized();<br>
+ else<br>
+ MA = MUD->getDefiningAccess();<br>
+ } else {<br>
MA = nullptr;<br>
+ }<br>
+<br>
return *this;<br>
}<br>
<br>
<br>
Modified: llvm/trunk/lib/Transforms/<wbr>Utils/MemorySSA.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=299322&r1=299321&r2=299322&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Transforms/Utils/MemorySSA.<wbr>cpp?rev=299322&r1=299321&r2=<wbr>299322&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/<wbr>Utils/MemorySSA.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/<wbr>Utils/MemorySSA.cpp Sun Apr 2 00:09:15 2017<br>
@@ -2181,9 +2181,9 @@ MemorySSA::CachingWalker::<wbr>getClobberingM<br>
// If this is an already optimized use or def, return the optimized result.<br>
// Note: Currently, we do not store the optimized def result because we'd need<br>
// a separate field, since we can't use it as the defining access.<br>
- if (MemoryUse *MU = dyn_cast<MemoryUse>(<wbr>StartingAccess))<br>
- if (MU->isOptimized())<br>
- return MU->getDefiningAccess();<br>
+ if (auto *MUD = dyn_cast<MemoryUseOrDef>(<wbr>StartingAccess))<br>
+ if (MUD->isOptimized())<br>
+ return MUD->getOptimized();<br>
<br>
const Instruction *I = StartingAccess->getMemoryInst(<wbr>);<br>
UpwardsMemoryQuery Q(I, StartingAccess);<br>
@@ -2199,8 +2199,8 @@ MemorySSA::CachingWalker::<wbr>getClobberingM<br>
if (<wbr>isUseTriviallyOptimizableToLiv<wbr>eOnEntry(*MSSA->AA, I)) {<br>
MemoryAccess *LiveOnEntry = MSSA->getLiveOnEntryDef();<br>
Cache.insert(StartingAccess, LiveOnEntry, Q.StartingLoc, Q.IsCall);<br>
- if (MemoryUse *MU = dyn_cast<MemoryUse>(<wbr>StartingAccess))<br>
- MU->setDefiningAccess(<wbr>LiveOnEntry, true);<br>
+ if (auto *MUD = dyn_cast<MemoryUseOrDef>(<wbr>StartingAccess))<br>
+ MUD->setDefiningAccess(<wbr>LiveOnEntry, true);<br>
return LiveOnEntry;<br>
}<br>
<br>
@@ -2217,8 +2217,8 @@ MemorySSA::CachingWalker::<wbr>getClobberingM<br>
DEBUG(dbgs() << *DefiningAccess << "\n");<br>
DEBUG(dbgs() << "Final Memory SSA clobber for " << *I << " is ");<br>
DEBUG(dbgs() << *Result << "\n");<br>
- if (MemoryUse *MU = dyn_cast<MemoryUse>(<wbr>StartingAccess))<br>
- MU->setDefiningAccess(Result, true);<br>
+ if (auto *MUD = dyn_cast<MemoryUseOrDef>(<wbr>StartingAccess))<br>
+ MUD->setDefiningAccess(Result, true);<br>
<br>
return Result;<br>
}<br>
<br>
Modified: llvm/trunk/lib/Transforms/<wbr>Utils/MemorySSAUpdater.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSAUpdater.cpp?rev=299322&r1=299321&r2=299322&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Transforms/Utils/<wbr>MemorySSAUpdater.cpp?rev=<wbr>299322&r1=299321&r2=299322&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/<wbr>Utils/MemorySSAUpdater.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/<wbr>Utils/MemorySSAUpdater.cpp Sun Apr 2 00:09:15 2017<br>
@@ -451,8 +451,8 @@ void MemorySSAUpdater::<wbr>removeMemoryAcces<br>
<br>
while (!MA->use_empty()) {<br>
Use &U = *MA->use_begin();<br>
- if (MemoryUse *MU = dyn_cast<MemoryUse>(U.getUser(<wbr>)))<br>
- MU->resetOptimized();<br>
+ if (auto *MUD = dyn_cast<MemoryUseOrDef>(U.<wbr>getUser()))<br>
+ MUD->resetOptimized();<br>
U.set(NewDefTarget);<br>
}<br>
}<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>