[PATCH] D64162: Summary: [Attributor] Liveness analysis abstract attribute used to indicate which BasicBlocks are dead and can therefore be ignored.

[Stefan Stipanovic via Phabricator via llvm-commits]

sstefan1 marked 6 inline comments as done.
sstefan1 added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1139
+      // If it is not in live set, then it is dead.
+      AssumedDeadBlocks.insert(&BB);
+
----------------
jdoerfert wrote:
> I'm unsure if we need two sets here. I was hoping: "not in AssumedLive" means "assumed dead". 
Agreed.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1170
+    }
+  }
+
----------------
jdoerfert wrote:
> Why do we need this logic. Above, you already explore exhaustively starting from old assumed `noreturn` instructions. I hoped that was enough.
This was mainly left over code from last diff. Should have been removed.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1172
+
+  if (AssumedDeadBlocks.size() != 0) {
+    indicateOptimisticFixpoint();
----------------
jdoerfert wrote:
> For now, I'd just return changed/unchanged. That can be determined by the return values of `explorePath`, e.g., if we did not explore any new instruction nothing has changed.
I'm not sure I can just return unchanged after one false explore. I'd stick with this for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64162