[NewPM][SimpleLoopUnswitch] Add option to not trivially unswitch

[aeubanks]

Summary

To help with debugging non-trivial unswitching issues.

Don't care about the legacy pass, nobody is using it.

If a pass's string params are empty (e.g. "simple-loop-unswitch"), don't
default to the empty constructor for the pass params. We should still
let the parser take care of it in case the parser has its own defaults.

[asbirlea]

This will be useful for debugging. Thank you.

llvm/include/llvm/Transforms/Scalar/SimpleLoopUnswitch.h

 /// additionally do non-trivial, full unswitching for branches and switches, and
 /// will do non-trivial, partial unswitching for branches.
 ///
 /// Because partial unswitching of switches is extremely unlikely to be possible
 /// in practice and significantly complicates the implementation, this pass does
 /// not currently implement that in any mode.
 class SimpleLoopUnswitchPass : public PassInfoMixin<SimpleLoopUnswitchPass> {
   bool NonTrivial;
+  bool Trivial;
 
 public:
-  SimpleLoopUnswitchPass(bool NonTrivial = false) : NonTrivial(NonTrivial) {}
+  SimpleLoopUnswitchPass(bool NonTrivial = false, bool Trivial = true)
+      : NonTrivial(NonTrivial), Trivial(Trivial) {}
 
   PreservedAnalyses run(Loop &L, LoopAnalysisManager &AM,
                         LoopStandardAnalysisResults &AR, LPMUpdater &U);
 };
 
 /// Create the legacy pass object for the simple loop unswitcher.
 ///
 /// See the documentaion for `SimpleLoopUnswitchPass` for details.
 Pass *createSimpleLoopUnswitchLegacyPass(bool NonTrivial = false);
 
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_SCALAR_SIMPLELOOPUNSWITCH_H

llvm/lib/Passes/PassBuilder.cpp

auto parsePassParameters(ParametersParseCallableT &&Parser, StringRef Name,
                          StringRef PassName) -> decltype(Parser(StringRef{})) {
   using ParametersT = typename decltype(Parser(StringRef{}))::value_type;
 
   StringRef Params = Name;
   if (!Params.consume_front(PassName)) {
     assert(false &&
            "unable to strip pass name from parametrized pass specification");
   }
-  if (Params.empty())
-    return ParametersT{};
-  if (!Params.consume_front("<") || !Params.consume_back(">")) {
+  if (!Params.empty() &&
+      (!Params.consume_front("<") || !Params.consume_back(">"))) {
     assert(false && "invalid format for parametrized pass name");
   }
 
   Expected<ParametersT> Result = Parser(Params);
   assert((Result || Result.template errorIsA<StringError>()) &&
          "Pass parameter parser can only return StringErrors.");
   return Result;
 }
if (ParamName == "interleave-forced-only") {
       return make_error<StringError>(
           formatv("invalid LoopVectorize parameter '{0}' ", ParamName).str(),
           inconvertibleErrorCode());
     }
   }
   return Opts;
 }
 
