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

[Whitney via Phabricator via llvm-commits]

Whitney marked an inline comment as done.
Whitney added inline comments.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:415
+
+      if (!isGuaranteedToTransferExecutionToSuccessor(Succ))
+        return true;
----------------
Meinersbur wrote:
> Whitney wrote:
> > Meinersbur wrote:
> > > Whitney wrote:
> > > > Meinersbur wrote:
> > > > > If you want to disallow throwing instructions, how did  you conclude that `isGuaranteedToTransferExecutionToSuccessor` is what you want and not `->mayThrow`?
> > > > > 
> > > > > There might be dynamically endless loops as well that isGuaranteedToTransferExecutionToSuccessor does not check (that is, only if the endless loop was inside a call).
> > > > I chose `isGuaranteedToTransferExecutionToSuccessor` instead of `->mayThrow` is because it consider more cases, for example it checks if memory operation trap. `isGuaranteedToTransferExecutionToSuccessor` is a superset of `->mayThrow`. 
> > > > 
> > > > What is dynamically endless loops? 
> > > > What is dynamically endless loops?
> > > ```
> > > header:
> > >   %cmp = icmp ule i32 0, %a ; anything that always returns true but LLVM cannot figure out
> > >   br i1 %cmp, label %header, label %exit
> > > 
> > > exit:
> > > ```
> > I would like to make sure I understand the concern correctly. Is the concern that the guard guards an dynamically endless loops before the loop of interested?
> Previously, you mentioned:
> 
> > In general, I would like to make sure if entered the guard region, then the loop is definitely going to be executed.
> 
> `isGuaranteedToTransferExecutionToSuccessor` is not sufficient for this goal:
> 
> 1. There might be such an endless loop between the guard's branch target and the loop of interest.
> 
> 2. There can be other branches between, including, but not limited to, other loop guards.
Thanks for the explanation. That's true. Thinking to disallow conditional branches in the guarded region but not in the loop. Not sure if that's the best way of solving this problem. What do you think?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63885