[PATCH] D124787: [NVPTX] Implement NVPTX AliasAnalysis
Andrew Savonichev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 31 12:04:17 PST 2023
asavonic added inline comments.
================
Comment at: llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.cpp:72
+ const MemoryLocation &Loc2, AAQueryInfo &AAQI,
+ const Instruction *CtxI) {
+ unsigned AS1 = Loc1.Ptr->getType()->getPointerAddressSpace();
----------------
tra wrote:
> `CtxI` does not seem to be used. Do we need it?
It is a new parameter from D136512. We do not currently use it, so I guess it is better to drop it to avoid warnings.
================
Comment at: llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.cpp:90
+ bool IgnoreLocals) {
+ unsigned AS = Loc.Ptr->getType()->getPointerAddressSpace();
+ if (isConstOrParam(AS))
----------------
tra wrote:
> IMO, the code would be more readable w/o the variable. Just do `if (isConstOrParam(Loc.Ptr->getType()->getPointerAddressSpace()))` directly here and below.
>
> Having the variable makes me, as a reader, ask -- "what was the variable set to before?", "what else is this variable used for?" and that's just a distraction here.
Makes sense. Done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124787/new/
https://reviews.llvm.org/D124787
More information about the llvm-commits
mailing list