Revert EH-specific branch folding changes to avoid compilation slow down

[andrew.w.kaylor]

Summary

This patch reverts a previous change (D14996) which was blowing up compilation times under certain circumstances. I don't think the conditions which motivated that change can occur since the introduction of catchswitch statements.

Reverting this change led to a couple of tests failing because the order of exception handlers is different than the tests expect, but I do not believe the difference is significant so I just updated the tests to reflect the new order. I'm posting this review primarily to get a second opinion on that judgment.

[rnk]

lgtm

test/CodeGen/X86/wineh-coreclr.ll

309 ↗

(On Diff #65638)

Thanks for the test case. :)

[MatzeB]

Thanks for looking into this.

llvm/trunk/lib/CodeGen/BranchFolding.cpp

if (!CurFallsThru) {
         }
       }
 
       // Okay, there is no really great place to put this block.  If, however,
       // the block before this one would be a fall-through if this block were
       // removed, move this block to the end of the function.
       MachineBasicBlock *PrevTBB = nullptr, *PrevFBB = nullptr;
       SmallVector<MachineOperand, 4> PrevCond;
-      // We're looking for cases where PrevBB could possibly fall through to
-      // FallThrough, but if FallThrough is an EH pad that wouldn't be useful
-      // so here we skip over any EH pads so we might have a chance to find
-      // a branch target from PrevBB.
-      while (FallThrough != MF.end() && FallThrough->isEHPad())
-        ++FallThrough;
-      // Now check to see if the current block is sitting between PrevBB and
-      // a block to which it could fall through.
       if (FallThrough != MF.end() &&
           !TII->analyzeBranch(PrevBB, PrevTBB, PrevFBB, PrevCond, true) &&
           PrevBB.isSuccessor(&*FallThrough)) {
         MBB->moveAfter(&MF.back());
         MadeChange = true;
         return MadeChange;
       }
     }

llvm/trunk/test/CodeGen/WinEH/wineh-noret-cleanup.ll

 ; CXX-LABEL: test:
 ; CXX-LABEL: $ip2state$test:
 ; CXX-NEXT:   .long   .Lfunc_begin0@IMGREL
 ; CXX-NEXT:   .long   -1
 ; CXX-NEXT:   .long   .Ltmp0@IMGREL+1
 ; CXX-NEXT:   .long   1
 ; CXX-NEXT:   .long   .Ltmp1@IMGREL+1
 ; CXX-NEXT:   .long   -1
-; CXX-NEXT:   .long   "?catch$3@?0?test@4HA"@IMGREL
+; CXX-NEXT:   .long   "?catch$2@?0?test@4HA"@IMGREL
 ; CXX-NEXT:   .long   2
 ; CXX-NEXT:   .long   .Ltmp2@IMGREL+1
 ; CXX-NEXT:   .long   3
 ; CXX-NEXT:   .long   .Ltmp3@IMGREL+1
 ; CXX-NEXT:   .long   2
-; CXX-NEXT:   .long   "?catch$5@?0?test@4HA"@IMGREL
+; CXX-NEXT:   .long   "?catch$4@?0?test@4HA"@IMGREL
 ; CXX-NEXT:   .long   4
 
 ; SEH-LABEL: test:
 ; SEH-LABEL: .Llsda_begin0:
 ; SEH-NEXT:    .long   .Ltmp0@IMGREL+1
 ; SEH-NEXT:    .long   .Ltmp1@IMGREL+1
 ; SEH-NEXT:    .long   dummy_filter@IMGREL
-; SEH-NEXT:    .long   .LBB0_3@IMGREL
+; SEH-NEXT:    .long   .LBB0_2@IMGREL
 ; SEH-NEXT:    .long   .Ltmp0@IMGREL+1
 ; SEH-NEXT:    .long   .Ltmp1@IMGREL+1
 ; SEH-NEXT:    .long   dummy_filter@IMGREL
-; SEH-NEXT:    .long   .LBB0_5@IMGREL
+; SEH-NEXT:    .long   .LBB0_4@IMGREL
 ; SEH-NEXT:    .long   .Ltmp2@IMGREL+1
 ; SEH-NEXT:    .long   .Ltmp3@IMGREL+1
