jdoerfert added a comment.

@nikic Is this OK or do you want it split?



================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:661
+      }
+    }
   }
----------------
atmnpatel wrote:
> nikic wrote:
> > These fixes look unrelated. Is it possible to test them separately?
> So my understanding is that the actual line that fixes the compile time error 
> is 651, that is, just having that line fixes the compile time error. I would 
> assume its because before I didn't tell the dominator tree to remove the edge 
> connecting the preheader and header, and not having that cascade, GVN was 
> unable to iterate properly in some cases over the (now) dead blocks because 
> it wasn't updated in LLVM's internal structures. The actual error was from 
> the iteration in GVN::assignValNumForDeadCode() where it would try to iterate 
> through a block that partially existed but didn't really.
> 
> The lines 652-660 that update MemorySSA I added because in the other more 
> general case above, we seem to update MemorySSA right after updating the 
> Dominator Tree.
Nit: Move DTU into the conditional


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86844/new/

https://reviews.llvm.org/D86844

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to