[PATCH] D39356: [ThinLTO] Fix missing call graph edges for calls with bitcasts.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Analysis/ModuleSummaryAnalysis.cpp:247
+      if (CalledValue) {
+        CalledValue = CalledValue->stripPointerCastsNoFollowAliases();
+        // Stripping pointer casts can reveal a called function.
----------------
Do we need to do this if CalledFunction is non-NULL? I would think the above condition could be "if (CalledValue and !CalledFunction)".


================
Comment at: llvm/lib/Analysis/ModuleSummaryAnalysis.cpp:251
+          CalledFunction = dyn_cast<Function>(CalledValue);
+      }
       // Check if this is an alias to a function. If so, get the
----------------
mehdi_amini wrote:
> vsapsai wrote:
> > mehdi_amini wrote:
> > > Isn't it enough to just replace ` auto *CalledValue = CS.getCalledValue();` with ` auto *CalledValue = CS.getCalledValue()->stripPointerCastsNoFollowAliases();` ?
> > Not enough. `CalledValue` will be what we want (and hopefully it won't be NULL) but `ImmutableCallSite CS` will still use callee with pointer cast and `CalledFunction` will be NULL.
> > 
> > Another approach can be adding `stripPointerCastsNoFollowAliases` to `ImmutableCallSite` or `CallSiteBase`. Or we can introduce another type of call site and add it there. I don't know if stripping casts will be useful or harmful in other places, for this I rely on people who are more familiar with this code.
> It is a good point that there is likely some general goodness in looking through pointer cast in `ImmutableCallSite` and/or `CallSiteBase`.
> I'd be interested to see if `ninja check-all` is passing after adding `stripPointerCastsNoFollowAliases` to `CallSiteBase`, and if not what the failures are.
Agree, this would be a good thing to try.


https://reviews.llvm.org/D39356