[PATCH] D126079: [NFC][LLVM] Rename all shouldExpandAtomic*InIR to shouldExpandAtomicInstInIR

[Shilei Tian via Phabricator via llvm-commits]

tianshilei1992 added a comment.

In D126079#3528232 <https://reviews.llvm.org/D126079#3528232>, @tra wrote:

> My original suggestion which resulted in this patch was to generalize the API to just one function accepting `Instruction*` and let the target deal with the details.
> For many targets there's a fair amount of duplicated code across the per-instruction variant implementation of `shouldExpandX` and using single call would allow to consolidate the atomic expansion logic in fewer places.
>
> Using overloads, indeed, only makes things more opaque without reducing the API surface.

I did have one patch locally that only keeps the function `shouldExpandAtomicInstInIR(Instruction *)` and in the body it has something like `if (auto *SI = dyn_cast<StoreInst>(I))`. In terms of readability, IMO it is same as overload. However, of course it can reduce number of APIs. I can use that method, but it looks like the controversy here is readability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126079