[PATCH] D53304: [GISel]: Allow Unused G_PHIs to be DCEd

[Daniel Sanders via Phabricator via llvm-commits]

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

LGTM with a nit. Also we should consider whether we ought to be calling isSafeToMove() when we're not moving things.



================
Comment at: lib/CodeGen/GlobalISel/Utils.cpp:140
   bool SawStore = false;
-  if (!MI.isSafeToMove(/*AA=*/nullptr, SawStore))
+  if (!MI.isSafeToMove(/*AA=*/nullptr, SawStore) &&
+      MI.getOpcode() != TargetOpcode::G_PHI)
----------------
As a side note, isSafeToMove() doesn't seem like the right check in any case since we're not planning to move it. We really want a mayHaveSideEffects() or isSafeToErase(). That said, the contents would be largely the same so it makes little difference aside from clearer labelling of the intent.


================
Comment at: lib/CodeGen/GlobalISel/Utils.cpp:141
+  if (!MI.isSafeToMove(/*AA=*/nullptr, SawStore) &&
+      MI.getOpcode() != TargetOpcode::G_PHI)
     return false;
----------------
We should use isPhi() instead. It's just as valid to delete dead PHI's we have in the late stages of the pipeline as it is to delete dead G_PHI's in the early stages


Repository:
  rL LLVM

https://reviews.llvm.org/D53304