[nfc][BoundsChecking] Refactor BoundsCheckingOptions

[vitalybuka]

Remove ReportingMode and ReportingOpts.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Vitaly Buka (vitalybuka)

Changes

Remove ReportingMode and ReportingOpts.

---

Full diff: https://github.com/llvm/llvm-project/pull/122346.diff

4 Files Affected:

• (modified) clang/lib/CodeGen/BackendUtil.cpp (+15-20)
• (modified) llvm/include/llvm/Transforms/Instrumentation/BoundsChecking.h (+9-12)
• (modified) llvm/lib/Passes/PassBuilder.cpp (+18-7)
• (modified) llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp (+35-64)

diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 2dbab785658aa4..f350accfc530bf 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1025,26 +1025,21 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
     // Register callbacks to schedule sanitizer passes at the appropriate part
     // of the pipeline.
     if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds))
-      PB.registerScalarOptimizerLateEPCallback(
-          [this](FunctionPassManager &FPM, OptimizationLevel Level) {
-            BoundsCheckingPass::ReportingMode Mode;
-            bool Merge = CodeGenOpts.SanitizeMergeHandlers.has(
-                SanitizerKind::LocalBounds);
-
-            if (CodeGenOpts.SanitizeTrap.has(SanitizerKind::LocalBounds)) {
-              Mode = BoundsCheckingPass::ReportingMode::Trap;
-            } else if (CodeGenOpts.SanitizeMinimalRuntime) {
-              Mode = CodeGenOpts.SanitizeRecover.has(SanitizerKind::LocalBounds)
-                         ? BoundsCheckingPass::ReportingMode::MinRuntime
-                         : BoundsCheckingPass::ReportingMode::MinRuntimeAbort;
-            } else {
-              Mode = CodeGenOpts.SanitizeRecover.has(SanitizerKind::LocalBounds)
-                         ? BoundsCheckingPass::ReportingMode::FullRuntime
-                         : BoundsCheckingPass::ReportingMode::FullRuntimeAbort;
-            }
-            BoundsCheckingPass::BoundsCheckingOptions Options(Mode, Merge);
-            FPM.addPass(BoundsCheckingPass(Options));
-          });
+      PB.registerScalarOptimizerLateEPCallback([this](FunctionPassManager &FPM,
+                                                      OptimizationLevel Level) {
+        BoundsCheckingPass::BoundsCheckingOptions Options;
+        Options.Merge =
+            CodeGenOpts.SanitizeMergeHandlers.has(SanitizerKind::LocalBounds);
+        if (!CodeGenOpts.SanitizeTrap.has(SanitizerKind::LocalBounds)) {
+          Options.Rt = {
+              /*MinRuntime=*/static_cast<bool>(
+                  CodeGenOpts.SanitizeMinimalRuntime),
+              /*MayReturn=*/
+              CodeGenOpts.SanitizeRecover.has(SanitizerKind::LocalBounds),
+          };
+        }
+        FPM.addPass(BoundsCheckingPass(Options));
+      });
 
     // Don't add sanitizers if we are here from ThinLTO PostLink. That already
     // done on PreLink stage.
diff --git a/llvm/include/llvm/Transforms/Instrumentation/BoundsChecking.h b/llvm/include/llvm/Transforms/Instrumentation/BoundsChecking.h
index ee71aa64f85eed..9c0506428bd625 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/BoundsChecking.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/BoundsChecking.h
@@ -10,6 +10,7 @@
 #define LLVM_TRANSFORMS_INSTRUMENTATION_BOUNDSCHECKING_H
 
 #include "llvm/IR/PassManager.h"
