Optimize cross block gc.relocate lowering. NFC.

[dantrushin]

Summary

At IR level gc.relocate is used to express new value of GC pointer
which may have changed during call wrapped by statepoint (IR instructions
have only single def; gc.relocate is a workaround for this limitation).
But MachineInstruction can have multiple results (DEF registers),
so gc.relocate is not needed here - it is equal to the corresponding
STATEPOINT DEF operand (when GC pointers are lowered via VRegs).

This means that during lowering of gc.relocate which has uses outside
its basic block we can simply use virtual register exported by STATEPOINT
and not generate CopyFromRegs/CopyToRegs SDNodes which copy one virtual
register to another, producing redundant COPY instruction.

This is purely compile time optimization. On big methods it can
improve compile time up to 10%.

[dantrushin]

ping

[dantrushin]

Rebase on tip

[skatkov]

Generally looks good. Could please land test first in the current implementation to see the difference caused by patch.

llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp

1270–1271

please use spaces...

[dantrushin]

Test update.

[dantrushin]

Update test comment lost during merge

[skatkov]

Well.. Do I understand correctly that the original problem comes from the fact that during lowering of statepoint if value is relocated through register then this register is exported unconditionally?

So if during statepoint lowering we export the value only in case there is a relocate outside of current basic block, the extra copy is disappeared.
gc.relocate should be handled in the same way - if there is a use outside of this basic block then export is needed otherwise no export at all. As I understand this corresponds to usual handling of instruction and no special handling of gc.relocate is required?

Do I miss anything?

[dantrushin]

In D123530#3473773, @skatkov wrote:

Well.. Do I understand correctly that the original problem comes from the fact that during lowering of statepoint if value is relocated through register then this register is exported unconditionally?

So if during statepoint lowering we export the value only in case there is a relocate outside of current basic block, the extra copy is disappeared.
gc.relocate should be handled in the same way - if there is a use outside of this basic block then export is needed otherwise no export at all. As I understand this corresponds to usual handling of instruction and no special handling of gc.relocate is required?

Yes.

Do I miss anything?

Well, the keyword here is simplicity. To make it work that way some tricks are needed to avoid complicated implementation (it already is not that simple). Please have a look at alternative solution: https://reviews.llvm.org/D124444

[dantrushin]

Superceeded by D124444

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::visit(const Instruction &I) {
   // Increase the SDNodeOrder if dealing with a non-debug instruction.
   if (!isa<DbgInfoIntrinsic>(I))
     ++SDNodeOrder;
 
   CurInst = &I;
 
   visit(I.getOpcode(), I);
 
-  if (!I.isTerminator() && !HasTailCall &&
-      !isa<GCStatepointInst>(I)) // statepoints handle their exports internally
+  // statepoints and relocates handle their exports internally
+  if (!I.isTerminator() && !HasTailCall && !isa<GCStatepointInst>(I) &&
+      !isa<GCRelocateInst>(I))
     CopyToExportRegsIfNeeded(&I);
 
   CurInst = nullptr;
 }
 
 void SelectionDAGBuilder::visitPHI(const PHINode &) {
   llvm_unreachable("SelectionDAGBuilder shouldn't visit PHI nodes!");
 }

llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp

RegsForValue RFV(*DAG.getContext(), DAG.getTargetLoweringInfo(),
                      None); // This is not an ABI copy.
     // We generate copy to/from regs even for local uses, hence we must
     // chain with current root to ensure proper ordering of copies w.r.t.
     // statepoint.
     SDValue Chain = DAG.getRoot();
     SDValue Relocation = RFV.getCopyFromRegs(DAG, FuncInfo, getCurSDLoc(),
                                              Chain, nullptr, nullptr);
     setValue(&Relocate, Relocation);
