[llvm] r317444 - [PassManager, SimplifyCFG] Revert r316908 and r316869.

David L. Jones via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 5 16:32:02 PST 2017


Author: dlj
Date: Sun Nov  5 16:32:01 2017
New Revision: 317444

URL: http://llvm.org/viewvc/llvm-project?rev=317444&view=rev
Log:
[PassManager, SimplifyCFG] Revert r316908 and r316869.

These cause Clang to crash with a segfault. See PR35210 for details.


Modified:
    llvm/trunk/include/llvm/Transforms/Scalar/SimplifyCFG.h
    llvm/trunk/lib/Passes/PassBuilder.cpp
    llvm/trunk/test/Transforms/PhaseOrdering/simplifycfg-options.ll

Modified: llvm/trunk/include/llvm/Transforms/Scalar/SimplifyCFG.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Scalar/SimplifyCFG.h?rev=317444&r1=317443&r2=317444&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Scalar/SimplifyCFG.h (original)
+++ llvm/trunk/include/llvm/Transforms/Scalar/SimplifyCFG.h Sun Nov  5 16:32:01 2017
@@ -31,16 +31,16 @@ class SimplifyCFGPass : public PassInfoM
   SimplifyCFGOptions Options;
 
 public:
-  /// The default constructor sets the pass options to create canonical IR,
-  /// rather than optimal IR. That is, by default we bypass transformations that
-  /// are likely to improve performance but make analysis for other passes more
-  /// difficult.
+  /// The default constructor sets the pass options to create optimal IR,
+  /// rather than canonical IR. That is, by default we do transformations that
+  /// are likely to improve performance but make analysis more difficult.
+  /// FIXME: This is inverted from what most instantiations of the pass should
+  /// be.
   SimplifyCFGPass()
       : SimplifyCFGPass(SimplifyCFGOptions()
-                            .forwardSwitchCondToPhi(false)
-                            .convertSwitchToLookupTable(false)
-                            .needCanonicalLoops(true)) {}
-
+                            .forwardSwitchCondToPhi(true)
+                            .convertSwitchToLookupTable(true)
+                            .needCanonicalLoops(false)) {}
 
   /// Construct a pass with optional optimizations.
   SimplifyCFGPass(const SimplifyCFGOptions &PassOptions);

Modified: llvm/trunk/lib/Passes/PassBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Passes/PassBuilder.cpp?rev=317444&r1=317443&r2=317444&view=diff
==============================================================================
--- llvm/trunk/lib/Passes/PassBuilder.cpp (original)
+++ llvm/trunk/lib/Passes/PassBuilder.cpp Sun Nov  5 16:32:01 2017
@@ -755,13 +755,8 @@ PassBuilder::buildModuleOptimizationPipe
   // Optimize parallel scalar instruction chains into SIMD instructions.
   OptimizePM.addPass(SLPVectorizerPass());
 
-  // Cleanup after all of the vectorizers. Simplification passes like CVP and
-  // GVN, loop transforms, and others have already run, so it's now better to
-  // convert to more optimized IR using more aggressive simplify CFG options.
-  OptimizePM.addPass(SimplifyCFGPass(SimplifyCFGOptions().
-                                         forwardSwitchCondToPhi(true).
-                                         convertSwitchToLookupTable(true).
-                                         needCanonicalLoops(false)));
+  // Cleanup after all of the vectorizers.
+  OptimizePM.addPass(SimplifyCFGPass());
   OptimizePM.addPass(InstCombinePass());
 
   // Unroll small loops to hide loop backedge latency and saturate any parallel

Modified: llvm/trunk/test/Transforms/PhaseOrdering/simplifycfg-options.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PhaseOrdering/simplifycfg-options.ll?rev=317444&r1=317443&r2=317444&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/PhaseOrdering/simplifycfg-options.ll (original)
+++ llvm/trunk/test/Transforms/PhaseOrdering/simplifycfg-options.ll Sun Nov  5 16:32:01 2017
@@ -1,36 +1,63 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -O1 -S < %s                    | FileCheck %s --check-prefix=ALL --check-prefix=OLDPM
-; RUN: opt -passes='default<O1>' -S < %s  | FileCheck %s --check-prefix=ALL --check-prefix=NEWPM
-
-declare void @foo()
+; RUN: opt -O1 -S < %s                    | FileCheck %s --check-prefix=OLDPM
+; RUN: opt -passes='default<O1>' -S < %s  | FileCheck %s --check-prefix=NEWPM
 
 ; Don't simplify unconditional branches from empty blocks in simplifyCFG
 ; until late in the pipeline because it can destroy canonical loop structure.
 
