[PATCH] D78592: [mlir][SCCP] Add support for propagating across symbol based calls

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/lib/Transforms/SCCP.cpp:579
+  // The callable of this call can't be resolved, mark any results overdefined.
+  if (!callableOp)
+    return markAllOverdefined(op, callResults);
----------------
bondhugula wrote:
> Will the default constructed value from 'lookup' above be nullptr?
Yes.


================
Comment at: mlir/lib/Transforms/SCCP.cpp:774
+  // If this terminator is not "return-like", conservatively mark all of the
+  // call-site results as overdefined.
+  auto callableResultLattices = latticeIt->second.getResultLatticeValues();
----------------
bondhugula wrote:
> But this terminator might not be the only terminator in the region of the callable op. Why mark call site results overdefined? You'd just propagate to successors the same way as for non-callable op region terminators that aren't return-like. ? You do have a test case ("complex_inner_if") where you have a cond_br in the callable. I was expecting that to not work as a result of this.
This branch is only taken when there are no CFG successors. This is the same behavior as the region case, i.e. if we see an "exiting" terminator that isn't return-like we assume all exit values are overdefined.


================
Comment at: mlir/test/Transforms/sccp-callgraph.mlir:106
+
+// CHECK-LABEL: func @non_call_users(
+func @non_call_users() -> (i32, i32) {
----------------
bondhugula wrote:
> But this function a 'call user'. The non-call user is further below. 
The function name is more of a test case name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78592