[PATCH] D46326: ThinLTO+CFI: short-circuit direct calls to jump table entries

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich added a comment.

In https://reviews.llvm.org/D46326#1085464, @dmikulin wrote:

> In https://reviews.llvm.org/D46326#1084593, @vlad.tsyrklevich wrote:
>
> > Also adding a `FDecl->replaceDirectCalls(F)` in the else block after line 1016 seems like it would also correctly rewrite direct calls for the other two cases (external functions, and local functions declared in the current TU.)
>
>
> Aren't the other cases captured by F->replaceUsesExceptBlockAddr(FDecl) at line 1016?


No, that statement is doing the reverse of what you're doing. It's replacing directs calls to a function to calls to the jumptable entry. (F/FDecl have different meanings in the 2 other cases than how you're using them.)



================
Comment at: llvm/lib/IR/Value.cpp:506
+      continue;
+    U.set(New);
+  }
----------------
dmikulin wrote:
> efriedma wrote:
> > This looks like it doesn't handle the case of a function with a function pointer argument correctly.
> I thought getCalledFunction on a CallInst takes care of it... 
It checks that the call is a direct call, but the use might be an argument of the function not the called function, e.g.
```
void bar();
foo(&bar);
```

Iterating through the uses of `bar` you'll hit the call instruction and replace `foo(&bar)` with `foo(&bar.cfi)` though it shouldn't be. You need to check that getCalledFunction is the one you actually expect.


https://reviews.llvm.org/D46326