[PATCH] D86977: [IRSim][IROutliner] Limit to extracting regions that only require inputs

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:316-330
+  // resolution that adds too much complexity at this stage.
+  if (Outputs.size() > 0) {
+    Region.IgnoreRegion = true;
+    return;
+  }
+
+  // TODO: Support regions with sunken allocas: values whose lifetimes are
----------------
Why cant these conditional blocks be merged? ie:


```
  if (Outputs.size() && ArgInputs.size() != OverallInputs.size()) {
    Region.IgnoreRegion = true;
    return;
  }
```


================
Comment at: llvm/lib/Transforms/IPO/IROutliner.cpp:510
+      findAddInputsOutputs(M, *OS);
+      if (!OS->IgnoreRegion)
+        OutlinedRegions.push_back(OS);
----------------
paquette wrote:
> If `findAddInputsOutputs` returned a bool, you could just do
> 
> ```
> if (findAddInputsOutputs(M, *OS) {
>   // Found the outputs and inputs. We can outline this.
>   OutlinedRegions.push_back(OS);
>   continue;
> }
> 
> // Couldn't find the outputs/inputs. Bail out.
> OS->reattachCandidate();
> ```
Can you add a comment on why `IgnoreRegion` dictates if push_back or reattachCandidate should be called here? 


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

https://reviews.llvm.org/D86977