[Statepoints][ISEL] gc.relocate uniquification should be based on SDValue, not IR Value.

[dantrushin]

Summary

When exporting statepoint results to virtual registers we try to avoid
generating exports for duplicated inputs. But we erroneously use
IR Value* to check if inputs are duplicated. Instead, we should use
SDValue, because even different IR values can get lowered to the same
SDValue.
I'm adding a (degenerate) test case which emphasizes importance of this
feature for invoke statepoints.
If we fail to export only unique values we will end up with something
like that:

%0 = STATEPOINT
%1 = COPY %0

landing_pad:

<use of %1>

And when exceptional path is taken, %1 is left uninitialized (COPY is never
execute).

[reames]

LGTM

However, I think this a workaround (and a useful optimization) not a true fix. What would happen if for some reason a COPY didn't get eliminated? (Imagine no duplicates, just a single pointer being relocated, with relocated value used down the exceptional path) I don't have a test case for this, but I'd advise digging into that possibility further. I'd need to study invoke lowering more closely to understand the risk.

(Just to be clear, it is a useful optimization and that's the basis for the LGTM.)

llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp

#endif
   NodeTys.push_back(MVT::Other);
   NodeTys.push_back(MVT::Glue);
 
   unsigned NumResults = NodeTys.size();
   MachineSDNode *StatepointMCNode =
     DAG.getMachineNode(TargetOpcode::STATEPOINT, getCurSDLoc(), NodeTys, Ops);
   DAG.setNodeMemRefs(StatepointMCNode, MemRefs);
 
-
   // For values lowered to tied-defs, create the virtual registers.  Note that
-  // for simplicity, we *always* create a vreg even within a single block. 
-  DenseMap<const Value *, Register> VirtRegs;
+  // for simplicity, we *always* create a vreg even within a single block.
+  DenseMap<SDValue, Register> VirtRegs;
   for (const auto *Relocate : SI.GCRelocates) {
     Value *Derived = Relocate->getDerivedPtr();
     SDValue SD = getValue(Derived);
     if (!LowerAsVReg.count(SD))
       continue;
 
     // Handle multiple gc.relocates of the same input efficiently.
-    if (VirtRegs.count(Derived))
+    if (VirtRegs.count(SD))
       continue;
 
     SDValue Relocated = SDValue(StatepointMCNode, LowerAsVReg[SD]);
 
     auto *RetTy = Relocate->getType();
     Register Reg = FuncInfo.CreateRegs(RetTy);
     RegsForValue RFV(*DAG.getContext(), DAG.getTargetLoweringInfo(),
                      DAG.getDataLayout(), Reg, RetTy, None);
     SDValue Chain = DAG.getRoot();
     RFV.getCopyToRegs(Relocated, DAG, getCurSDLoc(), Chain, nullptr);
     PendingExports.push_back(Chain);
-    
-    VirtRegs[Derived] = Reg; 
+    VirtRegs[SD] = Reg;
   }
 
   // Record for later use how each relocation was lowered.  This is needed to
   // allow later gc.relocates to mirror the lowering chosen.
   const Instruction *StatepointInstr = SI.StatepointInstr;
   auto &RelocationMap = FuncInfo.StatepointRelocationMaps[StatepointInstr];
   for (const GCRelocateInst *Relocate : SI.GCRelocates) {
     const Value *V = Relocate->getDerivedPtr();
     SDValue SDV = getValue(V);
     SDValue Loc = StatepointLowering.getLocation(SDV);
 
     RecordType Record;
     if (LowerAsVReg.count(SDV)) {
       Record.type = RecordType::VReg;
-      assert(VirtRegs.count(V));
-      Record.payload.Reg = VirtRegs[V];
+      assert(VirtRegs.count(SDV));
+      Record.payload.Reg = VirtRegs[SDV];
     } else if (Loc.getNode()) {
       Record.type = RecordType::Spill;
       Record.payload.FI = cast<FrameIndexSDNode>(Loc)->getIndex();
     } else {
       Record.type = RecordType::NoRelocate;
       // If we didn't relocate a value, we'll essentialy end up inserting an
       // additional use of the original value when lowering the gc.relocate.
       // We need to make sure the value is available at the new use, which

llvm/test/CodeGen/X86/statepoint-vreg-details.ll

 target triple = "x86_64-pc-linux-gnu"
 
 declare i1 @return_i1()
 declare void @func()
 declare void @consume(i32 addrspace(1)*)
 declare void @consume2(i32 addrspace(1)*, i32 addrspace(1)*)
 declare void @consume5(i32 addrspace(1)*, i32 addrspace(1)*, i32 addrspace(1)*, i32 addrspace(1)*, i32 addrspace(1)*)
 declare void @use1(i32 addrspace(1)*, i8 addrspace(1)*)
+declare i32* @fake_personality_function()
+declare i32 @foo(i32, i8 addrspace(1)*, i32, i32, i32)
 
 ; test most simple relocate
 define i1 @test_relocate(i32 addrspace(1)* %a) gc "statepoint-example" {
 ; CHECK-VREG-LABEL: name:            test_relocate
 ; CHECK-VREG:    %0:gr64 = COPY $rdi
 ; CHECK-VREG:    MOV64mr %stack.0, 1, $noreg, 0, $noreg, %0 :: (store 8 into %stack.0)
 ; CHECK-VREG:    %1:gr64 = STATEPOINT 0, 0, 0, @return_i1, 2, 0, 2, 0, 2, 0, 1, 8, %stack.0, 0, %0(tied-def 0), csr_64, implicit-def $rsp, implicit-def $ssp, implicit-def $al :: (volatile load store 8 on %stack.0)
 ; CHECK-VREG:    %2:gr8 = COPY $al

 }
 
 ; Two gc.relocates of a bitcasted pointer should only require a single spill/fill
 define void @test_gcptr_uniqueing(i32 addrspace(1)* %ptr) gc "statepoint-example" {
 ; CHECK-VREG-LABEL: name:            test_gcptr_uniqueing
 ; CHECK-VREG:    %0:gr64 = COPY $rdi
 ; CHECK-VREG:    MOV64mr %stack.0, 1, $noreg, 0, $noreg, %0 :: (store 8 into %stack.0)
 ; CHECK-VREG:    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
-; CHECK-VREG:    %2:gr64 = STATEPOINT 0, 0, 0, @func, 2, 0, 2, 0, 2, 2, %0, 2, 4278124286, 1, 8, %stack.0, 0, %0(tied-def 0), csr_64, implicit-def $rsp, implicit-def $ssp :: (volatile load store 8 on %stack.0)
+; CHECK-VREG:    %1:gr64 = STATEPOINT 0, 0, 0, @func, 2, 0, 2, 0, 2, 2, %0, 2, 4278124286, 1, 8, %stack.0, 0, %0(tied-def 0), csr_64, implicit-def $rsp, implicit-def $ssp :: (volatile load store 8 on %stack.0)
 ; CHECK-VREG:    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
-; CHECK-VREG:    %1:gr64 = COPY %2
 ; CHECK-VREG:    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
-; CHECK-VREG:    $rdi = COPY %2
+; CHECK-VREG:    $rdi = COPY %1
 ; CHECK-VREG:    $rsi = COPY %1
 ; CHECK-VREG:    CALL64pcrel32 @use1, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit $rsi, implicit-def $rsp, implicit-def $ssp
 
 ; CHECK-PREG-LABEL: name:            test_gcptr_uniqueing
 ; CHECK-PREG:    renamable $rbx = COPY $rdi
 ; CHECK-PREG:    MOV64mr %stack.0, 1, $noreg, 0, $noreg, renamable $rbx :: (store 8 into %stack.0)
 ; CHECK-PREG:    renamable $rbx = STATEPOINT 0, 0, 0, @func, 2, 0, 2, 0, 2, 2, killed renamable $rbx, 2, 4278124286, 1, 8, %stack.0, 0, renamable $rbx(tied-def 0), csr_64, implicit-def $rsp, implicit-def $ssp :: (volatile load store 8 on %stack.0)
 ; CHECK-PREG:    $rdi = COPY renamable $rbx
 ; CHECK-PREG:    $rsi = COPY killed renamable $rbx
 ; CHECK-PREG:    CALL64pcrel32 @use1, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit killed $rsi, implicit-def $rsp, implicit-def $ssp
 
   %ptr2 = bitcast i32 addrspace(1)* %ptr to i8 addrspace(1)*
   %tok = tail call token (i64, i32, void ()*, i32, i32, ...)
       @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) ["deopt" (i32 addrspace(1)* %ptr, i32 undef), "gc-live" (i32 addrspace(1)* %ptr, i8 addrspace(1)* %ptr2)]
   %a = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %tok, i32 0, i32 0)
   %b = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %tok, i32 1, i32 1)
   call void @use1(i32 addrspace(1)* %a, i8 addrspace(1)* %b)
   ret void
 }
 
