NVPTX: Trivial cleanups of NVPTXInferAddressSpaces

[arsenm]

Summary

• Move DEBUG_TYPE below includes
• Change unknown address space constant to be consistent with other passes
  • Grammar fixes in debug output

[jlebar]

lib/Target/NVPTX/NVPTXInferAddressSpaces.cpp

472

I don't really care, but I have to admit that I find '\n' in place of "\n" kind of a strange change to make.

[arsenm]

lib/Target/NVPTX/NVPTXInferAddressSpaces.cpp

472

I think it looks nicer and allegedly this saves a call by using the more inlinable char version of <<

[jlebar]

lib/Target/NVPTX/NVPTXInferAddressSpaces.cpp

472

I mean, presumably we are not optimizing for the speed of these dbgs() calls.

If my grep'ing is correct, *nobody* is doing '\n', so maybe we shouldn't make this change.

$ git grep 'dbgs().*"\\n"' | wc -l
1009
$ git grep "dbgs().*'\\n'" | wc -l
0

[arsenm]

lib/Target/NVPTX/NVPTXInferAddressSpaces.cpp

472

I've been consistently using single quote chars in AMDGPU so something is probably off

[arsenm]

r293567

lib/Target/NVPTX/NVPTXInferAddressSpaces.cpp

 //   %y' = phi float addrspace(3)* [ %input, undef ]
 // Then, it converts %y2 to
 //   %y2' = getelementptr %y', 1
 // Finally, it fixes the undef in %y' so that
 //   %y' = phi float addrspace(3)* [ %input, %y2' ]
 //
 //===----------------------------------------------------------------------===//
 
-#define DEBUG_TYPE "nvptx-infer-addrspace"
-
 #include "NVPTX.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
 
+#define DEBUG_TYPE "nvptx-infer-addrspace"
+
 using namespace llvm;
 
 namespace {
-const unsigned ADDRESS_SPACE_UNINITIALIZED = (unsigned)-1;
+static const unsigned UnknownAddressSpace = ~0u;
 
 using ValueToAddrSpaceMapTy = DenseMap<const Value *, unsigned>;
 
 /// \brief NVPTXInferAddressSpaces
 class NVPTXInferAddressSpaces: public FunctionPass {
   /// Target specific address space which uses of should be replaced if
   /// possible.
   unsigned GenericAddrSpace;

 
 // Defines the join operation on the address space lattice (see the file header
 // comments).
 unsigned NVPTXInferAddressSpaces::joinAddressSpaces(unsigned AS1,
                                                     unsigned AS2) const {
   if (AS1 == GenericAddrSpace || AS2 == GenericAddrSpace)
     return GenericAddrSpace;
 
-  if (AS1 == ADDRESS_SPACE_UNINITIALIZED)
+  if (AS1 == UnknownAddressSpace)
     return AS2;
-  if (AS2 == ADDRESS_SPACE_UNINITIALIZED)
+  if (AS2 == UnknownAddressSpace)
     return AS1;
 
   // The join of two different specific address spaces is generic.
   return (AS1 == AS2) ? AS1 : GenericAddrSpace;
 }
 
 bool NVPTXInferAddressSpaces::runOnFunction(Function &F) {
   if (skipFunction(F))
     return false;
 
   const TargetTransformInfo &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
   GenericAddrSpace = TTI.getUndesirableAddressSpace();
-  if (GenericAddrSpace == ADDRESS_SPACE_UNINITIALIZED)
+  if (GenericAddrSpace == UnknownAddressSpace)
     return false;
 
   // Collects all generic address expressions in postorder.
   std::vector<Value *> Postorder = collectGenericAddressExpressions(F);
 
   // Runs a data-flow analysis to refine the address spaces of every expression
   // in Postorder.
   ValueToAddrSpaceMapTy InferredAddrSpace;
   inferAddressSpaces(Postorder, &InferredAddrSpace);
 
   // Changes the address spaces of the generic address expressions who are
   // inferred to point to a specific address space.
   return rewriteWithNewAddressSpaces(Postorder, InferredAddrSpace, &F);
 }
 
 void NVPTXInferAddressSpaces::inferAddressSpaces(
     const std::vector<Value *> &Postorder,
     ValueToAddrSpaceMapTy *InferredAddrSpace) {
   SetVector<Value *> Worklist(Postorder.begin(), Postorder.end());
   // Initially, all expressions are in the uninitialized address space.
   for (Value *V : Postorder)
-    (*InferredAddrSpace)[V] = ADDRESS_SPACE_UNINITIALIZED;
+    (*InferredAddrSpace)[V] = UnknownAddressSpace;
 
   while (!Worklist.empty()) {
     Value* V = Worklist.pop_back_val();
 
     // Tries to update the address space of the stack top according to the
     // address spaces of its operands.
-    DEBUG(dbgs() << "Updating the address space of\n"
-                 << "  " << *V << "\n");
+    DEBUG(dbgs() << "Updating the address space of\n  " << *V << '\n');
     Optional<unsigned> NewAS = updateAddressSpace(*V, *InferredAddrSpace);
     if (!NewAS.hasValue())
       continue;
     // If any updates are made, grabs its users to the worklist because
     // their address spaces can also be possibly updated.
-    DEBUG(dbgs() << "  to " << NewAS.getValue() << "\n");
+    DEBUG(dbgs() << "  to " << NewAS.getValue() << '\n');
     (*InferredAddrSpace)[V] = NewAS.getValue();
 
     for (Value *User : V->users()) {
       // Skip if User is already in the worklist.
       if (Worklist.count(User))
         continue;
 
       auto Pos = InferredAddrSpace->find(User);

 }
 
 Optional<unsigned> NVPTXInferAddressSpaces::updateAddressSpace(
     const Value &V, const ValueToAddrSpaceMapTy &InferredAddrSpace) {
   assert(InferredAddrSpace.count(&V));
 
   // The new inferred address space equals the join of the address spaces
   // of all its pointer operands.
-  unsigned NewAS = ADDRESS_SPACE_UNINITIALIZED;
+  unsigned NewAS = UnknownAddressSpace;
   for (Value *PtrOperand : getPointerOperands(V)) {
     unsigned OperandAS;
     if (InferredAddrSpace.count(PtrOperand))
       OperandAS = InferredAddrSpace.lookup(PtrOperand);
     else
       OperandAS = PtrOperand->getType()->getPointerAddressSpace();
     NewAS = joinAddressSpaces(NewAS, OperandAS);
     // join(generic, *) = generic. So we can break if NewAS is already generic.
bool NVPTXInferAddressSpaces::rewriteWithNewAddressSpaces(
   for (Value *V : Postorder) {
     Value *NewV = ValueWithNewAddrSpace.lookup(V);
     if (NewV == nullptr)
       continue;
 
     SmallVector<Use *, 4> Uses;
     for (Use &U : V->uses())
       Uses.push_back(&U);
-    DEBUG(dbgs() << "Replacing the uses of " << *V << "\n  to\n  " << *NewV
-                 << "\n");
+
+    DEBUG(dbgs() << "Replacing the uses of " << *V
+                 << "\n  with\n  " << *NewV << '\n');
+
     for (Use *U : Uses) {
       if (isa<LoadInst>(U->getUser()) ||
           (isa<StoreInst>(U->getUser()) && U->getOperandNo() == 1)) {
         // If V is used as the pointer operand of a load/store, sets the pointer
         // operand to NewV. This replacement does not change the element type,
         // so the resultant load/store is still valid.
         U->set(NewV);
       } else if (isa<Instruction>(U->getUser())) {