[PATCH] D77817: [llvm][NFC] Replace CallSite with CallBase in Inliner

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

In D77817#1972613 <https://reviews.llvm.org/D77817#1972613>, @dblaikie wrote:

> In D77817#1972599 <https://reviews.llvm.org/D77817#1972599>, @davidxl wrote:
>
> > What is the convention here? Pointer CallBase* vs CallBase& ?
>
>
> I'd say "Same as other llvm::Instructions" - and I /assume/ the answer there is that they're still predominantly handled as pointers, rather than references?
>
> Seems for /literally/ Instruction*/Instruction& (as opposed to all possible types of Instructions - harder to search/gather data for all of them) it's about 7:1 in favor of *, but that's hard to evaluate/compare - some of those may need to be pointers because null is an option.
>
> Evidently references to Instructions aren't totally unheard of, so the preference "use references when something can't be null" could be observed here. A pretty clear endorsement of this is that "instructions(F)" gives you an iterator over Instruction&, not over Instruction*.


This seems like a reasonable guideline, though - i.e. using references in function signatures where the contract is the parameter must be non-null. Looking at the refactored sites, there are a few spots where the caller is assumed to pass a valid CallSite. While that wasn't checked, now may be a good time to have that signal.

Example: setInlineRemark

> But yeah - given the current codebase I think it's dealer's choice & not something I'd push strongly either way in review personally. (but I don't do lots of Instruction handling, so maybe some other folks have stronger ideas about which direction this sort of thing should be going in)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77817