-;
-; Cross-basicblock relocates are handled with spilling for now.
-; No need to check post-RA output
 define i1 @test_cross_bb(i32 addrspace(1)* %a, i1 %external_cond) gc "statepoint-example" {
 ; CHECK-VREG-LABEL: name:            test_cross_bb
 ; CHECK-VREG:  bb.0.entry:
 ; CHECK-VREG:         %1:gr32 = COPY $esi
 ; CHECK-VREG-NEXT:    %0:gr64 = COPY $rdi
 ; CHECK-VREG-NEXT:    %4:gr8 = COPY %1.sub_8bit
 ; CHECK-VREG-NEXT:    MOV64mr %stack.0, 1, $noreg, 0, $noreg, %0 :: (store 8 into %stack.0)
 ; CHECK-VREG-NEXT:    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
entry:
   %rel2 = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %safepoint_token,  i32 1, i32 1)
   %rel3 = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %safepoint_token,  i32 2, i32 2)
   %rel4 = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %safepoint_token,  i32 3, i32 3)
   %rel5 = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %safepoint_token,  i32 4, i32 4)
   call void @consume5(i32 addrspace(1)* %rel1, i32 addrspace(1)* %rel2, i32 addrspace(1)* %rel3, i32 addrspace(1)* %rel4, i32 addrspace(1)* %rel5)
   ret void
 }
 
