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

Justin Lebar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 26 22:51:46 PST 2017


jlebar added a comment.

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?


https://reviews.llvm.org/D29164





More information about the llvm-commits mailing list