[PATCH] D26088: Don't leave unused divs/rems sitting around in BypassSlowDivision.

[Justin Lebar via llvm-commits]

jlebar marked an inline comment as done.
jlebar added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/BypassSlowDivision.cpp:251
+  // Above we eagerly create divs and rems, as pairs, so that we can efficiently
+  // create divrem machine instructions.  Now erase unused any divs / rems so we
+  // don't leave extra instructions sitting around.
----------------
tra wrote:
> Should it be "erase any unused divs/rems..."?
Indeed, thanks!


================
Comment at: llvm/lib/Transforms/Utils/BypassSlowDivision.cpp:260
+      for (Value *Operand : Phi->operand_values())
+        if (Instruction *I = dyn_cast<Instruction>(Operand))
+          ToErase.push_back(I);
----------------
tra wrote:
> What's supposed to happen with operands that are not instructions?
> Destruction of unused subgraph sounds like a common operation, perhaps we already have a function to do it?
> What's supposed to happen with operands that are not instructions?

We don't need to delete them if they're constants.

> Destruction of unused subgraph sounds like a common operation, perhaps we already have a function to do it?

Aha, found it.  Thanks.  Also, this fixes a bug -- we weren't trying hard enough to delete trivially-dead things here.  mkuper pointed me in the right direction and I now have a test.


https://reviews.llvm.org/D26088