[PATCH] D124787: [NVPTX] Implement NVPTX AliasAnalysis
Artem Belevich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 31 10:51:30 PST 2023
tra accepted this revision.
tra added a comment.
Still LGTM with a couple of nits.
================
Comment at: llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.cpp:72
+ const MemoryLocation &Loc2, AAQueryInfo &AAQI,
+ const Instruction *CtxI) {
+ unsigned AS1 = Loc1.Ptr->getType()->getPointerAddressSpace();
----------------
`CtxI` does not seem to be used. Do we need it?
================
Comment at: llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.cpp:90
+ bool IgnoreLocals) {
+ unsigned AS = Loc.Ptr->getType()->getPointerAddressSpace();
+ if (isConstOrParam(AS))
----------------
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.
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