+; Different IR Values which maps to the same SDValue must be assigned to the same VReg.
+; This is test is similar to test_gcptr_uniqueing but explicitly uses invokes for which this is important
+; Otherwise we may get a copy of statepoint result, inserted at the end ot statepoint block and used at landing pad
+define void @test_duplicate_ir_values() gc "statepoint-example" personality i32* ()* @fake_personality_function{
+;CHECK-VREG-LABEL: name:            test_duplicate_ir_values
+;CHECK-VREG:   bb.0.entry:
+;CHECK-VREG:     %0:gr64 = STATEPOINT 1, 16, 5, %8, $edi, $rsi, $edx, $ecx, $r8d, 2, 0, 2, 0, 2, 0, 1, 8, %stack.0, 0, %1(tied-def 0), csr_64, implicit-def $rsp, implicit-def $ssp, implicit-def $eax :: (volatile load store 8 on %stack.0)
+;CHECK-VREG:     JMP_1 %bb.1
+;CHECK-VREG:   bb.1.normal_continue:
+;CHECK-VREG:     MOV64mr %stack.0, 1, $noreg, 0, $noreg, %0 :: (store 8 into %stack.0)
+;CHECK-VREG:     %13:gr32 = MOV32ri 10
+;CHECK-VREG:     $edi = COPY %13
+;CHECK-VREG:     STATEPOINT 2882400000, 0, 1, @__llvm_deoptimize, $edi, 2, 0, 2, 2, 2, 2, 1, 8, %stack.0, 0, 1, 8, %stack.0, 0, csr_64, implicit-def $rsp, implicit-def $ssp :: (volatile load store 8 on %stack.0)
+;CHECK-VREG:   bb.2.exceptional_return (landing-pad):
+;CHECK-VREG:     EH_LABEL <mcsymbol >
+;CHECK-VREG:     MOV64mr %stack.0, 1, $noreg, 0, $noreg, %0 :: (store 8 into %stack.0)
+;CHECK-VREG:     %12:gr32 = MOV32ri -271
+;CHECK-VREG:     $edi = COPY %12
+;CHECK-VREG:     STATEPOINT 2882400000, 0, 1, @__llvm_deoptimize, $edi, 2, 0, 2, 0, 2, 1, 1, 8, %stack.0, 0, csr_64, implicit-def $rsp, implicit-def $ssp :: (volatile load store 8 on %stack.0)
+
+entry:
+  %local.0 = load i8 addrspace(1)*, i8 addrspace(1)* addrspace(1)* undef, align 8
+  %local.9 = load i8 addrspace(1)*, i8 addrspace(1)* addrspace(1)* undef, align 8
+  %statepoint_token1 = invoke token (i64, i32, i32 (i32, i8 addrspace(1)*, i32, i32, i32)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_i32i32p1i8i32i32i32f(i64 1, i32 16, i32 (i32, i8 addrspace(1)*, i32, i32, i32)* nonnull @foo, i32 5, i32 0, i32 undef, i8 addrspace(1)* undef, i32 undef, i32 undef, i32 undef, i32 0, i32 0) [ "deopt"(), "gc-live"(i8 addrspace(1)* %local.0, i8 addrspace(1)* %local.9) ]
+          to label %normal_continue unwind label %exceptional_return
+
+normal_continue: ; preds = %entry
+  %local.0.relocated1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %statepoint_token1, i32 0, i32 0) ; (%local.0, %local.0)
+  %local.9.relocated1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %statepoint_token1, i32 1, i32 1) ; (%local.9, %local.9)
+  %safepoint_token2 = call token (i64, i32, void (i32)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidi32f(i64 2882400000, i32 0, void (i32)* nonnull @__llvm_deoptimize, i32 1, i32 2, i32 10, i32 0, i32 0) [ "deopt"(i8 addrspace(1)* %local.0.relocated1, i8 addrspace(1)* %local.9.relocated1), "gc-live"() ]
+  unreachable
+
+exceptional_return:                         ; preds = %entry
+  %lpad_token11090 = landingpad token
+          cleanup
+  %local.9.relocated2 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %lpad_token11090, i32 1, i32 1) ; (%local.9, %local.9)
+  %safepoint_token3 = call token (i64, i32, void (i32)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidi32f(i64 2882400000, i32 0, void (i32)* nonnull @__llvm_deoptimize, i32 1, i32 0, i32 -271, i32 0, i32 0) [ "deopt"(i8 addrspace(1)* %local.9.relocated2), "gc-live"() ]
+  unreachable
+}
+
 declare token @llvm.experimental.gc.statepoint.p0f_i1f(i64, i32, i1 ()*, i32, i32, ...)
 declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
 declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token, i32, i32)
 declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32)
 declare <2 x i8 addrspace(1)*> @llvm.experimental.gc.relocate.v2p1i8(token, i32, i32)
 declare i1 @llvm.experimental.gc.result.i1(token)
