InferAddressSpaces: Handle a few intrinsics

[jlebar]

lib/Transforms/Scalar/InferAddressSpaces.cpp

235

Nit, idiom is to wrap = on the first line, I think. Also idiom is not to have spaces around the curly braces, I believe. (I care because I just want to clang-format my code, and so if this adheres to a different format, then when I go and edit this, I'm going to have spurious formatting changes, which ends up costing reviewer time. Same applies if I'm not the author but the reviewer.)

249

Really prefer clang-format's formatting of this function header:

void InferAddressSpaces::collectRewritableIntrinsicOperands(
    IntrinsicInst *II, std::vector<std::pair<Value *, bool>> *PostorderStack,
    DenseSet<Value *> *Visited) const {
  switch (II->getIntrinsicID()) {
  case Intrinsic::objectsize:

By indenting four spaces rather than two, it becomes unambiguous which lines are part of the function header vs the body. As-is I had to read it multiple times to parse it correctly.

If you're amenable, we could save ourselves a lot of energy by simply agreeing to use clang-format (and to file bugs when it's ugly -- djasper is pretty responsive).

For myself, I've wrapped arc diff so it runs git-clang-format over all the lines I touch, so there's no additional cognitive load: https://github.com/jlebar/conf/blob/master/bin/arc. The git-clang-format script is in clang/tools/clang-format/git-clang-format.

621

Nit, suggest

bool InferAddressSpaces::handleComplexPtrUse(User &U, Value *OldV,
                                             Value *NewV) const {

This is more aesthetically pleasing (and so, marginally easier to read) because the second line isn't a lot longer than the first. (And yes, this is clang-format's output. :)

[arsenm]

Looks like I accidentally merged this with the other memintrinsic patch when I committed it. I pushed a format fix patch in r293843 to fix these

lib/Transforms/Scalar/InferAddressSpaces.cpp

private:
   updateAddressSpace(const Value &V,
                      const ValueToAddrSpaceMapTy &InferredAddrSpace) const;
 
   // Tries to infer the specific address space of each address expression in
   // Postorder.
   void inferAddressSpaces(const std::vector<Value *> &Postorder,
                           ValueToAddrSpaceMapTy *InferredAddrSpace) const;
 
+  bool handleComplexPtrUse(User &U, Value *OldV, Value *NewV) const;
+
   // Changes the generic address expressions in function F to point to specific
   // address spaces if InferredAddrSpace says so. Postorder is the postorder of
   // all generic address expressions in the use-def graph of function F.
   bool
   rewriteWithNewAddressSpaces(const std::vector<Value *> &Postorder,
                               const ValueToAddrSpaceMapTy &InferredAddrSpace,
                               Function *F) const;
 
   void appendsGenericAddressExpressionToPostorderStack(
     Value *V, std::vector<std::pair<Value *, bool>> *PostorderStack,
     DenseSet<Value *> *Visited) const;
 
+  bool rewriteIntrinsicOperands(IntrinsicInst *II,
+                                Value *OldV, Value *NewV) const;
+  void collectRewritableIntrinsicOperands(
+    IntrinsicInst *II,
+    std::vector<std::pair<Value *, bool>> *PostorderStack,
+    DenseSet<Value *> *Visited) const;
+
   std::vector<Value *> collectGenericAddressExpressions(Function &F) const;
   Value *cloneValueWithNewAddressSpace(
     Value *V, unsigned NewAddrSpace,
     const ValueToValueMapTy &ValueWithNewAddrSpace,
     SmallVectorImpl<const Use *> *UndefUsesToFix) const;
   unsigned joinAddressSpaces(unsigned AS1, unsigned AS2) const;
 };
 } // end anonymous namespace
static SmallVector<Value *, 2> getPointerOperands(const Value &V) {
   case Instruction::AddrSpaceCast:
   case Instruction::GetElementPtr:
     return {Op.getOperand(0)};
   default:
     llvm_unreachable("Unexpected instruction type.");
   }
 }
 