-; SEH-NEXT:    .long   "?dtor$2@?0?test@4HA"@IMGREL
+; SEH-NEXT:    .long   "?dtor$5@?0?test@4HA"@IMGREL
 ; SEH-NEXT:    .long   0
 ; SEH-NEXT:    .long   .Ltmp2@IMGREL+1
 ; SEH-NEXT:    .long   .Ltmp3@IMGREL+1
 ; SEH-NEXT:    .long   dummy_filter@IMGREL
-; SEH-NEXT:    .long   .LBB0_5@IMGREL
+; SEH-NEXT:    .long   .LBB0_4@IMGREL
 ; SEH-NEXT:  .Llsda_end0:

llvm/trunk/test/CodeGen/X86/wineh-coreclr.ll

 ;       } catch(type1) {
 ;       }
 ;       __unreachable();
 ;     }
 ;   } catch(type2) {
 ;   }
 ; }
 ;
-; CHECK-LABEL: test2:     # @test2
-; CHECK-NEXT: [[test2_begin:.*func_begin.*]]:
 define void @test2() personality i8* bitcast (void ()* @ProcessCLRException to i8*) {
 entry:
-; CHECK: .seh_endprologue
-; CHECK: [[test2_before_f1:.+]]:
-; CHECK-NEXT: movl $1, %ecx
-; CHECK-NEXT: callq f
-; CHECK-NEXT: [[test2_after_f1:.+]]:
   invoke void @f(i32 1)
     to label %exit unwind label %fault
 fault:
-; CHECK: .seh_proc [[test2_fault:[^ ]+]]
   %fault.pad = cleanuppad within none [i32 undef]
-; CHECK: .seh_endprologue
-; CHECK: [[test2_before_f2:.+]]:
-; CHECK-NEXT: movl $2, %ecx
-; CHECK-NEXT: callq f
-; CHECK-NEXT: [[test2_after_f2:.+]]:
   invoke void @f(i32 2) ["funclet"(token %fault.pad)]
     to label %unreachable unwind label %exn.dispatch.inner
 exn.dispatch.inner:
   %catchswitch.inner = catchswitch within %fault.pad [label %catch1] unwind label %exn.dispatch.outer
 catch1:
   %catch.pad1 = catchpad within %catchswitch.inner [i32 1]
-; CHECK: .seh_proc [[test2_catch1:[^ ]+]]
   catchret from %catch.pad1 to label %unreachable
 exn.dispatch.outer:
   %catchswitch.outer = catchswitch within none [label %catch2] unwind to caller
 catch2:
   %catch.pad2 = catchpad within %catchswitch.outer [i32 2]
-; CHECK: .seh_proc [[test2_catch2:[^ ]+]]
   catchret from %catch.pad2 to label %exit
 exit:
   ret void
 unreachable:
   unreachable
-; CHECK: [[test2_end:.*func_end.*]]:
 }
+; CHECK-LABEL: test2:     # @test2
+; CHECK-NEXT: [[test2_begin:.*func_begin.*]]:
+; CHECK: .seh_endprologue
+; CHECK: [[test2_before_f1:.+]]:
+; CHECK-NEXT: movl $1, %ecx
+; CHECK-NEXT: callq f
+; CHECK-NEXT: [[test2_after_f1:.+]]:
+; CHECK: .seh_proc [[test2_catch1:[^ ]+]]
+; CHECK: .seh_proc [[test2_catch2:[^ ]+]]
+; CHECK: .seh_proc [[test2_fault:[^ ]+]]
+; CHECK: .seh_endprologue
+; CHECK: [[test2_before_f2:.+]]:
+; CHECK-NEXT: movl $2, %ecx
+; CHECK-NEXT: callq f
+; CHECK-NEXT: [[test2_after_f2:.+]]:
+; CHECK: [[test2_end:.*func_end.*]]:
+
 
 ; Now check for EH table in xdata (following standard xdata)
 ; CHECK-LABEL: .section .xdata
 ; standard xdata comes here
 ; CHECK:      .long 3{{$}}
 ;                   ^ number of funclets
