[PATCH] D69349: [InlineCost] Fix infinite loop in indirect call evaluation

[Ehud Katz via Phabricator via llvm-commits]

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

In D69349#1742057 <https://reviews.llvm.org/D69349#1742057>, @fedor.sergeev wrote:

> Can you add a test similar to what you have here that excercises indirect call through a parameter?
>  Say, do
>
>   define void @func1() {
>     %t = bitcast void ()* @func3 to void ()*
>     tail call void @func2(void()* %t)
>     ret void
>   }
>   define void @func2(void()* %f) {
>     tail call void %f()
>     ret void
>   }
>
>
> and then......
>
>   define void @func6() {
>     %t2 = bitcast void (void()*)* @func2 to void (void()*)* 
>     %t3 = bitcast void ()* @func3 to void ()*
>     tail call void %t2(void()* %t3)
>     ret void
>   }
>
>
> This results in recursive call to func2+ and your fix should handle it just right.


The change you suggest, doesn't cause the crash.



================
Comment at: llvm/lib/Analysis/InlineCost.cpp:1267
   }
 
+  // When we have a concrete function, first try to simplify it directly.
----------------
fedor.sergeev wrote:
> Please, add assert here on F, e.g.:
>   assert(F && "call to known function expected here")
Done


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:1318
+    // Pretend to inline the function, with a custom threshold.
+    if (IsIndirectCall & BoostIndirectCalls) {
+      auto IndirectCallParams = Params;
----------------
fedor.sergeev wrote:
> & -> && ?
Not a mistake, though the compiler will probably lower the later to just a simple "AND" (because they are both booleans), so I guess the later form is clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69349