InferAddressSpaces: Avoid assertion failure with replacing identical cloned constexpr

[niravd]

Summary

Have cloneConstantExprWithNewAddressSpaces return nullptr when
returning initial ConstantExpr.

[arsenm]

I'm not sure I see why this would crash?

test/Transforms/InferAddressSpaces/NVPTX/clone_constexpr.ll

3–4

Should remove these since they are redundant with the opt argument

23

Name values

41–43

You should be able to remove all the metadata in the test

41–53

You should be able to remove all the metadata in the test

[niravd]

In D33995#775210, @arsenm wrote:

I'm not sure I see why this would crash?

I should have written it as assert. When the replacement is done, it assume that the new value is an address space cast and asserts in llvm::ConstantExpr::getAddrSpaceCast.

[niravd]

Address comments

[arsenm]

In D33995#775320, @niravd wrote:

In D33995#775210, @arsenm wrote:

I'm not sure I see why this would crash?

I should have written it as assert. When the replacement is done, it assume that the new value is an address space cast and asserts in llvm::ConstantExpr::getAddrSpaceCast.

LGTM. Can you add a comment to this effect somewhere though

lib/Transforms/Scalar/InferAddressSpaces.cpp

if (Src0->getType()->getPointerAddressSpace() ==
 
       return ConstantExpr::getSelect(
           CE->getOperand(0), ConstantExpr::getAddrSpaceCast(Src0, TargetType),
           ConstantExpr::getAddrSpaceCast(Src1, TargetType));
     }
   }
 
   // Computes the operands of the new constant expression.
+  bool IsNew = false;
   SmallVector<Constant *, 4> NewOperands;
   for (unsigned Index = 0; Index < CE->getNumOperands(); ++Index) {
     Constant *Operand = CE->getOperand(Index);
     // If the address space of `Operand` needs to be modified, the new operand
     // with the new address space should already be in ValueWithNewAddrSpace
     // because (1) the constant expressions we consider (i.e. addrspacecast,
     // bitcast, and getelementptr) do not incur cycles in the data flow graph
     // and (2) this function is called on constant expressions in postorder.
     if (Value *NewOperand = ValueWithNewAddrSpace.lookup(Operand)) {
+      IsNew = true;
       NewOperands.push_back(cast<Constant>(NewOperand));
     } else {
       // Otherwise, reuses the old operand.
       NewOperands.push_back(Operand);
     }
   }
 
+  if (!IsNew)
+    return nullptr;
+
   if (CE->getOpcode() == Instruction::GetElementPtr) {
     // Needs to specify the source type while constructing a getelementptr
     // constant expression.
     return CE->getWithOperands(
       NewOperands, TargetType, /*OnlyIfReduced=*/false,
       NewOperands[0]->getType()->getPointerElementType());
   }
 

test/Transforms/InferAddressSpaces/NVPTX/clone_constexpr.ll

+; RUN: opt -S -mtriple=nvptx64-nvidia-cuda -infer-address-spaces %s | FileCheck %s
+
+target datalayout = "e-i64:64-v16:16-v32:32-n16:32:64"
+target triple = "nvptx64-nvidia-cuda"
+
+%struct.S = type { [5 x i32] }
+
+$g1 = comdat any
+
+@g1 = linkonce_odr addrspace(3) global %struct.S zeroinitializer, comdat, align 4
+
+; CHECK-LABEL: @foo(
+; CHECK:  %0 = tail call i32 @llvm.nvvm.read.ptx.sreg.tid.x() #2, !range !4
+; CHECK:  %idxprom.i = zext i32 %0 to i64
+; CHECK:  %arrayidx.i = getelementptr %struct.S, %struct.S* addrspacecast (%struct.S addrspace(3)* @g1 to %struct.S*), i64 0, i32 0, i64 %idxprom.i
+; CHECK:  tail call void @f1(i32* %arrayidx.i, i32 undef) #0
+; CHECK:  %1 = load i32, i32* getelementptr (%struct.S, %struct.S* addrspacecast (%struct.S addrspace(3)* @g1 to %struct.S*), i64 0, i32 0, i64 0), align 4, !tbaa !5
+; CHECK:  %L.sroa.0.0.insert.ext.i = zext i32 %1 to i64
+; CHECK:  tail call void @f2(i64* null, i64 %L.sroa.0.0.insert.ext.i) #0
+; CHECK:  ret void
+define void @foo() local_unnamed_addr #0 {
+entry:
+  %0 = tail call i32 @llvm.nvvm.read.ptx.sreg.tid.x() #2, !range !4
+  %idxprom.i = zext i32 %0 to i64
+  %arrayidx.i = getelementptr %struct.S, %struct.S* addrspacecast (%struct.S addrspace(3)* @g1 to %struct.S*), i64 0, i32 0, i64 %idxprom.i
+  tail call void @f1(i32* %arrayidx.i, i32 undef) #0
+  %1 = load i32, i32* getelementptr (%struct.S, %struct.S* addrspacecast (%struct.S addrspace(3)* @g1 to %struct.S*), i64 0, i32 0, i64 0), align 4, !tbaa !5
+  %L.sroa.0.0.insert.ext.i = zext i32 %1 to i64
+  tail call void @f2(i64* null, i64 %L.sroa.0.0.insert.ext.i) #0
+  ret void
+}
+
+declare void @f1(i32*, i32) local_unnamed_addr #0
+declare void @f2(i64*, i64) local_unnamed_addr #0
+declare i32 @llvm.nvvm.read.ptx.sreg.tid.x() #1
+
+attributes #0 = { convergent nounwind }
+attributes #1 = { nounwind readnone }
+attributes #2 = { nounwind }
+
+!nvvm.annotations = !{!0}
+!llvm.module.flags = !{!1, !2}
+!llvm.ident = !{!3}
+
+!0 = !{void ()* @foo, !"kernel", i32 1}
+!1 = !{i32 1, !"wchar_size", i32 4}
+!2 = !{i32 4, !"nvvm-reflect-ftz", i32 0}
+!3 = !{!"clang version 5.0.0 (trunk 304904) (llvm/trunk 304910)"}
+!4 = !{i32 0, i32 1024}
+!5 = !{!6, !6, i64 0}
+!6 = !{!"int", !7, i64 0}
+!7 = !{!"omnipotent char", !8, i64 0}
+!8 = !{!"Simple C++ TBAA"}