[PATCH] D124787: [NVPTX] Implement NVPTX AliasAnalysis

[Andrew Savonichev via Phabricator via llvm-commits]

asavonic added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.cpp:64
+  if (Result == AliasResult::NoAlias)
+    return Result;
+
----------------
tra wrote:
> Nit. I'd rephrase it a bit and put `alias()` call closer to the condition check. 
> ```
>   if (getAliasResult(AS1, AS2) != AliasResult::NoAlias) 
>       return AAResultBase::alias(Loc1, Loc2, AAQI);
>   return  AliasResult::NoAlias;
> ```
> Maybe, even fold it into a single `return (...== NoAlias) ? NoAlias : alias();`
Done.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.cpp:71
+                                           AAQueryInfo &AAQI, bool OrLocal) {
+  unsigned AS = Loc.Ptr->getType()->getPointerAddressSpace();
+  if (AS == AddressSpace::ADDRESS_SPACE_CONST)
----------------
tra wrote:
> Kernel parameters are also read-only: https://docs.nvidia.com/cuda/parallel-thread-execution/index.html#state-spaces
> 
Thanks! Added param space to the check and updated LIT tests.


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

https://reviews.llvm.org/D124787