+declare void @__llvm_deoptimize(i32)
+declare token @llvm.experimental.gc.statepoint.p0f_isVoidi32f(i64 immarg, i32 immarg, void (i32)*, i32 immarg, i32 immarg, ...)
+declare token @llvm.experimental.gc.statepoint.p0f_i32i32p1i8i32i32i32f(i64 immarg, i32 immarg, i32 (i32, i8 addrspace(1)*, i32, i32, i32)*, i32 immarg, i32 immarg, ...)
 
-; Entry for test_relocate
-; Num locations
-; Location 1 Constant 0
-; Location 2 Constant 0
-; Location 3 Constant 0
-; Location 4 Register $rbx
-; Location 5 Register $rbx
-;  Entry for test_mixed
-; Num locations
-; Location 1 Constant 0
-; Location 2 Constant 0
-; Location 3 Constant 0
-; Location 4 Register $r14
-; Location 5 Register $r14
-; Location 6 Constant 0
-; Location 7 Constant 0
-; Location 8 Register $r15
-; Location 9 Register $r15
-; Location 10 Register $rbx
-; Location 11 Register $rbx
-; Entry for test_alloca
-; Num locations
-; Location 1 Constant 0
-; Location 2 Constant 0
-; Location 3 Constant 0
-; Location 4 Register $rbx
-; Location 5 Register $rbx
-; Location 6 Direct $rsp + 0
-; Entry for test_base_derive
-; Num locations
-; Location 1 Constant 0
-; Location 2 Constant 0
-; Location 3 Constant 0
-; Location 4 Indirect $rsp + 8
-; Location 5 Register $rbx
-; Entry for test_deopt_gcpointer
-; Num locations
-; Location 1 Constant 0
-; Location 2 Constant 0
-; Location 3 Constant 1
-; Location 4Indirect $rsp + 8
-; Location 5 Register $rbx
-; Location 6
-; Entry for test_gcrelocate_uniqueing
-; Num locations
-; Location 1 Constant 0
-; Location 2 Constant 0
-; Location 3 Constant 2
-; Location 4 Register $rbx
-; Location 5 Constant Index 0
-; Location 6 Register $rbx
-; Location 7 Register $rbx
-; Entry for test_gcptr_uniqueing
-; Num locations
-; Location 1 Constant 0
-; Location 2 Constant 0
-; Location 3 Constant 2
-; Location 4 Register $rbx
-; Location 5 Constant Index 0
-; Location 6 Register $rbx
-; Location 7 Register $rbx
-; Entry for test_cross_bb
-; Num locations
-; Location 1 Constant 0
-; Location 2 Constant 0
-; Location 3 Constant 0
-; Location 4 Indirect $rsp + 0
-; Location 5 Indirect $rsp + 0