+    // Reuse corresponding statepoint register.
+    FuncInfo.ValueMap[&Relocate] = InReg;
     return;
   }
 
   if (Record.type == RecordType::Spill) {
     unsigned Index = Record.payload.FI;
     SDValue SpillSlot = DAG.getTargetFrameIndex(Index, getFrameIndexTy());
 
     // All the reloads are independent and are reading memory only modified by
auto LoadVT = DAG.getTargetLoweringInfo().getValueType(DAG.getDataLayout(),
                                                            Relocate.getType());
 
     SDValue SpillLoad =
         DAG.getLoad(LoadVT, getCurSDLoc(), Chain, SpillSlot, LoadMMO);
     PendingLoads.push_back(SpillLoad.getValue(1));
 
     assert(SpillLoad.getNode());
     setValue(&Relocate, SpillLoad);
-    return;
-  }
+  } else {
 
     assert(Record.type == RecordType::NoRelocate);
     SDValue SD = getValue(DerivedPtr);
 
     if (SD.isUndef() && SD.getValueType().getSizeInBits() <= 64) {
       // Lowering relocate(undef) as arbitrary constant. Current constant value
       // is chosen such that it's unlikely to be a valid pointer.
-    setValue(&Relocate, DAG.getTargetConstant(0xFEFEFEFE, SDLoc(SD), MVT::i64));
-    return;
+      SD = DAG.getTargetConstant(0xFEFEFEFE, SDLoc(SD), MVT::i64);
     }
 
     // We didn't need to spill these special cases (constants and allocas).
     // See the handling in spillIncomingValueForStatepoint for detail.
     setValue(&Relocate, SD);
   }
+  CopyToExportRegsIfNeeded(&Relocate);
+}
 
 void SelectionDAGBuilder::LowerDeoptimizeCall(const CallInst *CI) {
   const auto &TLI = DAG.getTargetLoweringInfo();
   SDValue Callee = DAG.getExternalSymbol(TLI.getLibcallName(RTLIB::DEOPTIMIZE),
                                          TLI.getPointerTy(DAG.getDataLayout()));
 
   // We don't lower calls to __llvm_deoptimize as varargs, but as a regular
   // call.  We also do not lower the return value to any virtual register, and

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

entry:
   %ptr = select i1 %cmp, i8 addrspace(1)* %0, i8 addrspace(1)* %1
   %token = call token (i64, i32, void (i8 addrspace(1)*, i8 addrspace(1)*)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidp1i8p1i8f(i64 10, i32 0, void (i8 addrspace(1)*, i8 addrspace(1)*)* elementtype(void (i8 addrspace(1)*, i8 addrspace(1)*)) @bar, i32 2, i32 0, i8 addrspace(1)* null, i8 addrspace(1)* %ptr, i32 0, i32 0) [ "deopt"(), "gc-live"(i8 addrspace(1)* %0, i8 addrspace(1)* %1) ]
   %rel0 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %token, i32 0, i32 0)
   %rel1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %token, i32 1, i32 1)
   %res = select i1 %cmp, i8 addrspace(1)* %rel0, i8 addrspace(1)* %rel1
   ret i8 addrspace(1)* %res
 }
 
-; Show that ISEL of gc.relocate used in other BB does generate extra COPY instruction.
+; Check that ISEL of gc.relocate used in other BB does not generate extra COPY instruction.
 define i1 @test_cross_bb_reloc(i32 addrspace(1)* %a, i1 %external_cond) gc "statepoint-example" {
 ; CHECK-VREG_LABEL: test_cross_bb_reloc:
 ; CHECK-VREG:    bb.0.entry:
 ; CHECK-VREG:      [[VREG:%[^ ]+]]:gr64 = STATEPOINT 0, 0, 0, @return_i1, 2, 0, 2, 0, 2, 0, 2, 1, %2(tied-def 0), 2, 0, 2, 1, 0, 0, csr_64, implicit-def $rsp, implicit-def $ssp, implicit-def $al
-; CHECK-VREG:      [[EXTRA:%[^ ]+]]:gr64 = COPY [[VREG]]
+; CHECK-VREG-NOT:  COPY [[VREG]]
 ; CHECK-VREG:    bb.1.left:
-; CHECK-VREG:      $rdi = COPY [[EXTRA]]
+; CHECK-VREG:      $rdi = COPY [[VREG]]
 ; CHECK-VREG:      CALL64pcrel32 @consume, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp
 ; CHECK-VREG:      $al = COPY %1
 ; CHECK-VREG:      RET 0, $al
 
 entry:
   %safepoint_token = tail call token (i64, i32, i1 ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_i1f(i64 0, i32 0, i1 ()* elementtype(i1 ()) @return_i1, i32 0, i32 0, i32 0, i32 0) ["gc-live" (i32 addrspace(1)* %a)]
   %call1 = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %safepoint_token,  i32 0, i32 0)
   %call2 = call zeroext i1 @llvm.experimental.gc.result.i1(token %safepoint_token)