[PATCH] D157689: [GuardWidening] Refactor to work with the list of checks to widen/hoist

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:549
+    if (!PDT)
+      return true;
     return !PDT->dominates(DominatedBlock, DominatingBlock);
----------------
Nit: unrelated change.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:697
+                                    SmallVectorImpl<Value *> &ChecksToWiden,
+                                    Instruction *InsertPt, Value *&Result) {
   using namespace llvm::PatternMatch;
----------------
Can we return the result instead of returning a boolean and storing the result in a separate argument?
i.e.

```
std::optional<Value *> GuardWideningImpl::mergeChecks(SmallVectorImpl<Value *> &ChecksToHoist,
                                    SmallVectorImpl<Value *> &ChecksToWiden,
                                    Instruction *InsertPt)
```

If this works, pls land this separately before this patch (don't need explicit review) and then rebase on this revision. Makes it easier for review. Thanks. 

In places where only the boolean is required, you can just check the optional has value (In `isWideningCondProfitable` for example).



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

https://reviews.llvm.org/D157689