[PATCH] Removing unnecessary addrspacecasts from non-generic address spaces to the generic address space

[Eli Bendersky]

================
Comment at: lib/Target/NVPTX/NVPTXFavorNonGenericAddrSpaces.cpp:43
@@ +42,3 @@
+
+  // Optimizes load/store instructions. Idx is the index of the pointer operand
+  // (0 for load, and 1 for store). Returns true if it changes anything.
----------------
Use triple-/// comments for public documentation, http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments

================
Comment at: lib/Target/NVPTX/NVPTXFavorNonGenericAddrSpaces.cpp:15
@@ +14,3 @@
+// For now, it optimizes the following specific patterns:
+// 1. load (addrspacecast X) => load X
+// 2. store V, (addrspacecast X) => store V, X
----------------
I think this needs a more detailed top-level comment explaining when such optimizations are valid. In other words - what you mean by "unnecessary". You can give a couple of examples here.

================
Comment at: lib/Target/NVPTX/NVPTXFavorNonGenericAddrSpaces.cpp:76
@@ +75,3 @@
+  // generic address space.
+  return SrcTy->getAddressSpace() != 0 && DestTy->getAddressSpace() == 0;
+}
----------------
NVPTX.h has an AddressSpace enum, so you don't need the magic 0 constant

================
Comment at: lib/Target/NVPTX/NVPTXFavorNonGenericAddrSpaces.cpp:79
@@ +78,3 @@
+
+// Optimizes "gep (addrspacecast X), indices" into "addrspacecast (gep X,
+// indices)".  Moving addrspacecast out enables more optimizations on
----------------
"Optimizes" is confusing here, since the function only reorders stuff to expose optimization opportunities

================
Comment at: lib/Target/NVPTX/NVPTXFavorNonGenericAddrSpaces.cpp:94
@@ +93,3 @@
+//
+// Function optimizeMemoryInstruction will further translate the above snippet
+// into
----------------
This example is nice - you can probably place it in the file header instead of here, since it explains how multiple functions in the module interact

================
Comment at: test/CodeGen/NVPTX/access-non-generic.ll:1
@@ +1,2 @@
+; RUN: llc < %s -march=nvptx -mcpu=sm_20 | FileCheck %s
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_20 | FileCheck %s
----------------
Since this is a IR pass, I suggest adding some IR-level tests as well. These can help test the various input combinations to the pass in a more fine grained manner (looking exactly at the sequences of IR instructions the pass generates)


http://llvm-reviews.chandlerc.com/D3235