[PATCH] D75923: [Attributor] Deduction based on recursive path exploration

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1064
+      return true;
+    };
+
----------------
uenoku wrote:
> jdoerfert wrote:
> > What happens if there is a different terminator here? I mean do we handle them all correctly?
> Does "different terminator" mean like `switch` or other terminator? If you mean so, I think it can handle such cases.
Yes, `switch`, `ret`, ..., need to be handled correctly


================
Comment at: llvm/test/Transforms/Attributor/dereferenceable-1.ll:451
   tail call void @rec-branch-2(i32 1, i32 1, i32 1, i32* %ptr)
   br label %if.end8
 
----------------
uenoku wrote:
> jdoerfert wrote:
> > We should investigate if it is sound to use assumed information in the exploration to catch this case.
> I feel it is difficult to use assumed information in this deduction scheme. 
> I'll investigate more but I think it would be really complex reasoning to maintain the soundness.
> 
> If we don't consider the cost, I think running Attributor twice will be bug free and effective solution because we can easily use assume information of 1st iteration as known information of 2nd iteration.
> If we don't consider the cost, I think running Attributor twice will be bug free and effective solution because we can easily use assume information of 1st iteration as known information of 2nd iteration.

I'm not sure I follow but re-running does generally not solve the problem. We need optimistic initialization to solve problems, e.g., the dereferenceable deduction in this test case.


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

https://reviews.llvm.org/D75923