[PATCH] D11768: FastISel fails to remove dead code

[Wolfgang Pieb]

wolfgangp created this revision.
wolfgangp added a subscriber: llvm-commits.

When FastISel fails to translate an instruction it hands off code generation to SelectionDAG. Before it does so, it may have generated local value instructions to feed phi nodes in successor blocks. These instructions will then be generated again by SelectionDAG, causing duplication and less efficient code, including extra spill instructions.
Consider the following example:


define zeroext i1 @_Z3fooee(x86_fp80 %x, x86_fp80 %y) {
entry:
  %x.addr = alloca x86_fp80, align 16
  %y.addr = alloca x86_fp80, align 16
  store x86_fp80 %x, x86_fp80* %x.addr, align 16
  store x86_fp80 %y, x86_fp80* %y.addr, align 16
  %0 = load x86_fp80, x86_fp80* %x.addr, align 16
  %1 = load x86_fp80, x86_fp80* %y.addr, align 16
  %cmp = fcmp oeq x86_fp80 %0, %1
  br i1 %cmp, label %lor.end, label %lor.rhs

lor.rhs:                                          ; preds = %entry
  %2 = load x86_fp80, x86_fp80* %x.addr, align 16
  %call = call zeroext i1 @_Z3bare(x86_fp80 %2)
  br label %lor.end

lor.end:                                          ; preds = %lor.rhs, %entry
  %3 = phi i1 [ true, %entry ], [ %call, %lor.rhs ]
  ret i1 %3
}

FastISel fails to translate one of the instructions in the entry block and leaves the rest of code generation to SelectionDAG. However, it fails to remove the instructions it generated to supply the phi node in the lor.end block:

        ...
        subq    $64, %rsp
        fldt    32(%rbp)
        fldt    16(%rbp)
        movb    $1, %al         <=======
        fstpt   -16(%rbp)
        fld     %st(0)
        fstpt   -32(%rbp)
        fldt    -16(%rbp)
        movb    $1, %cl         <=======
        fucompi %st(1)
        fstp    %st(0)
        movb    %al, -33(%rbp)          # 1-byte Spill <======== not used later
        movb    %cl, -34(%rbp)          # 1-byte Spill <======== used
        jne     .LBB0_1
        jp      .LBB0_1
        jmp     .LBB0_2
.LBB0_1: 
        ...


The patch proposes to remove all phi-node handling instructions as dead code when FastISel quits.


http://reviews.llvm.org/D11768

Files:
  lib/CodeGen/SelectionDAG/FastISel.cpp
  test/CodeGen/X86/fast-isel-deadcode.ll

Index: test/CodeGen/X86/fast-isel-deadcode.ll
===================================================================
--- test/CodeGen/X86/fast-isel-deadcode.ll
+++ test/CodeGen/X86/fast-isel-deadcode.ll
@@ -0,0 +1,39 @@
+; RUN: llc < %s -O0 -march=x86 | FileCheck %s
+;
+; Generated from
+;
+; extern bool bar (long double);
+; bool foo(long double x, long double y)
+; {
+;   return (x == y) || (bar(x));
+; }
+;
+; ModuleID = 'test.cpp'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define zeroext i1 @_Z3fooee(x86_fp80 %x, x86_fp80 %y) {
+entry:
+  %x.addr = alloca x86_fp80, align 16
+  %y.addr = alloca x86_fp80, align 16
+  store x86_fp80 %x, x86_fp80* %x.addr, align 16
+  store x86_fp80 %y, x86_fp80* %y.addr, align 16
+  %0 = load x86_fp80, x86_fp80* %x.addr, align 16
+  %1 = load x86_fp80, x86_fp80* %y.addr, align 16
+  %cmp = fcmp oeq x86_fp80 %0, %1
+  br i1 %cmp, label %lor.end, label %lor.rhs
+
+lor.rhs:                                          ; preds = %entry
+  %2 = load x86_fp80, x86_fp80* %x.addr, align 16
+  %call = call zeroext i1 @_Z3bare(x86_fp80 %2)
+  br label %lor.end
+
+lor.end:                                          ; preds = %lor.rhs, %entry
+  %3 = phi i1 [ true, %entry ], [ %call, %lor.rhs ]
+  ret i1 %3
+}
+
+declare zeroext i1 @_Z3bare(x86_fp80)
+
+; CHECK: movb $1
+; CHECK-NOT: movb $1
Index: lib/CodeGen/SelectionDAG/FastISel.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/FastISel.cpp
+++ lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -1327,6 +1327,7 @@
 }
 
 bool FastISel::selectInstruction(const Instruction *I) {
+  MachineInstr *SaveLastLocalValue = getLastLocalValue();
   // Just before the terminator instruction, insert instructions to
   // feed PHI nodes in successor blocks.
   if (isa<TerminatorInst>(I))
@@ -1380,8 +1381,21 @@
 
   DbgLoc = DebugLoc();
   // Undo phi node updates, because they will be added again by SelectionDAG.
-  if (isa<TerminatorInst>(I))
+  if (isa<TerminatorInst>(I)) {
+    // PHI node handling may have generated some local value instructions.
+    // Remove them like other dead code.
+    MachineInstr *CurLastLocalValue = getLastLocalValue();
+    if (CurLastLocalValue != SaveLastLocalValue) {
+      MachineBasicBlock::iterator FirstDeadInst(SaveLastLocalValue);
+      if (SaveLastLocalValue)
+        ++FirstDeadInst;
+      else
+        FirstDeadInst = FuncInfo.MBB->getFirstNonPHI();
+      setLastLocalValue(SaveLastLocalValue);
+      removeDeadCode(FirstDeadInst, FuncInfo.InsertPt);
+    }
     FuncInfo.PHINodesToUpdate.resize(FuncInfo.OrigNumPHINodesToUpdate);
+  }
   return false;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11768.31363.patch
Type: text/x-patch
Size: 2703 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150805/494274f6/attachment.bin>