+; FIXME: The new pass manager is not limiting simplifycfg at any point in the pipeline,
+; so it performs a transformation before loop optimizations that is avoided in the old PM.
+
 define i1 @PR33605(i32 %a, i32 %b, i32* %c) {
-; ALL-LABEL: @PR33605(
-; ALL-NEXT:  for.body:
-; ALL-NEXT:    [[OR:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
-; ALL-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32, i32* [[C:%.*]], i64 1
-; ALL-NEXT:    [[TMP0:%.*]] = load i32, i32* [[ARRAYIDX]], align 4
-; ALL-NEXT:    [[CMP:%.*]] = icmp eq i32 [[OR]], [[TMP0]]
-; ALL-NEXT:    br i1 [[CMP]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
-; ALL:       if.then:
-; ALL-NEXT:    store i32 [[OR]], i32* [[ARRAYIDX]], align 4
-; ALL-NEXT:    tail call void @foo()
-; ALL-NEXT:    br label [[IF_END]]
-; ALL:       if.end:
-; ALL-NEXT:    [[CHANGED_1_OFF0:%.*]] = phi i1 [ true, [[IF_THEN]] ], [ false, [[FOR_BODY:%.*]] ]
-; ALL-NEXT:    [[TMP1:%.*]] = load i32, i32* [[C]], align 4
-; ALL-NEXT:    [[CMP_1:%.*]] = icmp eq i32 [[OR]], [[TMP1]]
-; ALL-NEXT:    br i1 [[CMP_1]], label [[IF_END_1:%.*]], label [[IF_THEN_1:%.*]]
-; ALL:       if.then.1:
-; ALL-NEXT:    store i32 [[OR]], i32* [[C]], align 4
-; ALL-NEXT:    tail call void @foo()
-; ALL-NEXT:    br label [[IF_END_1]]
-; ALL:       if.end.1:
-; ALL-NEXT:    [[CHANGED_1_OFF0_1:%.*]] = phi i1 [ true, [[IF_THEN_1]] ], [ [[CHANGED_1_OFF0]], [[IF_END]] ]
-; ALL-NEXT:    ret i1 [[CHANGED_1_OFF0_1]]
+; OLDPM-LABEL: @PR33605(
+; OLDPM-NEXT:  for.body:
+; OLDPM-NEXT:    [[OR:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
+; OLDPM-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32, i32* [[C:%.*]], i64 1
+; OLDPM-NEXT:    [[TMP0:%.*]] = load i32, i32* [[ARRAYIDX]], align 4
+; OLDPM-NEXT:    [[CMP:%.*]] = icmp eq i32 [[OR]], [[TMP0]]
+; OLDPM-NEXT:    br i1 [[CMP]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; OLDPM:       if.then:
+; OLDPM-NEXT:    store i32 [[OR]], i32* [[ARRAYIDX]], align 4
+; OLDPM-NEXT:    tail call void @foo()
+; OLDPM-NEXT:    br label [[IF_END]]
+; OLDPM:       if.end:
+; OLDPM-NEXT:    [[CHANGED_1_OFF0:%.*]] = phi i1 [ true, [[IF_THEN]] ], [ false, [[FOR_BODY:%.*]] ]
+; OLDPM-NEXT:    [[TMP1:%.*]] = load i32, i32* [[C]], align 4
+; OLDPM-NEXT:    [[CMP_1:%.*]] = icmp eq i32 [[OR]], [[TMP1]]
+; OLDPM-NEXT:    br i1 [[CMP_1]], label [[IF_END_1:%.*]], label [[IF_THEN_1:%.*]]
+; OLDPM:       if.then.1:
+; OLDPM-NEXT:    store i32 [[OR]], i32* [[C]], align 4
+; OLDPM-NEXT:    tail call void @foo()
+; OLDPM-NEXT:    br label [[IF_END_1]]
+; OLDPM:       if.end.1:
+; OLDPM-NEXT:    [[CHANGED_1_OFF0_1:%.*]] = phi i1 [ true, [[IF_THEN_1]] ], [ [[CHANGED_1_OFF0]], [[IF_END]] ]
+; OLDPM-NEXT:    ret i1 [[CHANGED_1_OFF0_1]]
+;
+; NEWPM-LABEL: @PR33605(
+; NEWPM-NEXT:  entry:
+; NEWPM-NEXT:    [[OR:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
+; NEWPM-NEXT:    br label [[FOR_COND_OUTER:%.*]]
+; NEWPM:       for.cond.outer:
+; NEWPM-NEXT:    [[I_0_PH:%.*]] = phi i32 [ [[DEC:%.*]], [[IF_THEN:%.*]] ], [ 2, [[ENTRY:%.*]] ]
+; NEWPM-NEXT:    [[CHANGED_0_OFF0_PH:%.*]] = phi i1 [ true, [[IF_THEN]] ], [ false, [[ENTRY]] ]
+; NEWPM-NEXT:    br label [[FOR_COND:%.*]]
+; NEWPM:       for.cond:
+; NEWPM-NEXT:    [[I_0:%.*]] = phi i32 [ [[DEC]], [[FOR_BODY:%.*]] ], [ [[I_0_PH]], [[FOR_COND_OUTER]] ]
+; NEWPM-NEXT:    [[DEC]] = add nsw i32 [[I_0]], -1
+; NEWPM-NEXT:    [[TOBOOL:%.*]] = icmp eq i32 [[I_0]], 0
+; NEWPM-NEXT:    br i1 [[TOBOOL]], label [[FOR_COND_CLEANUP:%.*]], label [[FOR_BODY]]
+; NEWPM:       for.cond.cleanup:
+; NEWPM-NEXT:    ret i1 [[CHANGED_0_OFF0_PH]]
+; NEWPM:       for.body:
+; NEWPM-NEXT:    [[IDXPROM:%.*]] = sext i32 [[DEC]] to i64
+; NEWPM-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32, i32* [[C:%.*]], i64 [[IDXPROM]]
+; NEWPM-NEXT:    [[TMP0:%.*]] = load i32, i32* [[ARRAYIDX]], align 4
+; NEWPM-NEXT:    [[CMP:%.*]] = icmp eq i32 [[OR]], [[TMP0]]
+; NEWPM-NEXT:    br i1 [[CMP]], label [[FOR_COND]], label [[IF_THEN]]
+; NEWPM:       if.then:
+; NEWPM-NEXT:    store i32 [[OR]], i32* [[ARRAYIDX]], align 4
+; NEWPM-NEXT:    tail call void @foo()
+; NEWPM-NEXT:    br label [[FOR_COND_OUTER]]
 ;
 entry:
   br label %for.cond
@@ -64,43 +91,5 @@ if.end:
   br label %for.cond
 }
 
-; PR34603 - https://bugs.llvm.org/show_bug.cgi?id=34603
-; We should have a select of doubles, not a select of double pointers.
-; SimplifyCFG should not flatten this before early-cse has a chance to eliminate redundant ops.
-
-define double @max_of_loads(double* %x, double* %y, i64 %i) {
-; ALL-LABEL: @max_of_loads(
-; ALL-NEXT:  entry:
-; ALL-NEXT:    [[XI_PTR:%.*]] = getelementptr double, double* [[X:%.*]], i64 [[I:%.*]]
-; ALL-NEXT:    [[YI_PTR:%.*]] = getelementptr double, double* [[Y:%.*]], i64 [[I]]
-; ALL-NEXT:    [[XI:%.*]] = load double, double* [[XI_PTR]], align 8
-; ALL-NEXT:    [[YI:%.*]] = load double, double* [[YI_PTR]], align 8
-; ALL-NEXT:    [[CMP:%.*]] = fcmp ogt double [[XI]], [[YI]]
-; ALL-NEXT:    [[Y_SINK:%.*]] = select i1 [[CMP]], double* [[X]], double* [[Y]]
-; ALL-NEXT:    [[YI_PTR_AGAIN:%.*]] = getelementptr double, double* [[Y_SINK]], i64 [[I]]
-; ALL-NEXT:    [[YI_AGAIN:%.*]] = load double, double* [[YI_PTR_AGAIN]], align 8
-; ALL-NEXT:    ret double [[YI_AGAIN]]
-;
-entry:
-  %xi_ptr = getelementptr double, double* %x, i64 %i
-  %yi_ptr = getelementptr double, double* %y, i64 %i
-  %xi = load double, double* %xi_ptr
-  %yi = load double, double* %yi_ptr
-  %cmp = fcmp ogt double %xi, %yi
-  br i1 %cmp, label %if, label %else
-
-if:
-  %xi_ptr_again = getelementptr double, double* %x, i64 %i
-  %xi_again = load double, double* %xi_ptr_again
-  br label %end
-
-else:
-  %yi_ptr_again = getelementptr double, double* %y, i64 %i
-  %yi_again = load double, double* %yi_ptr_again
-  br label %end
-
-end:
-  %max = phi double [ %xi_again,  %if ], [ %yi_again, %else ]
-  ret double %max
-}
+declare void @foo()
 




More information about the llvm-commits mailing list