[PATCH] D99976: Allow invokable sub-classes of IntrinsicInst

[Eric Christopher via Phabricator via llvm-commits]

echristo added a comment.

In D99976#2706447 <https://reviews.llvm.org/D99976#2706447>, @nikic wrote:

> In D99976#2703736 <https://reviews.llvm.org/D99976#2703736>, @aeubanks wrote:
>
>> This slightly regressed compile times while it was in
>> http://llvm-compile-time-tracker.com/compare.php?from=080d48f279e2b70e4e9fc65bc4947a53ffc982f3&to=d87b9b81ccb95217181ce75515c6c68bbb408ca4&stat=instructions
>
> Interesting, I didn't expect that. Presumably this is because most places working with IntrinsicInst now have to perform two branches rather than one, and apparently it gets used a lot. Most likely this overestimates actual impact (assuming predicted branches), but still, better to avoid it if we can.

FWIW we also saw this in a build with predicted branches. :)

> Could it make sense to have both IntrinsicBase and IntrinsicInst, similar to how we have CallBase and CallInst?

Perhaps? I think some of the cases you've got like CallInst *findIntrinsicBlah(...) could return CallBase and that might make sense since an intrinsic "found" could be invokeable. Perhaps name it something neither Call or Invoke to encompass that it can be used for both? (Maybe Callable? I don't have any good ideas and the thesaurus is full of hilarious, but unlikely to be useful synonyms :)

Thoughts?


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

https://reviews.llvm.org/D99976