Address review feedback

Author: mmha

clang/include/clang/CIR/Dialect/IR/CIRDialect.td

@@ -27,6 +27,11 @@ def CIR_Dialect : Dialect {
   let useDefaultAttributePrinterParser = 0;
   let useDefaultTypePrinterParser = 0;
 
+  // Enable constant materialization for the CIR dialect. This generates a
+  // declaration for the cir::CIRDialect::materializeConstant function. This
+  // hook is necessary for canonicalization to properly handle attributes
+  // returned by fold methods, allowing them to be materialized as constant
+  // operations in the IR.
   let hasConstantMaterializer = 1;
 
   let extraClassDeclaration = [{

clang/include/clang/CIR/Dialect/Passes.td

@@ -32,12 +32,17 @@ def CIRCanonicalize : Pass<"cir-canonicalize"> {
 def CIRSimplify : Pass<"cir-simplify"> {
   let summary = "Performs CIR simplification and code optimization";
   let description = [{
-    The pass performs code simplification and optimization on CIR.
-
-    Unlike the `cir-canonicalize` pass, this pass contains more aggresive code
-    transformations that could significantly affect CIR-to-source fidelity.
-    Example transformations performed in this pass include ternary folding,
-    code hoisting, etc.
+    The pass performs semantics-preserving code simplifications and optimizations
+    on CIR while maintaining strict program correctness. 
+    
+    Unlike the `cir-canonicalize` pass, these transformations may reduce the IR's
+    structural similarity to the original source code as a trade-off for improved
+    code quality. This can affect debugging fidelity by altering intermediate
+    representations of folded expressions, hoisted operations, and other 
+    optimized constructs.
+    
+    Example transformations include ternary expression folding and code hoisting
+    while preserving program semantics.
   }];
   let constructor = "mlir::createCIRSimplifyPass()";
   let dependentDialects = ["cir::CIRDialect"];

clang/include/clang/CIR/FrontendAction/CIRGenAction.h

@@ -49,7 +49,7 @@ class CIRGenAction : public clang::ASTFrontendAction {
 public:
   ~CIRGenAction() override;
 
-  OutputType action;
+  OutputType Action;
 };
 
 class EmitCIRAction : public CIRGenAction {

clang/lib/CIR/Dialect/Transforms/CIRSimplify.cpp

@@ -37,12 +37,12 @@ namespace {
 ///
 /// For example, we will simplify the following ternary operation:
 ///
-///   %0 = cir.ternary (%condition, true {
-///     %1 = cir.const ...
-///     cir.yield %1
-///   } false {
+///   %0 = ...
+///   %1 = cir.ternary (%condition, true {
+///     %2 = cir.const ...
 ///     cir.yield %2
-///   })
+///   } false {
+///     cir.yield %0
 ///
 /// into the following sequence of operations:
 ///
@@ -103,6 +103,24 @@ struct SimplifyTernary final : public OpRewritePattern<TernaryOp> {
   }
 };
 
+/// Simplify select operations with boolean constants into simpler forms.
+///
+/// This pattern simplifies select operations where both true and false values
+/// are boolean constants. Two specific cases are handled:
+///
+/// 1. When selecting between true and false based on a condition,
+///    the operation simplifies to just the condition itself:
+///
+///    %0 = cir.select if %condition then true else false
+///    ->
+///    (replaced with %condition directly)
+///
+/// 2. When selecting between false and true based on a condition,
+///    the operation simplifies to the logical negation of the condition:
+///
+///    %0 = cir.select if %condition then false else true
+///    ->
+///    %0 = cir.unary not %condition
 struct SimplifySelect : public OpRewritePattern<SelectOp> {
   using OpRewritePattern<SelectOp>::OpRewritePattern;
 

clang/lib/CIR/FrontendAction/CIRGenAction.cpp

@@ -62,17 +62,16 @@ class CIRGenConsumer : public clang::ASTConsumer {
   IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
   std::unique_ptr<CIRGenerator> Gen;
   const FrontendOptions &FEOptions;
-  CodeGenOptions &codeGenOptions;
+  CodeGenOptions &CGO;
 
 public:
   CIRGenConsumer(CIRGenAction::OutputType Action, CompilerInstance &CI,
-                 CodeGenOptions &codeGenOptions,
-                 std::unique_ptr<raw_pwrite_stream> OS)
+                 CodeGenOptions &CGO, std::unique_ptr<raw_pwrite_stream> OS)
       : Action(Action), CI(CI), OutputStream(std::move(OS)),
         FS(&CI.getVirtualFileSystem()),
         Gen(std::make_unique<CIRGenerator>(CI.getDiagnostics(), std::move(FS),
                                            CI.getCodeGenOpts())),
-        FEOptions(CI.getFrontendOpts()), codeGenOptions(codeGenOptions) {}
+        FEOptions(CI.getFrontendOpts()), CGO(CGO) {}
 
   void Initialize(ASTContext &Ctx) override {
     assert(!Context && "initialized multiple times");
@@ -105,7 +104,7 @@ class CIRGenConsumer : public clang::ASTConsumer {
       // Setup and run CIR pipeline.
       if (runCIRToCIRPasses(MlirModule, MlirCtx, C,
                             !FEOptions.ClangIRDisableCIRVerifier,
-                            codeGenOptions.OptimizationLevel > 0)
+                            CGO.OptimizationLevel > 0)
               .failed()) {
         CI.getDiagnostics().Report(diag::err_cir_to_cir_transform_failed);
         return;
@@ -142,7 +141,7 @@ class CIRGenConsumer : public clang::ASTConsumer {
 void CIRGenConsumer::anchor() {}
 
 CIRGenAction::CIRGenAction(OutputType Act, mlir::MLIRContext *MLIRCtx)
-    : MLIRCtx(MLIRCtx ? MLIRCtx : new mlir::MLIRContext), action(Act) {}
+    : MLIRCtx(MLIRCtx ? MLIRCtx : new mlir::MLIRContext), Action(Act) {}
 
 CIRGenAction::~CIRGenAction() { MLIRMod.release(); }
 
@@ -165,14 +164,14 @@ getOutputStream(CompilerInstance &CI, StringRef InFile,
 }
 
 std::unique_ptr<ASTConsumer>
-CIRGenAction::CreateASTConsumer(CompilerInstance &ci, StringRef inFile) {
-  std::unique_ptr<llvm::raw_pwrite_stream> out = ci.takeOutputStream();
+CIRGenAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
+  std::unique_ptr<llvm::raw_pwrite_stream> Out = CI.takeOutputStream();
 
-  if (!out)
-    out = getOutputStream(ci, inFile, action);
+  if (!Out)
+    Out = getOutputStream(CI, InFile, Action);
 
   auto Result = std::make_unique<cir::CIRGenConsumer>(
-      action, ci, ci.getCodeGenOpts(), std::move(out));
+      Action, CI, CI.getCodeGenOpts(), std::move(Out));
 
   return Result;
 }

clang/test/CIR/Transforms/ternary-fold.cir

@@ -34,6 +34,22 @@ module {
   // CHECK-NEXT:   cir.return %[[#B]] : !s32i
   // CHECK-NEXT: }
 
+  cir.func @simplify_ternary_false_const(%arg0 : !cir.bool, %arg1 : !s32i) -> !s32i {
+    %0 = cir.ternary (%arg0, true {
+      cir.yield %arg1 : !s32i
+    }, false {
+      %1 = cir.const #cir.int<24> : !s32i
+      cir.yield %1 : !s32i
+    }) : (!cir.bool) -> !s32i
+    cir.return %0 : !s32i
+  }
+
+  //      CHECK: cir.func @simplify_ternary_false_const(%[[ARG0:.+]]: !cir.bool, %[[ARG1:.+]]: !s32i) -> !s32i {
+  // CHECK-NEXT:   %[[#A:]] = cir.const #cir.int<24> : !s32i
+  // CHECK-NEXT:   %[[#B:]] = cir.select if %[[ARG0]] then %[[ARG1]] else %[[#A]] : (!cir.bool, !s32i, !s32i) -> !s32i
+  // CHECK-NEXT:   cir.return %[[#B]] : !s32i
+  // CHECK-NEXT: }
+
   cir.func @non_simplifiable_ternary(%arg0 : !cir.bool) -> !s32i {
     %0 = cir.alloca !s32i, !cir.ptr<!s32i>, ["a", init]
     %1 = cir.ternary (%arg0, true {