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

[Dmitry Mikulin via Phabricator via llvm-commits]

dmikulin marked 3 inline comments as done.
dmikulin added a comment.



> 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.)

Sounds like we need to skip direct calls in replaceUsesExceptBlockAddr(). Make it replaceUsesExceptBlockAddrOrDirectCalls()



================
Comment at: llvm/lib/IR/Value.cpp:506
+      continue;
+    U.set(New);
+  }
----------------
vlad.tsyrklevich wrote:
> 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.
I see... I thought there'd be a more straightforward direct call check, but I can't seem to find anything in the code. Changed to check if the getCalledFunction() is the same as what we're trying to replace, i.e. ''this'.


https://reviews.llvm.org/D46326