+// TODO: Move logic to TTI?
+bool InferAddressSpaces::rewriteIntrinsicOperands(IntrinsicInst *II,
+                                                  Value *OldV,
+                                                  Value *NewV) const {
+  Module *M = II->getParent()->getParent()->getParent();
+
+  switch (II->getIntrinsicID()) {
+  case Intrinsic::objectsize:
+  case Intrinsic::amdgcn_atomic_inc:
+  case Intrinsic::amdgcn_atomic_dec: {
+    Type *DestTy = II->getType();
+    Type *SrcTy = NewV->getType();
+    Function *NewDecl
+      = Intrinsic::getDeclaration(M, II->getIntrinsicID(), { DestTy, SrcTy });
+    II->setArgOperand(0, NewV);
+    II->setCalledFunction(NewDecl);
+    return true;
+  }
+  default:
+    return false;
+  }
+}
+
+// TODO: Move logic to TTI?
+void InferAddressSpaces::collectRewritableIntrinsicOperands(
+  IntrinsicInst *II,
+  std::vector<std::pair<Value *, bool>> *PostorderStack,
+  DenseSet<Value *> *Visited) const {
+  switch (II->getIntrinsicID()) {
+  case Intrinsic::objectsize:
+  case Intrinsic::amdgcn_atomic_inc:
+  case Intrinsic::amdgcn_atomic_dec:
+    appendsGenericAddressExpressionToPostorderStack(
+      II->getArgOperand(0), PostorderStack, Visited);
+    break;
+  default:
+    break;
+  }
+}
+
+// Returns all generic address expressions in function F. The elements are
 // If V is an unvisited generic address expression, appends V to PostorderStack
 // and marks it as visited.
 void InferAddressSpaces::appendsGenericAddressExpressionToPostorderStack(
   Value *V, std::vector<std::pair<Value *, bool>> *PostorderStack,
   DenseSet<Value *> *Visited) const {
   assert(V->getType()->isPointerTy());
   if (isAddressExpression(*V) &&
       V->getType()->getPointerAddressSpace() == GenericAddrSpace) {
     if (Visited->insert(V).second)
       PostorderStack->push_back(std::make_pair(V, false));
   }
 }
 
-// Returns all generic address expressions in function F. The elements are
 // ordered in postorder.
 std::vector<Value *>
 InferAddressSpaces::collectGenericAddressExpressions(Function &F) const {
   // This function implements a non-recursive postorder traversal of a partial
   // use-def graph of function F.
   std::vector<std::pair<Value*, bool>> PostorderStack;
   // The set of visited expressions.
   DenseSet<Value*> Visited;
if (LoadInst *LI = dyn_cast<LoadInst>(&I)) {
       appendsGenericAddressExpressionToPostorderStack(
         MI->getRawDest(), &PostorderStack, &Visited);
 
       // Handle 2nd operand for memcpy/memmove.
       if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(MI)) {
         appendsGenericAddressExpressionToPostorderStack(
           MTI->getRawSource(), &PostorderStack, &Visited);
       }
+    } else if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(&I)) {
+      collectRewritableIntrinsicOperands(II, &PostorderStack, &Visited);
     }
