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 ↗

(On Diff #101745)

Should remove these since they are redundant with the opt argument

23 ↗

(On Diff #101745)

Name values

41–43 ↗

(On Diff #101745)

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

41–53 ↗

(On Diff #101745)

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

llvm/trunk/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, we will replace the Value with itself. However, replaced values
+  // are assumed to wrapped in a addrspace cast later so drop it now.
+  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());
   }
 

llvm/trunk/test/Transforms/InferAddressSpaces/NVPTX/clone_constexpr.ll

+; RUN: opt -S -mtriple=nvptx64-nvidia-cuda -infer-address-spaces %s | FileCheck %s
+
+%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:  %x0 = tail call i32 @llvm.nvvm.read.ptx.sreg.tid.x() #2
+; CHECK:  %idxprom.i = zext i32 %x0 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:  %x1 = 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
+; CHECK:  %L.sroa.0.0.insert.ext.i = zext i32 %x1 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:
+  %x0 = tail call i32 @llvm.nvvm.read.ptx.sreg.tid.x() #2
+  %idxprom.i = zext i32 %x0 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
+  %x1 = 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
+  %L.sroa.0.0.insert.ext.i = zext i32 %x1 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 }