[PATCH] D29164: NVPTX: Refactor NVPTXInferAddressSpaces to check TTI

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In https://reviews.llvm.org/D29164#658488, @jlebar wrote:

> I like the new name and comment a lot.  Do you think we should rename "GenericAddrSpace"?
>
> I'm still not sold on returning -1 rather than using an optional<unsigned>, though.  The meaning of
>
>   GenericAddrSpace = TTI.getFlatAddressSpace();
>   if (GenericAddrSpace == ADDRESS_SPACE_UNINITIALIZED)
>   
>
> is highly nonobvious, relying on the fact that ADDRESS_SPACE_UNINITIALIZED is the same as the default return value of getFlatAddressSpace().  Whereas if we used optional, it would become:
>
>   optional<unsigned> MaybeFlatAS = TTI.getFlatAddressSpace();
>   if (!MaybeFlatAS) return;
>   FlatAS = *MaybeFlatAS;
>   
>
> which I think pretty directly conveys what we're getting at?


I fixed the name of ADDRESS_SPACE_UNINITIALIZED to be UnknownAddressSpace in the cleanup commit which I think is more clear. I'd rather not deviate from the API/convention the other address space TTI functions already use (e.g. LSR defines the same constant and passes it to isLegalAddressingMode sometimes)



================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:236
+  /// address space.
+  unsigned getUndesirableAddressSpace() const;
+
----------------
arsenm wrote:
> jlebar wrote:
> > jlebar wrote:
> > > Maybe I'm biased because I'm coming from the CUDA world, but "undesirable" doesn't really seem to capture what's going on as well as 'generic'., and indeed the comment uses "generic" to good effect.
> > > 
> > > What are you trying to get across with the name, exactly?
> > Maybe this should return optional<unsigned>?
> I was trying to avoid a second meaning of the term generic address space. LLVM already unfortunately clobbers the meaning of "generic" address space to mean addrspace(0) and all the assumptions it makes about it. This is generic/flat in a GPU world sense
Seems like overkill, and other places already use the -1 convention. Address spaces are only allowed to be 24-bits, so we don't need to spare every bit.


https://reviews.llvm.org/D29164