-
-    // TODO: Support target intrinsics
   }
 
   std::vector<Value *> Postorder; // The resultant postorder.
   while (!PostorderStack.empty()) {
     // If the operands of the expression on the top are already explored,
     // adds that expression to the resultant postorder.
     if (PostorderStack.back().second) {
       Postorder.push_back(PostorderStack.back().first);
static bool isSimpleCompatiblePtrUse(Use &U) {
   }
 
   return false;
 }
 
 /// Handle updating uses that require more complex processing than simply
 /// replacing an operand. e.g. intrinsic uses that need to be re-mangled.
 /// \returns true on sucess but does not remove the user instruction \p U.
-static bool handleComplexPtrUse(User &U, Value *OldV, Value *NewV) {
+bool InferAddressSpaces::handleComplexPtrUse(User &U,
+                                             Value *OldV, Value *NewV) const {
   if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(&U)) {
     if (MI->isVolatile())
       return false;
 
     CallInst *NewCall;
     IRBuilder<> B(MI);
     MDNode *TBAA = MI->getMetadata(LLVMContext::MD_tbaa);
     MDNode *ScopeMD = MI->getMetadata(LLVMContext::MD_alias_scope);
if (MemSetInst *MSI = dyn_cast<MemSetInst>(MI)) {
       }
     } else
       llvm_unreachable("unhandled MemIntrinsic");
 
     MI->eraseFromParent();
     return true;
   }
 
+  if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(&U))
+    return rewriteIntrinsicOperands(II, OldV, NewV);
+
   return false;
 }
 
 bool InferAddressSpaces::rewriteWithNewAddressSpaces(
   const std::vector<Value *> &Postorder,
   const ValueToAddrSpaceMapTy &InferredAddrSpace, Function *F) const {
   // For each address expression to be modified, creates a clone of it with its
   // pointer operands converted to the new address space. Since the pointer

test/Transforms/InferAddressSpaces/AMDGPU/intrinsics.ll

+; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -infer-address-spaces %s | FileCheck %s
+
+; CHECK-LABEL: @objectsize_group_to_flat_i32(
+; CHECK: %val = call i32 @llvm.objectsize.i32.p3i8(i8 addrspace(3)* %group.ptr, i1 true)
+define i32 @objectsize_group_to_flat_i32(i8 addrspace(3)* %group.ptr) #0 {
+  %cast = addrspacecast i8 addrspace(3)* %group.ptr to i8 addrspace(4)*
+  %val = call i32 @llvm.objectsize.i32.p4i8(i8 addrspace(4)* %cast, i1 true)
+  ret i32 %val
+}
+
+; CHECK-LABEL: @objectsize_global_to_flat_i64(
+; CHECK: %val = call i64 @llvm.objectsize.i64.p3i8(i8 addrspace(3)* %global.ptr, i1 true)
+define i64 @objectsize_global_to_flat_i64(i8 addrspace(3)* %global.ptr) #0 {
+  %cast = addrspacecast i8 addrspace(3)* %global.ptr to i8 addrspace(4)*
+  %val = call i64 @llvm.objectsize.i64.p4i8(i8 addrspace(4)* %cast, i1 true)
+  ret i64 %val
+}
+
+; CHECK-LABEL: @atomicinc_global_to_flat_i32(
+; CHECK: call i32 @llvm.amdgcn.atomic.inc.i32.p1i32(i32 addrspace(1)* %global.ptr, i32 %y)
+define i32 @atomicinc_global_to_flat_i32(i32 addrspace(1)* %global.ptr, i32 %y) #0 {
+  %cast = addrspacecast i32 addrspace(1)* %global.ptr to i32 addrspace(4)*
+  %ret = call i32 @llvm.amdgcn.atomic.inc.i32.p4i32(i32 addrspace(4)* %cast, i32 %y)
+  ret i32 %ret
+}
+
+; CHECK-LABEL: @atomicinc_group_to_flat_i32(
+; CHECK: %ret = call i32 @llvm.amdgcn.atomic.inc.i32.p3i32(i32 addrspace(3)* %group.ptr, i32 %y)
+define i32 @atomicinc_group_to_flat_i32(i32 addrspace(3)* %group.ptr, i32 %y) #0 {
+  %cast = addrspacecast i32 addrspace(3)* %group.ptr to i32 addrspace(4)*
+  %ret = call i32 @llvm.amdgcn.atomic.inc.i32.p4i32(i32 addrspace(4)* %cast, i32 %y)
+  ret i32 %ret
+}
+
+; CHECK-LABEL: @atomicinc_global_to_flat_i64(
+; CHECK: call i64 @llvm.amdgcn.atomic.inc.i64.p1i64(i64 addrspace(1)* %global.ptr, i64 %y)
+define i64 @atomicinc_global_to_flat_i64(i64 addrspace(1)* %global.ptr, i64 %y) #0 {
+  %cast = addrspacecast i64 addrspace(1)* %global.ptr to i64 addrspace(4)*
+  %ret = call i64 @llvm.amdgcn.atomic.inc.i64.p4i64(i64 addrspace(4)* %cast, i64 %y)
+  ret i64 %ret
+}
+
+; CHECK-LABEL: @atomicinc_group_to_flat_i64(
+; CHECK: call i64 @llvm.amdgcn.atomic.inc.i64.p3i64(i64 addrspace(3)* %group.ptr, i64 %y)
+define i64 @atomicinc_group_to_flat_i64(i64 addrspace(3)* %group.ptr, i64 %y) #0 {
+  %cast = addrspacecast i64 addrspace(3)* %group.ptr to i64 addrspace(4)*
+  %ret = call i64 @llvm.amdgcn.atomic.inc.i64.p4i64(i64 addrspace(4)* %cast, i64 %y)
+  ret i64 %ret
+}
+
+; CHECK-LABEL: @atomicdec_global_to_flat_i32(
+; CHECK: call i32 @llvm.amdgcn.atomic.dec.i32.p1i32(i32 addrspace(1)* %global.ptr, i32 %val)
+define i32 @atomicdec_global_to_flat_i32(i32 addrspace(1)* %global.ptr, i32 %val) #0 {
+  %cast = addrspacecast i32 addrspace(1)* %global.ptr to i32 addrspace(4)*
+  %ret = call i32 @llvm.amdgcn.atomic.dec.i32.p4i32(i32 addrspace(4)* %cast, i32 %val)
+  ret i32 %ret
+}
+
+; CHECK-LABEL: @atomicdec_group_to_flat_i32(
+; CHECK: %ret = call i32 @llvm.amdgcn.atomic.dec.i32.p3i32(i32 addrspace(3)* %group.ptr, i32 %val)
+define i32 @atomicdec_group_to_flat_i32(i32 addrspace(3)* %group.ptr, i32 %val) #0 {
+  %cast = addrspacecast i32 addrspace(3)* %group.ptr to i32 addrspace(4)*
+  %ret = call i32 @llvm.amdgcn.atomic.dec.i32.p4i32(i32 addrspace(4)* %cast, i32 %val)
+  ret i32 %ret
+}
+
+; CHECK-LABEL: @atomicdec_global_to_flat_i64(
+; CHECK: call i64 @llvm.amdgcn.atomic.dec.i64.p1i64(i64 addrspace(1)* %global.ptr, i64 %y)
+define i64 @atomicdec_global_to_flat_i64(i64 addrspace(1)* %global.ptr, i64 %y) #0 {
+  %cast = addrspacecast i64 addrspace(1)* %global.ptr to i64 addrspace(4)*
+  %ret = call i64 @llvm.amdgcn.atomic.dec.i64.p4i64(i64 addrspace(4)* %cast, i64 %y)
+  ret i64 %ret
+}
+
+; CHECK-LABEL: @atomicdec_group_to_flat_i64(
+; CHECK: call i64 @llvm.amdgcn.atomic.dec.i64.p3i64(i64 addrspace(3)* %group.ptr, i64 %y)
+define i64 @atomicdec_group_to_flat_i64(i64 addrspace(3)* %group.ptr, i64 %y) #0 {
+  %cast = addrspacecast i64 addrspace(3)* %group.ptr to i64 addrspace(4)*
+  %ret = call i64 @llvm.amdgcn.atomic.dec.i64.p4i64(i64 addrspace(4)* %cast, i64 %y)
+  ret i64 %ret
+}
+
+declare i32 @llvm.objectsize.i32.p4i8(i8 addrspace(4)*, i1) #1
+declare i64 @llvm.objectsize.i64.p4i8(i8 addrspace(4)*, i1) #1
+declare i32 @llvm.amdgcn.atomic.inc.i32.p4i32(i32 addrspace(4)* nocapture, i32) #2
+declare i64 @llvm.amdgcn.atomic.inc.i64.p4i64(i64 addrspace(4)* nocapture, i64) #2
+declare i32 @llvm.amdgcn.atomic.dec.i32.p4i32(i32 addrspace(4)* nocapture, i32) #2
+declare i64 @llvm.amdgcn.atomic.dec.i64.p4i64(i64 addrspace(4)* nocapture, i64) #2
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind readnone }
+attributes #2 = { nounwind argmemonly }