-Expected<bool> parseLoopUnswitchOptions(StringRef Params) {
-  bool Result = false;
+Expected<std::pair<bool, bool>> parseLoopUnswitchOptions(StringRef Params) {
+  std::pair<bool, bool> Result = {false, true};
   while (!Params.empty()) {
     StringRef ParamName;
     std::tie(ParamName, Params) = Params.split(';');
 
     bool Enable = !ParamName.consume_front("no-");
     if (ParamName == "nontrivial") {
-      Result = Enable;
+      Result.first = Enable;
+    } else if (ParamName == "trivial") {
+      Result.second = Enable;
     } else {
       return make_error<StringError>(
           formatv("invalid LoopUnswitch pass parameter '{0}' ", ParamName)
               .str(),
           inconvertibleErrorCode());
     }
   }
   return Result;

llvm/lib/Passes/PassRegistry.def

 LOOP_PASS("loop-versioning-licm", LoopVersioningLICMPass())
 #undef LOOP_PASS
 
 #ifndef LOOP_PASS_WITH_PARAMS
 #define LOOP_PASS_WITH_PARAMS(NAME, CLASS, CREATE_PASS, PARSER, PARAMS)
 #endif
 LOOP_PASS_WITH_PARAMS("simple-loop-unswitch",
                       "SimpleLoopUnswitchPass",
-                      [](bool NonTrivial) {
-                        return SimpleLoopUnswitchPass(NonTrivial);
+                      [](std::pair<bool, bool> Params) {
+                        return SimpleLoopUnswitchPass(Params.first, Params.second);
                       },
                       parseLoopUnswitchOptions,
-                      "no-nontrivial;nontrivial")
+                      "nontrivial;no-nontrivial;trivial;no-trivial")
 #undef LOOP_PASS_WITH_PARAMS

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

 /// The `UnswitchCB` callback provided will be run after unswitching is
 /// complete, with the first parameter set to `true` if the provided loop
 /// remains a loop, and a list of new sibling loops created.
 ///
 /// If `SE` is non-null, we will update that analysis based on the unswitching
 /// done.
 static bool
 unswitchLoop(Loop &L, DominatorTree &DT, LoopInfo &LI, AssumptionCache &AC,
-             AAResults &AA, TargetTransformInfo &TTI, bool NonTrivial,
+             AAResults &AA, TargetTransformInfo &TTI, bool Trivial,
+             bool NonTrivial,
              function_ref<void(bool, bool, ArrayRef<Loop *>)> UnswitchCB,
              ScalarEvolution *SE, MemorySSAUpdater *MSSAU) {
   assert(L.isRecursivelyLCSSAForm(DT, LI) &&
          "Loops must be in LCSSA form before unswitching.");
 
   // Must be in loop simplified form: we need a preheader and dedicated exits.
   if (!L.isLoopSimplifyForm())
     return false;
 
   // Try trivial unswitch first before loop over other basic blocks in the loop.
-  if (unswitchAllTrivialConditions(L, DT, LI, SE, MSSAU)) {
+  if (Trivial && unswitchAllTrivialConditions(L, DT, LI, SE, MSSAU)) {
     // If we unswitched successfully we will want to clean up the loop before
     // processing it further so just mark it as unswitched and return.
     UnswitchCB(/*CurrentLoopValid*/ true, false, {});
     return true;
   }
 
   // Check whether we should continue with non-trivial conditions.
   // EnableNonTrivialUnswitch: Global variable that forces non-trivial
PreservedAnalyses SimpleLoopUnswitchPass::run(Loop &L, LoopAnalysisManager &AM,
   };
 
   Optional<MemorySSAUpdater> MSSAU;
   if (AR.MSSA) {
     MSSAU = MemorySSAUpdater(AR.MSSA);
     if (VerifyMemorySSA)
       AR.MSSA->verifyMemorySSA();
   }
-  if (!unswitchLoop(L, AR.DT, AR.LI, AR.AC, AR.AA, AR.TTI, NonTrivial,
+  if (!unswitchLoop(L, AR.DT, AR.LI, AR.AC, AR.AA, AR.TTI, Trivial, NonTrivial,
                     UnswitchCB, &AR.SE,
                     MSSAU.hasValue() ? MSSAU.getPointer() : nullptr))
     return PreservedAnalyses::all();
 
   if (AR.MSSA && VerifyMemorySSA)
     AR.MSSA->verifyMemorySSA();
 
   // Historically this pass has had issues with the dominator tree so verify it
auto UnswitchCB = [&L, &LPM](bool CurrentLoopValid, bool PartiallyInvariant,
     } else
       LPM.markLoopAsDeleted(*L);
   };
 
   if (MSSA && VerifyMemorySSA)
     MSSA->verifyMemorySSA();
 
   bool Changed =
-      unswitchLoop(*L, DT, LI, AC, AA, TTI, NonTrivial, UnswitchCB, SE,
+      unswitchLoop(*L, DT, LI, AC, AA, TTI, true, NonTrivial, UnswitchCB, SE,
                    MSSAU.hasValue() ? MSSAU.getPointer() : nullptr);
 
   if (MSSA && VerifyMemorySSA)
     MSSA->verifyMemorySSA();
 
   // Historically this pass has had issues with the dominator tree so verify it
   // in asserts builds.
   assert(DT.verify(DominatorTree::VerificationLevel::Fast));

llvm/test/Other/print-passes.ll

 ; CHECK: loop-unroll<O0;O1;O2;O3;full-unroll-max=N;no-partial;partial;no-peeling;peeling;no-profile-peeling;profile-peeling;no-runtime;runtime;no-upperbound;upperbound>
 ; CHECK: Function analyses:
 ; CHECK: no-op-function
 ; CHECK: Function alias analyses:
 ; CHECK: basic-aa
 ; CHECK: Loop passes:
 ; CHECK: no-op-loop
 ; CHECK: Loop passes with params:
-; CHECK: simple-loop-unswitch<no-nontrivial;nontrivial>
+; CHECK: simple-loop-unswitch<nontrivial;no-nontrivial;trivial;no-trivial>
 ; CHECK: Loop analyses:
 ; CHECK: no-op-loop

llvm/test/Transforms/SimpleLoopUnswitch/options.ll

+; RUN: opt -passes='simple-loop-unswitch<no-trivial>' -S < %s | FileCheck %s --check-prefix=NOTRIVIAL
+; RUN: opt -passes='simple-loop-unswitch' -S < %s | FileCheck %s --check-prefix=TRIVIAL
+; RUN: opt -passes='simple-loop-unswitch<trivial>' -S < %s | FileCheck %s --check-prefix=TRIVIAL
+
+declare void @some_func() noreturn
+
+; NOTRIVIAL-NOT: split
+; TRIVIAL: split
+define i32 @test1(i32* %var, i1 %cond1, i1 %cond2) {
+entry:
+  br label %loop_begin
+
+loop_begin:
+  br i1 %cond1, label %continue, label %loop_exit  ; first trivial condition
+
+continue:
+  %var_val = load i32, i32* %var
+  br i1 %cond2, label %do_something, label %loop_exit  ; second trivial condition
+
+do_something:
+  call void @some_func() noreturn nounwind
+  br label %loop_begin
+
+loop_exit:
+  ret i32 0
+}
 No newline at end of file