+#include <optional>
 
 namespace llvm {
 class Function;
@@ -19,19 +20,15 @@ class Function;
 class BoundsCheckingPass : public PassInfoMixin<BoundsCheckingPass> {
 
 public:
-  enum class ReportingMode {
-    Trap,
-    MinRuntime,
-    MinRuntimeAbort,
-    FullRuntime,
-    FullRuntimeAbort,
-  };
-
   struct BoundsCheckingOptions {
-    BoundsCheckingOptions(ReportingMode Mode, bool Merge);
-
-    ReportingMode Mode;
-    bool Merge;
+    struct Runtime {
+      Runtime(bool MinRuntime, bool MayReturn)
+          : MinRuntime(MinRuntime), MayReturn(MayReturn) {}
+      bool MinRuntime;
+      bool MayReturn;
+    };
+    std::optional<Runtime> Rt; // Trap if empty.
+    bool Merge = false;
   };
 
   BoundsCheckingPass(BoundsCheckingOptions Options) : Options(Options) {}
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 30b8d7c9499488..b75387ac556e39 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1286,21 +1286,32 @@ parseRegAllocFastPassOptions(PassBuilder &PB, StringRef Params) {
 
 Expected<BoundsCheckingPass::BoundsCheckingOptions>
 parseBoundsCheckingOptions(StringRef Params) {
-  BoundsCheckingPass::BoundsCheckingOptions Options(
-      BoundsCheckingPass::ReportingMode::Trap, false);
+  BoundsCheckingPass::BoundsCheckingOptions Options;
   while (!Params.empty()) {
     StringRef ParamName;
     std::tie(ParamName, Params) = Params.split(';');
     if (ParamName == "trap") {
-      Options.Mode = BoundsCheckingPass::ReportingMode::Trap;
+      Options.Rt = std::nullopt;
     } else if (ParamName == "rt") {
-      Options.Mode = BoundsCheckingPass::ReportingMode::FullRuntime;
+      Options.Rt = {
+          /*MinRuntime=*/false,
+          /*MayReturn=*/true,
+      };
     } else if (ParamName == "rt-abort") {
-      Options.Mode = BoundsCheckingPass::ReportingMode::FullRuntimeAbort;
+      Options.Rt = {
+          /*MinRuntime=*/false,
+          /*MayReturn=*/false,
+      };
     } else if (ParamName == "min-rt") {
-      Options.Mode = BoundsCheckingPass::ReportingMode::MinRuntime;
+      Options.Rt = {
+          /*MinRuntime=*/true,
+          /*MayReturn=*/true,
+      };
     } else if (ParamName == "min-rt-abort") {
-      Options.Mode = BoundsCheckingPass::ReportingMode::MinRuntimeAbort;
+      Options.Rt = {
+          /*MinRuntime=*/true,
+          /*MayReturn=*/false,
+      };
     } else if (ParamName == "merge") {
       Options.Merge = true;
     } else {
diff --git a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
index 41e50385812460..3d2a2663230c06 100644
--- a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
+++ b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
@@ -43,10 +43,6 @@ STATISTIC(ChecksUnable, "Bounds checks unable to add");
 
 using BuilderTy = IRBuilder<TargetFolder>;
 
-BoundsCheckingPass::BoundsCheckingOptions::BoundsCheckingOptions(
-    ReportingMode Mode, bool Merge)
-    : Mode(Mode), Merge(Merge) {}
-
 /// Gets the conditions under which memory accessing instructions will overflow.
 ///
 /// \p Ptr is the pointer that will be read/written, and \p InstVal is either
@@ -166,42 +162,19 @@ static void insertBoundsCheck(Value *Or, BuilderTy &IRB, GetTrapBBT GetTrapBB) {
   BranchInst::Create(TrapBB, Cont, Or, OldBB);
 }
 
-struct ReportingOpts {
-  bool MayReturn = false;
-  bool UseTrap = false;
-  bool MinRuntime = false;
-  bool MayMerge = true;
-  StringRef Name;
-
-  ReportingOpts(BoundsCheckingPass::ReportingMode Mode, bool Merge) {
-    switch (Mode) {
-    case BoundsCheckingPass::ReportingMode::Trap:
-      UseTrap = true;
-      break;
-    case BoundsCheckingPass::ReportingMode::MinRuntime:
-      Name = "__ubsan_handle_local_out_of_bounds_minimal";
-      MinRuntime = true;
-      MayReturn = true;
-      break;
-    case BoundsCheckingPass::ReportingMode::MinRuntimeAbort:
-      Name = "__ubsan_handle_local_out_of_bounds_minimal_abort";
-      MinRuntime = true;
-      break;
-    case BoundsCheckingPass::ReportingMode::FullRuntime:
-      Name = "__ubsan_handle_local_out_of_bounds";
-      MayReturn = true;
-      break;
-    case BoundsCheckingPass::ReportingMode::FullRuntimeAbort:
-      Name = "__ubsan_handle_local_out_of_bounds_abort";
-      break;
-    }
-
-    MayMerge = Merge;
-  }
-};
+static std::string getRuntimeCallName(
+    const BoundsCheckingPass::BoundsCheckingOptions::Runtime &Opts) {
+  std::string Name = "__ubsan_handle_local_out_of_bounds";
+  if (Opts.MinRuntime)
+    Name += "_minimal";
+  if (!Opts.MayReturn)
+    Name += "_abort";
+  return Name;
+}
 
-static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI,
-                              ScalarEvolution &SE, const ReportingOpts &Opts) {
+static bool
+addBoundsChecking(Function &F, TargetLibraryInfo &TLI, ScalarEvolution &SE,
+                  const BoundsCheckingPass::BoundsCheckingOptions &Opts) {
   if (F.hasFnAttribute(Attribute::NoSanitizeBounds))
     return false;
 
@@ -239,11 +212,16 @@ static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI,
       TrapInfo.push_back(std::make_pair(&I, Or));
   }
 
+  std::string Name;
+  if (Opts.Rt)
+    Name = getRuntimeCallName(*Opts.Rt);
+
   // Create a trapping basic block on demand using a callback. Depending on
   // flags, this will either create a single block for the entire function or
   // will create a fresh block every time it is called.
   BasicBlock *ReuseTrapBB = nullptr;
-  auto GetTrapBB = [&ReuseTrapBB, &Opts](BuilderTy &IRB, BasicBlock *Cont) {
+  auto GetTrapBB = [&ReuseTrapBB, &Opts, &Name](BuilderTy &IRB,
+                                                BasicBlock *Cont) {
     Function *Fn = IRB.GetInsertBlock()->getParent();
     auto DebugLoc = IRB.getCurrentDebugLocation();
     IRBuilder<>::InsertPointGuard Guard(IRB);
@@ -257,23 +235,24 @@ static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI,
     BasicBlock *TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn);
     IRB.SetInsertPoint(TrapBB);
 
-    bool DebugTrapBB = !Opts.MayMerge;
-    CallInst *TrapCall = Opts.UseTrap
-                             ? InsertTrap(IRB, DebugTrapBB)
-                             : InsertCall(IRB, Opts.MayReturn, Opts.Name);
+    bool DebugTrapBB = !Opts.Merge;
+    CallInst *TrapCall = Opts.Rt ? InsertCall(IRB, Opts.Rt->MayReturn, Name)
+                                 : InsertTrap(IRB, DebugTrapBB);
     if (DebugTrapBB)
       TrapCall->addFnAttr(llvm::Attribute::NoMerge);
 
     TrapCall->setDoesNotThrow();
     TrapCall->setDebugLoc(DebugLoc);
-    if (Opts.MayReturn) {
+
+    bool MayReturn = Opts.Rt && Opts.Rt->MayReturn;
+    if (MayReturn) {
       IRB.CreateBr(Cont);
     } else {
       TrapCall->setDoesNotReturn();
       IRB.CreateUnreachable();
     }
 
-    if (!Opts.MayReturn && SingleTrapBB && !DebugTrapBB)
+    if (!MayReturn && SingleTrapBB && !DebugTrapBB)
       ReuseTrapBB = TrapBB;
 
     return TrapBB;
@@ -292,8 +271,7 @@ PreservedAnalyses BoundsCheckingPass::run(Function &F, FunctionAnalysisManager &
   auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
   auto &SE = AM.getResult<ScalarEvolutionAnalysis>(F);
 
-  if (!addBoundsChecking(F, TLI, SE,
-                         ReportingOpts(Options.Mode, Options.Merge)))
+  if (!addBoundsChecking(F, TLI, SE, Options))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
@@ -303,22 +281,15 @@ void BoundsCheckingPass::printPipeline(
     raw_ostream &OS, function_ref<StringRef(StringRef)> MapClassName2PassName) {
   static_cast<PassInfoMixin<BoundsCheckingPass> *>(this)->printPipeline(
       OS, MapClassName2PassName);
-  switch (Options.Mode) {
-  case ReportingMode::Trap:
-    OS << "<trap";
-    break;
-  case ReportingMode::MinRuntime:
-    OS << "<min-rt";
-    break;
-  case ReportingMode::MinRuntimeAbort:
-    OS << "<min-rt-abort";
-    break;
-  case ReportingMode::FullRuntime:
-    OS << "<rt";
-    break;
-  case ReportingMode::FullRuntimeAbort:
-    OS << "<rt-abort";
-    break;
+  OS << "<";
+  if (Options.Rt) {
+    if (Options.Rt->MinRuntime)
+      OS << "min-";
+    OS << "rt";
+    if (!Options.Rt->MayReturn)
+      OS << "-abort";
+  } else {
+    OS << "trap";
   }
   if (Options.Merge)
     OS << ";merge";