[PATCH] D108656: [IROutliner] Using Canonical Values to find Corresponding Items between Regions

[Jessica Paquette via Phabricator via llvm-commits]

paquette accepted this revision.
paquette added a comment.
This revision is now accepted and ready to land.

This looks okay with nits.

I think the commit message can be more descriptive here though. It should explain why you're changing behaviour surrounding PHIs/stores etc.



================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:1145
+      // overall function.
+      StoreInst *SI = dyn_cast<StoreInst>(I);
+      assert(SI && "Is not a store instrution");
----------------
https://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates

Seems like it's more appropriate to use `cast` here than `dyn_cast`, considering you assert right after.


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:1146
+      StoreInst *SI = dyn_cast<StoreInst>(I);
+      assert(SI && "Is not a store instrution");
+
----------------



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

https://reviews.llvm.org/D108656