[PATCH] D76013: [GC] Loosen ordering on statepoint reloads to allow CSE

[Philip Reames via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f997b4f0118: [GC] Loosen ordering on statepoint reloads to allow CSE (authored by reames).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76013/new/

https://reviews.llvm.org/D76013

Files:
  llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
  llvm/test/CodeGen/X86/statepoint-duplicates-export.ll


Index: llvm/test/CodeGen/X86/statepoint-duplicates-export.ll
===================================================================
--- llvm/test/CodeGen/X86/statepoint-duplicates-export.ll
+++ llvm/test/CodeGen/X86/statepoint-duplicates-export.ll
@@ -44,19 +44,16 @@
 define i1 @test2(i32 addrspace(1)* %arg) gc "statepoint-example" {
 ; CHECK-LABEL: test2:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    subq $24, %rsp
-; CHECK-NEXT:    .cfi_def_cfa_offset 32
-; CHECK-NEXT:    movq %rdi, {{[0-9]+}}(%rsp)
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    movq %rdi, (%rsp)
 ; CHECK-NEXT:    callq func
 ; CHECK-NEXT:  .Ltmp2:
-; CHECK-NEXT:    movq {{[0-9]+}}(%rsp), %rax
-; CHECK-NEXT:    movq %rax, {{[0-9]+}}(%rsp)
 ; CHECK-NEXT:    callq func
 ; CHECK-NEXT:  .Ltmp3:
-; CHECK-NEXT:    movq {{[0-9]+}}(%rsp), %rax
-; CHECK-NEXT:    orq {{[0-9]+}}(%rsp), %rax
+; CHECK-NEXT:    cmpq $0, (%rsp)
 ; CHECK-NEXT:    sete %al
-; CHECK-NEXT:    addq $24, %rsp
+; CHECK-NEXT:    popq %rcx
 ; CHECK-NEXT:    .cfi_def_cfa_offset 8
 ; CHECK-NEXT:    retq
 entry:
Index: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
+++ llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
@@ -973,10 +973,13 @@
   unsigned Index = *DerivedPtrLocation;
   SDValue SpillSlot = DAG.getTargetFrameIndex(Index, getFrameIndexTy());
 
-  // Note: We know all of these reloads are independent, but don't bother to
-  // exploit that chain wise.  DAGCombine will happily do so as needed, so
-  // doing it here would be a small compile time win at most.
-  SDValue Chain = getRoot();
+  // All the reloads are independent and are reading memory only modified by
+  // statepoints (i.e. no other aliasing stores); informing SelectionDAG of
+  // this this let's CSE kick in for free and allows reordering of instructions
+  // if possible.  The lowering for statepoint sets the root, so this is
+  // ordering all reloads with the either a) the statepoint node itself, or b)
+  // the entry of the current block for an invoke statepoint.
+  const SDValue Chain = DAG.getRoot(); // != Builder.getRoot()
 
   auto &MF = DAG.getMachineFunction();
   auto &MFI = MF.getFrameInfo();
@@ -991,8 +994,7 @@
 
   SDValue SpillLoad = DAG.getLoad(LoadVT, getCurSDLoc(), Chain,
                                   SpillSlot, LoadMMO);
-
-  DAG.setRoot(SpillLoad.getValue(1));
+  PendingLoads.push_back(SpillLoad.getValue(1));
 
   assert(SpillLoad.getNode());
   setValue(&Relocate, SpillLoad);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76013.249728.patch
Type: text/x-patch
Size: 2622 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200311/1427651e/attachment.bin>