[PATCH] D49337: Skip debuginfo intrinsic in markLiveBlocks.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D49337#1162719, @trentxintong wrote:

> Hi @hfinkel.
>
> Thank you for taking a look at this code. I agree we should definitely "else" the StoreInst check and I have observed improvements in Xcode Instrument for doing so.
>
> And I should also move the DbgInfoIntrinsic check inside the IntrinsicInst check by comparing IntrinsicID.
>
> The problem with moving the IntrinsicInst checks inside the dyn_cast<CallInst> is that every intrinsic call will need to go through a dyn_cast<CallInst> check, a getCalledValue() and then a dyn_cast<Function>(CalledValue) check.  If we have a lot of intrinsic which we do when compiling with debuginfo, we could end up being overall slower than keeping the IntrinsicInst check.
>
> When we compile without debuginfo, moving the intrinsic check inside the CallInst check will not necessarily bring us performance, as now every CallInst needs to checks for the existing cases in the if (dyn_cast<CallInst>)) and the Intrinsics (which require dyn_cast<Function>(CalledValue)).  4


I'm not sure that I understand what you're saying. "is that every intrinsic call will need to go through a dyn_cast<CallInst> check, a getCalledValue() and then a dyn_cast<Function>(CalledValue) check." - this is exactly what the IntrinsicInst dyn_cast does internally. When we cast to IntrinsicInst we call the following in IntrinsicInst:

  static bool classof(const CallInst *I) {
    if (const Function *CF = I->getCalledFunction())
      return CF->isIntrinsic();
    return false;
  }
  static bool classof(const Value *V) {
    return isa<CallInst>(V) && classof(cast<CallInst>(V));
  }

so we pay for the isa<CallInst>, which should be the same cost as the dyn_cast<CallInst>, and then call I->getCalledFunction(), which is implemented as a dyn_cast<Function> of getCalledValue(). When we do isa<DbgInfoIntrinsic>, we do all of that, and then also call getIntrinsicID and compare to one of the IDs for the debug info intrinsics. Thus, to be clear, what I'm saying is that if we're worried about duplicated work here, we shouldn't be using IntrinsicInst at all.

> Right now I am seeing the version which we keep the IntrinsicInst check and adding the DbgInfoIntrinsic inside it to be the fastest on sqlite3.c with debuginfo.




Repository:
  rL LLVM

https://reviews.llvm.org/D49337