[PATCH] D17965: [NVPTX] Adds a new address space inference pass.

[Jingyue Wu via llvm-commits]

jingyue added a comment.

All comments addressed. Submitting...


================
Comment at: lib/Target/NVPTX/NVPTXInferAddressSpaces.cpp:413
@@ +412,3 @@
+namespace llvm {
+void initializeNVPTXInferAddressSpacesPass(PassRegistry &);
+}
----------------
jlebar wrote:
> jingyue wrote:
> > jlebar wrote:
> > > Is there some reason not to define this one-liner here?
> > I don't get what you mean. What's the one-liner? 
> I just mean  moving the definition of this function into this declaration.  Not a big deal either way.
The one-liner definition below expands to `llvm::initializeNVPTXInferAddressSpacesPass`, so it has to be out of `namespace llvm`. 

================
Comment at: test/CodeGen/NVPTX/access-non-generic.ll:6
@@ -4,1 +5,3 @@
+; RUN: opt < %s -S -nvptx-infer-addrspace | FileCheck %s --check-prefix IR
+; RUN: opt < %s -S -nvptx-infer-addrspace | FileCheck %s --check-prefix IR-WITH-LOOP
 
----------------
jingyue wrote:
> jlebar wrote:
> > Are these incompatible -- i.e., we can't combine these two into one FileCheck call with two use-prefixes?
> IR-WITH-LOOP only works with `nvptx-infer-addrspace` and not `nvptx-favor-non-generic`, so I split the tests into
> 
> IR: check both favor and infer
> IR-WITH-LOOP: check only favor
I didn't know I could chain multiple `--check-prefix` together. Thanks for pointing this out. 


http://reviews.llvm.org/D17965