AMDGPU: Whitelist handled intrinsics

[arsenm]

Summary

We shouldn't crash on unhandled intrinsics. Also simplify failure handling in loop.

[• tstellarAMD]

LGTM.

lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp

546–551

I'm not sure why this is here. If it doesn't make sense, feel free to remove it.

[arsenm]

r259546

lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp

546–551

I think we should replace it with a return false and an assert so it comes up again when calls are supported

lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp

for (std::vector<Value*>::iterator I = WorkList.begin(),
     default:
       Inst->dump();
       llvm_unreachable("Inconsistency in instructions promotable to vector");
     }
   }
   return true;
 }
 
+static bool isCallPromotable(CallInst *CI) {
+  // TODO: We might be able to handle some cases where the callee is a
+  // constantexpr bitcast of a function.
+  if (!CI->getCalledFunction())
+    return false;
+
+  IntrinsicInst *II = dyn_cast<IntrinsicInst>(CI);
+  if (!II)
+    return false;
+
+  switch (II->getIntrinsicID()) {
+  case Intrinsic::memcpy:
+  case Intrinsic::memset:
+  case Intrinsic::lifetime_start:
+  case Intrinsic::lifetime_end:
+  case Intrinsic::invariant_start:
+  case Intrinsic::invariant_end:
+  case Intrinsic::invariant_group_barrier:
+    return true;
+  default:
+    return false;
+  }
+}
+
 static bool collectUsesWithPtrTypes(Value *Val, std::vector<Value*> &WorkList) {
-  bool Success = true;
   for (User *User : Val->users()) {
-    if(std::find(WorkList.begin(), WorkList.end(), User) != WorkList.end())
+    if (std::find(WorkList.begin(), WorkList.end(), User) != WorkList.end())
       continue;
+
     if (CallInst *CI = dyn_cast<CallInst>(User)) {
-      // TODO: We might be able to handle some cases where the callee is a
-      // constantexpr bitcast of a function.
-      if (!CI->getCalledFunction())
+      if (!isCallPromotable(CI))
         return false;
 
       WorkList.push_back(User);
       continue;
     }
 
     // FIXME: Correctly handle ptrtoint instructions.
     Instruction *UseInst = dyn_cast<Instruction>(User);
     if (UseInst && UseInst->getOpcode() == Instruction::PtrToInt)
       return false;
 
     if (StoreInst *SI = dyn_cast_or_null<StoreInst>(UseInst)) {
       // Reject if the stored value is not the pointer operand.
       if (SI->getPointerOperand() != Val)
         return false;
     }
 
     if (!User->getType()->isPointerTy())
       continue;
 
     WorkList.push_back(User);
-
-    Success &= collectUsesWithPtrTypes(User, WorkList);
+    if (!collectUsesWithPtrTypes(User, WorkList))
+      return false;
   }
-  return Success;
+
+  return true;
 }
 
 void AMDGPUPromoteAlloca::visitAlloca(AllocaInst &I) {
   if (!I.isStaticAlloca())
     return;
 
   IRBuilder<> Builder(&I);
 
if (!Call) {
 
       // FIXME: It doesn't really make sense to try to do this for all
       // instructions.
       V->mutateType(NewTy);
       continue;
     }
 
     IntrinsicInst *Intr = dyn_cast<IntrinsicInst>(Call);
     if (!Intr) {
+      // FIXME: What is this for? It doesn't make sense to promote arbitrary
+      // function calls. If the call is to a defined function that can also be
+      // promoted, we should be able to do this once that function is also
+      // rewritten.
+
       std::vector<Type*> ArgTypes;
       for (unsigned ArgIdx = 0, ArgEnd = Call->getNumArgOperands();
                                 ArgIdx != ArgEnd; ++ArgIdx) {
         ArgTypes.push_back(Call->getArgOperand(ArgIdx)->getType());
       }
       Function *F = Call->getCalledFunction();
       FunctionType *NewType = FunctionType::get(Call->getType(), ArgTypes,
                                                 F->isVarArg());

test/CodeGen/AMDGPU/promote-alloca-unhandled-intrinsic.ll

+; RUN: opt -S -mtriple=amdgcn-unknown-amdhsa < %s | FileCheck %s
+
+; This is just an arbitrary intrinisic that shouldn't ever need to be
+; handled to ensure it doesn't crash.
+
+declare i32 @llvm.setjmp(i8*) #2
+declare void @eh.sjlj.functioncontext(i8*) #2
+
+; CHECK-LABEL: @try_promote_unhandled_intrinsic(
+; CHECK: alloca
+; CHECK: call void @eh.sjlj.functioncontext(i8* %tmp1)
+define void @try_promote_unhandled_intrinsic(i32 addrspace(1)* %arg) #2 {
+bb:
+  %tmp = alloca i32, align 4
+  %tmp1 = bitcast i32* %tmp to i8*
+  %tmp2 = getelementptr inbounds i32, i32 addrspace(1)* %arg, i64 1
+  %tmp3 = load i32, i32 addrspace(1)* %tmp2
+  store i32 %tmp3, i32* %tmp
+  call void @eh.sjlj.functioncontext(i8* %tmp1)
+  ret void
+}
+
+attributes #0 = { argmemonly nounwind }
+attributes #1 = { nounwind readnone }
+attributes #2 = { nounwind }