[PATCH] D63885: [LOOPINFO] Introduce the loop guard API.

Whitney via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 3 06:22:44 PDT 2019


Whitney marked 2 inline comments as done.
Whitney added a comment.

> I like your distinction between weak and proper guards. We probably need better terminology, but both are useful. For the moment, I'd suggest we start with the stronger form and add the weaker one later if needed.

For the moment, I strengthened the definition of loop guard by disallowing non safe instructions guarded by the branch but not in the loop, so there is only the stronger form. I agree that we should add the weaker form later only when needed.



================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:732
+  /// dominator tree.
+  BranchInst *getLoopGuardBranch(const DominatorTree &DT,
+                                 const PostDominatorTree &PDT) const;
----------------
reames wrote:
> Whitney wrote:
> > reames wrote:
> > > PostDom is not widely available in the optimizer.  I strongly suspect that a narrower, but more widely usable, API would be better in practice.
> > I heard that there is an intension to make PostDom more widely available, and I think that's the right thing to do. What do you think?
> It has "been the intention" for years.  Unless someone has committed to doing the work in the next six months, I'd advice not relying on it.  
@Meinersbur @jdoerfert @kbarton Do you know if there is anyone working / plan to work on it?


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:737
+  /// dominates the loop header, and the other successor postdominates the loop
+  /// header, i.e. the branch instruction \p BI is a loop guard.
+  bool isLoopGuardBranch(const BranchInst &BI, const DominatorTree &DT,
----------------
reames wrote:
> Whitney wrote:
> > reames wrote:
> > > Your definition would seem to disallow:
> > > if (C) return;
> > > for (...) {}
> > > 
> > > That is C wouldn't be considered a guard condition.  Intentional?  
> > I think this is fine, in most cases, LLVM tries to common the return blocks, so `if (C)` will jump to the common exit block, and this algorithm will work. Although we can intensionally write LLVMIR with multiple return blocks, I don't think it will be the common case. 
> Put two calls on the return paths to different targets and we don't merge them.
Do you mind giving me an example? I tried the code below, and that doesn't work. 
```
void bar1();
void bar2();
int foo(bool cond, int *A) {
  if (cond) return 1;
  for (long i = 0; i < 100; ++i) {
    A[i] = 0;
  }
  bar1();
  bar2();
  return 0;
}
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63885





More information about the llvm-commits mailing list