-; CHECK-NEXT: .long [[test2_fault]]-[[test2_begin]]
-;                   ^ offset from L_begin to start of 1st funclet
 ; CHECK-NEXT: .long [[test2_catch1]]-[[test2_begin]]
 ;                   ^ offset from L_begin to start of 2nd funclet
 ; CHECK-NEXT: .long [[test2_catch2]]-[[test2_begin]]
 ;                   ^ offset from L_begin to start of 3rd funclet
+; CHECK-NEXT: .long [[test2_fault]]-[[test2_begin]]
+;                   ^ offset from L_begin to start of 1st funclet
 ; CHECK-NEXT: .long [[test2_end]]-[[test2_begin]]
 ;                   ^ offset from L_begin to end of last funclet
 ; CHECK-NEXT: .long 4
 ;                   ^ number of EH clauses
 ; Clause 1: call f(1) is guarded by fault
 ; CHECK-NEXT: .long 4
 ;                   ^ flags (4 => fault handler)
 ; CHECK-NEXT: .long ([[test2_before_f1]]-[[test2_begin]])+1
 ;                   ^ offset of start of clause
 ; CHECK-NEXT: .long ([[test2_after_f1]]-[[test2_begin]])+1
 ;                   ^ offset of end of clause
 ; CHECK-NEXT: .long [[test2_fault]]-[[test2_begin]]
 ;                   ^ offset of start of handler
-; CHECK-NEXT: .long [[test2_catch1]]-[[test2_begin]]
+; CHECK-NEXT: .long [[test2_end]]-[[test2_begin]]
 ;                   ^ offset of end of handler
 ; CHECK-NEXT: .long 0
 ;                   ^ type token slot (null for fault)
 ; Clause 2: call f(1) is also guarded by catch2
 ; CHECK-NEXT: .long 0
 ;                   ^ flags (0 => catch handler)
 ; CHECK-NEXT: .long ([[test2_before_f1]]-[[test2_begin]])+1
 ;                   ^ offset of start of clause
 ; CHECK-NEXT: .long ([[test2_after_f1]]-[[test2_begin]])+1
 ;                   ^ offset of end of clause
 ; CHECK-NEXT: .long [[test2_catch2]]-[[test2_begin]]
 ;                   ^ offset of start of handler
-; CHECK-NEXT: .long [[test2_end]]-[[test2_begin]]
+; CHECK-NEXT: .long [[test2_fault]]-[[test2_begin]]
 ;                   ^ offset of end of handler
 ; CHECK-NEXT: .long 2
 ;                   ^ type token of catch (from catchpad)
 ; Clause 3: calls f(2) is guarded by catch1
 ; CHECK-NEXT: .long 0
 ;                   ^ flags (0 => catch handler)
 ; CHECK-NEXT: .long ([[test2_before_f2]]-[[test2_begin]])+1
 ;                   ^ offset of start of clause

 ; CHECK-NEXT: .long 8
 ;                   ^ flags (0 => catch handler | 8 => duplicate)
 ; CHECK-NEXT: .long ([[test2_before_f2]]-[[test2_begin]])+1
 ;                   ^ offset of start of clause
 ; CHECK-NEXT: .long ([[test2_after_f2]]-[[test2_begin]])+1
 ;                   ^ offset of end of clause
 ; CHECK-NEXT: .long [[test2_catch2]]-[[test2_begin]]
 ;                   ^ offset of start of handler
-; CHECK-NEXT: .long [[test2_end]]-[[test2_begin]]
+; CHECK-NEXT: .long [[test2_fault]]-[[test2_begin]]
 ;                   ^ offset of end of handler
 ; CHECK-NEXT: .long 2
 ;                   ^ type token of catch (from catchpad)
 
 ; Test with several cleanups that need to infer their unwind dests from each
 ; other, the inner one needing to make the inference from an invoke, ignoring
 ; not-really-unwinding calls/unwind-to-caller catchswitches, as well as some
 ; internal invokes/catchswitches