[PATCH] D152358: [CGP] Remove operand of llvm.assume more aggressively.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:707
 
+// Returns ture if transitive closure of instruction I is trivially dead
+// if no uses. In this case Closure contains this transitive closure. Returns
----------------
sentence sounds like it's missing a word here?


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:711
+// llvm.assume is considered as trivial dead instruction.
+static bool FindClosureOfWouldBeTriviallyDeadInstructions(
+    Instruction *I, SmallPtrSetImpl<Instruction *> &Closure,
----------------
Start with lowercase


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:711
+// llvm.assume is considered as trivial dead instruction.
+static bool FindClosureOfWouldBeTriviallyDeadInstructions(
+    Instruction *I, SmallPtrSetImpl<Instruction *> &Closure,
----------------
arsenm wrote:
> Start with lowercase
I'm not sure what closure means in this context


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:715
+  assert(wouldInstructionBeTriviallyDead(I, TLInfo) && "expect trivially dead");
+  Closure.insert(I);
+  // Simple Case. no users.
----------------
Do this after the early exit?


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:739-740
+
+// For the given trivially dead instruction if no uses, collect all instruction
+// instruction which can be removed together with it.
+static void FindInstructionsToRemoveWithAssumeOperand(
----------------
How is this different from RecursivelyDeleteTriviallyDeadInstructions?


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

https://reviews.llvm.org/D152358