[CIR] Upstream cir-canonicalize pass #131891
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change introduces the cir-canonicalize pass. This is a simple cir-to-cir transformation that eliminates empty scopes and redundant branches. It will be expanded in future changes to simplify other redundant instruction sequences. MLIR verification and mlir-specific command-line option handling is also introduced here.
andykaylor
requested review from
bcardosolopes,
erichkeane,
lanza and
dkolsen-pgi
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThis change introduces the cir-canonicalize pass. This is a simple cir-to-cir transformation that eliminates empty scopes and redundant branches. It will be expanded in future changes to simplify other redundant instruction sequences. MLIR verification and mlir-specific command-line option handling is also introduced here. Patch is 30.79 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/131891.diff 26 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 5a05f6c4e3e30..fcbbe4caf3c9d 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -386,4 +386,12 @@ def warn_hlsl_langstd_minimal :
Warning<"support for HLSL language version %0 is incomplete, "
"recommend using %1 instead">,
InGroup<HLSLDXCCompat>;
+
+// ClangIR frontend errors
+def err_cir_to_cir_transform_failed : Error<
+ "CIR-to-CIR transformation failed">, DefaultFatal;
+
+def err_cir_verification_failed_pre_passes : Error<
+ "CIR module verification error before running CIR-to-CIR passes">,
+ DefaultFatal;
}
diff --git a/clang/include/clang/CIR/CIRGenerator.h b/clang/include/clang/CIR/CIRGenerator.h
index 414eba80b88b8..883dce9deb8e3 100644
--- a/clang/include/clang/CIR/CIRGenerator.h
+++ b/clang/include/clang/CIR/CIRGenerator.h
@@ -55,6 +55,10 @@ class CIRGenerator : public clang::ASTConsumer {
void Initialize(clang::ASTContext &astContext) override;
bool HandleTopLevelDecl(clang::DeclGroupRef group) override;
mlir::ModuleOp getModule() const;
+ mlir::MLIRContext &getMLIRContext() { return *mlirContext; };
+ const mlir::MLIRContext &getMLIRContext() const { return *mlirContext; };
+
+ bool verifyModule() const;
};
} // namespace cir
diff --git a/clang/include/clang/CIR/CIRToCIRPasses.h b/clang/include/clang/CIR/CIRToCIRPasses.h
new file mode 100644
index 0000000000000..361ebb9e9b840
--- /dev/null
+++ b/clang/include/clang/CIR/CIRToCIRPasses.h
@@ -0,0 +1,39 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file declares an interface for running CIR-to-CIR passes.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef CLANG_CIR_CIRTOCIRPASSES_H
+#define CLANG_CIR_CIRTOCIRPASSES_H
+
+#include "mlir/Pass/Pass.h"
+
+#include <memory>
+
+namespace clang {
+class ASTContext;
+}
+
+namespace mlir {
+class MLIRContext;
+class ModuleOp;
+} // namespace mlir
+
+namespace cir {
+
+// Run set of cleanup/prepare/etc passes CIR <-> CIR.
+mlir::LogicalResult runCIRToCIRPasses(mlir::ModuleOp theModule,
+ mlir::MLIRContext &mlirCtx,
+ clang::ASTContext &astCtx,
+ bool enableVerifier);
+
+} // namespace cir
+
+#endif // CLANG_CIR_CIRTOCIRPASSES_H_
diff --git a/clang/include/clang/CIR/Dialect/Passes.h b/clang/include/clang/CIR/Dialect/Passes.h
index b691849dfc563..aa84241bdecf0 100644
--- a/clang/include/clang/CIR/Dialect/Passes.h
+++ b/clang/include/clang/CIR/Dialect/Passes.h
@@ -20,6 +20,7 @@ class ASTContext;
}
namespace mlir {
+std::unique_ptr<Pass> createCIRCanonicalizePass();
std::unique_ptr<Pass> createCIRFlattenCFGPass();
void populateCIRPreLoweringPasses(mlir::OpPassManager &pm);
diff --git a/clang/include/clang/CIR/Dialect/Passes.td b/clang/include/clang/CIR/Dialect/Passes.td
index 84b7ecba2630a..1d9707541e917 100644
--- a/clang/include/clang/CIR/Dialect/Passes.td
+++ b/clang/include/clang/CIR/Dialect/Passes.td
@@ -11,6 +11,24 @@
include "mlir/Pass/PassBase.td"
+def CIRCanonicalize : Pass<"cir-canonicalize"> {
+ let summary = "Performs CIR canonicalization";
+ let description = [{
+ Perform canonicalizations on CIR and removes some redundant operations.
+
+ This pass performs basic cleanup and canonicalization transformations that
+ hopefully do not affect CIR-to-source fidelity and high-level code analysis
+ passes too much. Example transformations performed in this pass include
+ empty scope cleanup, trivial try cleanup, redundant branch cleanup, etc.
+ Those more "heavyweight" transformations and those transformations that
+ could significantly affect CIR-to-source fidelity are performed in the
+ `cir-simplify` pass.
+ }];
+
+ let constructor = "mlir::createCIRCanonicalizePass()";
+ let dependentDialects = ["cir::CIRDialect"];
+}
+
def CIRFlattenCFG : Pass<"cir-flatten-cfg"> {
let summary = "Produces flatten CFG";
let description = [{
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 6adff30f5c91a..52bf56727a51b 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -103,6 +103,18 @@ struct MissingFeatures {
static bool scalableVectors() { return false; }
static bool unsizedTypes() { return false; }
static bool vectorType() { return false; }
+
+ // Future CIR operations
+ static bool labelOp() { return false; }
+ static bool brCondOp() { return false; }
+ static bool switchOp() { return false; }
+ static bool tryOp() { return false; }
+ static bool unaryOp() { return false; }
+ static bool selectOp() { return false; }
+ static bool complexCreateOp() { return false; }
+ static bool complexRealOp() { return false; }
+ static bool complexImagOp() { return false; }
+ static bool callOp() { return false; }
};
} // namespace cir
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 66ae8f1c7f064..75d00b5b32cd4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2978,6 +2978,15 @@ def fapple_link_rtlib : Flag<["-"], "fapple-link-rtlib">, Group<f_Group>,
HelpText<"Force linking the clang builtins runtime library">;
/// ClangIR-specific options - BEGIN
+def clangir_disable_passes : Flag<["-"], "clangir-disable-passes">,
+ Visibility<[ClangOption, CC1Option]>,
+ HelpText<"Disable CIR transformations pipeline">,
+ MarshallingInfoFlag<FrontendOpts<"ClangIRDisablePasses">>;
+def clangir_disable_verifier : Flag<["-"], "clangir-disable-verifier">,
+ Visibility<[ClangOption, CC1Option]>,
+ HelpText<"ClangIR: Disable MLIR module verifier">,
+ MarshallingInfoFlag<FrontendOpts<"ClangIRDisableCIRVerifier">>;
+
defm clangir : BoolFOption<"clangir",
FrontendOpts<"UseClangIRPipeline">, DefaultFalse,
PosFlag<SetTrue, [], [ClangOption, CC1Option], "Use the ClangIR pipeline to compile">,
@@ -4822,8 +4831,9 @@ def : Joined<["-"], "mllvm=">,
Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>, Alias<mllvm>,
HelpText<"Alias for -mllvm">, MetaVarName<"<arg>">;
def mmlir : Separate<["-"], "mmlir">,
- Visibility<[ClangOption, CLOption, FC1Option, FlangOption]>,
- HelpText<"Additional arguments to forward to MLIR's option processing">;
+ Visibility<[ClangOption, CC1Option, FC1Option, FlangOption]>,
+ HelpText<"Additional arguments to forward to MLIR's option processing">,
+ MarshallingInfoStringVector<FrontendOpts<"MLIRArgs">>;
def ffuchsia_api_level_EQ : Joined<["-"], "ffuchsia-api-level=">,
Group<m_Group>, Visibility<[ClangOption, CC1Option]>,
HelpText<"Set Fuchsia API level">,
diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h
index 99b2d9a98ed1f..0e0cbaa537332 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -412,6 +412,12 @@ class FrontendOptions {
LLVM_PREFERRED_TYPE(bool)
unsigned UseClangIRPipeline : 1;
+ /// Disable Clang IR specific (CIR) passes
+ unsigned ClangIRDisablePasses : 1;
+
+ /// Disable Clang IR (CIR) verifier
+ unsigned ClangIRDisableCIRVerifier : 1;
+
CodeCompleteOptions CodeCompleteOpts;
/// Specifies the output format of the AST.
@@ -488,6 +494,10 @@ class FrontendOptions {
/// should only be used for debugging and experimental features.
std::vector<std::string> LLVMArgs;
+ /// A list of arguments to forward to MLIR's option processing; this
+ /// should only be used for debugging and experimental features.
+ std::vector<std::string> MLIRArgs;
+
/// File name of the file that will provide record layouts
/// (in the format produced by -fdump-record-layouts).
std::string OverrideRecordLayoutsFile;
@@ -533,7 +543,8 @@ class FrontendOptions {
EmitExtensionSymbolGraphs(false),
EmitSymbolGraphSymbolLabelsForTesting(false),
EmitPrettySymbolGraphs(false), GenReducedBMI(false),
- UseClangIRPipeline(false), TimeTraceGranularity(500),
+ UseClangIRPipeline(false), ClangIRDisablePasses(false),
+ ClangIRDisableCIRVerifier(false), TimeTraceGranularity(500),
TimeTraceVerbose(false) {}
/// getInputKindForExtension - Return the appropriate input kind for a file
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index 0e3e15ca2cadc..36bfc2cdf6f02 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -23,6 +23,7 @@
#include "mlir/IR/BuiltinOps.h"
#include "mlir/IR/Location.h"
#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/Verifier.h"
using namespace clang;
using namespace clang::CIRGen;
@@ -488,6 +489,13 @@ mlir::Type CIRGenModule::convertType(QualType type) {
return genTypes.convertType(type);
}
+bool CIRGenModule::verifyModule() const {
+ // Verify the module after we have finished constructing it, this will
+ // check the structural properties of the IR and invoke any specific
+ // verifiers we have on the CIR operations.
+ return mlir::verify(theModule).succeeded();
+}
+
DiagnosticBuilder CIRGenModule::errorNYI(SourceLocation loc,
llvm::StringRef feature) {
unsigned diagID = diags.getCustomDiagID(
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index 2a798f4cd56a9..734cafa2e07bb 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -91,6 +91,8 @@ class CIRGenModule : public CIRGenTypeCache {
void emitTopLevelDecl(clang::Decl *decl);
+ bool verifyModule() const;
+
/// Return the address of the given function. If funcType is non-null, then
/// this function will use the specified type if it has to create it.
// TODO: this is a bit weird as `GetAddr` given we give back a FuncOp?
diff --git a/clang/lib/CIR/CodeGen/CIRGenerator.cpp b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
index 6fa31ab707139..33f0c292c7710 100644
--- a/clang/lib/CIR/CodeGen/CIRGenerator.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
@@ -40,6 +40,8 @@ void CIRGenerator::Initialize(ASTContext &astContext) {
*mlirContext.get(), astContext, codeGenOpts, diags);
}
+bool CIRGenerator::verifyModule() const { return cgm->verifyModule(); }
+
mlir::ModuleOp CIRGenerator::getModule() const { return cgm->getModule(); }
bool CIRGenerator::HandleTopLevelDecl(DeclGroupRef group) {
diff --git a/clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp b/clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp
new file mode 100644
index 0000000000000..8aa73ea5c1d2a
--- /dev/null
+++ b/clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp
@@ -0,0 +1,147 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements pass that canonicalizes CIR operations, eliminating
+// redundant branches, empty scopes, and other unnecessary operations.
+//
+//===----------------------------------------------------------------------===//
+
+#include "PassDetail.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/IR/Block.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/PatternMatch.h"
+#include "mlir/IR/Region.h"
+#include "mlir/Support/LogicalResult.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "clang/CIR/Dialect/IR/CIRDialect.h"
+#include "clang/CIR/Dialect/Passes.h"
+#include "clang/CIR/MissingFeatures.h"
+
+using namespace mlir;
+using namespace cir;
+
+namespace {
+
+/// Removes branches between two blocks if it is the only branch.
+///
+/// From:
+/// ^bb0:
+/// cir.br ^bb1
+/// ^bb1: // pred: ^bb0
+/// cir.return
+///
+/// To:
+/// ^bb0:
+/// cir.return
+struct RemoveRedundantBranches : public OpRewritePattern<BrOp> {
+ using OpRewritePattern<BrOp>::OpRewritePattern;
+
+ LogicalResult matchAndRewrite(BrOp op,
+ PatternRewriter &rewriter) const final {
+ Block *block = op.getOperation()->getBlock();
+ Block *dest = op.getDest();
+
+ assert(!cir::MissingFeatures::labelOp());
+
+ // Single edge between blocks: merge it.
+ if (block->getNumSuccessors() == 1 &&
+ dest->getSinglePredecessor() == block) {
+ rewriter.eraseOp(op);
+ rewriter.mergeBlocks(dest, block);
+ return success();
+ }
+
+ return failure();
+ }
+};
+
+struct RemoveEmptyScope
+ : public OpRewritePattern<ScopeOp>::SplitMatchAndRewrite {
+ using SplitMatchAndRewrite::SplitMatchAndRewrite;
+
+ LogicalResult match(ScopeOp op) const final {
+ // TODO: Remove this logic once CIR uses MLIR infrastructure to remove
+ // trivially dead operations
+ if (op.isEmpty()) {
+ return success();
+ }
+
+ Region *region = &(op.getScopeRegion()); // getRegions().front();
+ if (region && region->getBlocks().front().getOperations().size() == 1) {
+ return success(isa<YieldOp>(region->getBlocks().front().front()));
+ }
+
+ return failure();
+ }
+
+ void rewrite(ScopeOp op, PatternRewriter &rewriter) const final {
+ rewriter.eraseOp(op);
+ }
+};
+
+//===----------------------------------------------------------------------===//
+// CIRCanonicalizePass
+//===----------------------------------------------------------------------===//
+
+struct CIRCanonicalizePass : public CIRCanonicalizeBase<CIRCanonicalizePass> {
+ using CIRCanonicalizeBase::CIRCanonicalizeBase;
+
+ // The same operation rewriting done here could have been performed
+ // by CanonicalizerPass (adding hasCanonicalizer for target Ops and
+ // implementing the same from above in CIRDialects.cpp). However, it's
+ // currently too aggressive for static analysis purposes, since it might
+ // remove things where a diagnostic can be generated.
+ //
+ // FIXME: perhaps we can add one more mode to GreedyRewriteConfig to
+ // disable this behavior.
+ void runOnOperation() override;
+};
+
+void populateCIRCanonicalizePatterns(RewritePatternSet &patterns) {
+ // clang-format off
+ patterns.add<
+ RemoveRedundantBranches,
+ RemoveEmptyScope
+ >(patterns.getContext());
+ // clang-format on
+}
+
+void CIRCanonicalizePass::runOnOperation() {
+ // Collect rewrite patterns.
+ RewritePatternSet patterns(&getContext());
+ populateCIRCanonicalizePatterns(patterns);
+
+ // Collect operations to apply patterns.
+ llvm::SmallVector<Operation *, 16> ops;
+ getOperation()->walk([&](Operation *op) {
+ assert(!cir::MissingFeatures::brCondOp());
+ assert(!cir::MissingFeatures::switchOp());
+ assert(!cir::MissingFeatures::tryOp());
+ assert(!cir::MissingFeatures::unaryOp());
+ assert(!cir::MissingFeatures::selectOp());
+ assert(!cir::MissingFeatures::complexCreateOp());
+ assert(!cir::MissingFeatures::complexRealOp());
+ assert(!cir::MissingFeatures::complexImagOp());
+ assert(!cir::MissingFeatures::callOp());
+ // CastOp here is to perform a manual `fold` in
+ // applyOpPatternsGreedily
+ if (isa<BrOp, ScopeOp, CastOp>(op))
+ ops.push_back(op);
+ });
+
+ // Apply patterns.
+ if (applyOpPatternsGreedily(ops, std::move(patterns)).failed())
+ signalPassFailure();
+}
+
+} // namespace
+
+std::unique_ptr<Pass> mlir::createCIRCanonicalizePass() {
+ return std::make_unique<CIRCanonicalizePass>();
+}
diff --git a/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt b/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
index aa27074cc6131..648666d2461de 100644
--- a/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
+++ b/clang/lib/CIR/Dialect/Transforms/CMakeLists.txt
@@ -1,4 +1,5 @@
add_clang_library(MLIRCIRTransforms
+ CIRCanonicalize.cpp
FlattenCFG.cpp
DEPENDS
diff --git a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
index 0f686a36b982b..8ef2e8fe15db7 100644
--- a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
+++ b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
@@ -9,7 +9,9 @@
#include "clang/CIR/FrontendAction/CIRGenAction.h"
#include "mlir/IR/MLIRContext.h"
#include "mlir/IR/OwningOpRef.h"
+#include "clang/Basic/DiagnosticFrontend.h"
#include "clang/CIR/CIRGenerator.h"
+#include "clang/CIR/CIRToCIRPasses.h"
#include "clang/CIR/LowerToLLVM.h"
#include "clang/CodeGen/BackendUtil.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -59,6 +61,7 @@ class CIRGenConsumer : public clang::ASTConsumer {
ASTContext *Context{nullptr};
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
std::unique_ptr<CIRGenerator> Gen;
+ const FrontendOptions &FEOptions;
public:
CIRGenConsumer(CIRGenAction::OutputType Action, CompilerInstance &CI,
@@ -66,7 +69,8 @@ class CIRGenConsumer : public clang::ASTConsumer {
: Action(Action), CI(CI), OutputStream(std::move(OS)),
FS(&CI.getVirtualFileSystem()),
Gen(std::make_unique<CIRGenerator>(CI.getDiagnostics(), std::move(FS),
- CI.getCodeGenOpts())) {}
+ CI.getCodeGenOpts())),
+ FEOptions(CI.getFrontendOpts()) {}
void Initialize(ASTContext &Ctx) override {
assert(!Context && "initialized multiple times");
@@ -81,7 +85,31 @@ class CIRGenConsumer : public clang::ASTConsumer {
void HandleTranslationUnit(ASTContext &C) override {
Gen->HandleTranslationUnit(C);
+
+ if (!FEOptions.ClangIRDisableCIRVerifier) {
+ if (!Gen->verifyModule()) {
+ CI.getDiagnostics().Report(
+ diag::err_cir_verification_failed_pre_passes);
+ llvm::report_fatal_error(
+ "CIR codegen: module verification error before running CIR passes");
+ return;
+ }
+ }
+
mlir::ModuleOp MlirModule = Gen->getModule();
+ mlir::MLIRContext &MlirCtx = Gen->getMLIRContext();
+
+ if (!FEOptions.ClangIRDisablePasses) {
+ // Setup and run CIR pipeline.
+ std::string passOptParsingFailure;
+ if (runCIRToCIRPasses(MlirModule, MlirCtx, C,
+ !FEOptions.ClangIRDisableCIRVerifier)
+ .failed()) {
+ CI.getDiagnostics().Report(diag::err_cir_to_cir_transform_failed);
+ return;
+ }
+ }
+
switch (Action) {
case CIRGenAction::OutputType::EmitCIR:
if (OutputStream && MlirModule) {
diff --git a/clang/lib/CIR/FrontendAction/CMakeLists.txt b/clang/lib/CIR/FrontendAction/CMakeLists.txt
index 1ebac07f44662..50d6ea7108ce1 100644
--- a/clang/lib/CIR/FrontendAction/CMakeLists.txt
+++ b/clang/lib/CIR/FrontendAction/CMakeLists.txt
@@ -15,8 +15,10 @@ add_clang_library(clangCIRFrontendAction
LINK_LIBS
clangAST
+ clangBasic
clangFrontend
clangCIR
+ clangCIRLoweringCommon
clangCIRLoweringDirectToLLVM
clangCodeGen
MLIRCIR
diff --git a/clang/lib/CIR/Lowering/CIRPasses.cpp b/clang/lib/CIR/Lowering/CIRPasses.cpp
index 235acbfee8967..1616ac6145151 100644
--- a/clang/lib/CIR/Lowering/CIRPasses.cpp
+++ b/clang/lib/CIR/Lowering/CIRPasses.cpp
@@ -11,9 +11,28 @@
//===----------------------------------------------------------------------===//
// #include "clang/AST/ASTContext.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/Pass/PassManager.h"
#include "clang/CIR/Dialect/Passes.h"
+#include "llvm/Support/TimeProfiler.h"
-#include "mlir/Pass/PassManager.h"
+namespace cir {
+mlir::LogicalResult runCIRToCIRPasses(mlir::ModuleOp theModule,
+ mlir::MLIRContext &mlirContext,
+ clang::ASTContext &astContext,
+ bool enableVerifier) {
+
+ llvm::TimeTraceScope scope("CIR To CIR Passes")...
[truncated]
|
erichkeane
reviewed
Mar 18, 2025
| Perform canonicalizations on CIR and removes some redundant operations. | ||
|
|
||
| This pass performs basic cleanup and canonicalization transformations that | ||
| hopefully do not affect CIR-to-source fidelity and high-level code analysis |
There was a problem hiding this comment.
instead of 'hopefully' something like 'are not intended to affect', or 'should not affect'.
| This pass performs basic cleanup and canonicalization transformations that | ||
| hopefully do not affect CIR-to-source fidelity and high-level code analysis | ||
| passes too much. Example transformations performed in this pass include | ||
| empty scope cleanup, trivial try cleanup, redundant branch cleanup, etc. |
There was a problem hiding this comment.
Suggested change
| empty scope cleanup, trivial try cleanup, redundant branch cleanup, etc. | |
| empty scope cleanup, trivial `try` cleanup, redundant branch cleanup, etc. |
|
|
||
| if (!FEOptions.ClangIRDisablePasses) { | ||
| // Setup and run CIR pipeline. | ||
| std::string passOptParsingFailure; |
There was a problem hiding this comment.
unused variable?
Also, in either case, does it make sense to combine the two 'ifs'?
So :
if (!FEOptions.ClangIRDisablePasses && !Gen->verifyModule()) ...
There was a problem hiding this comment.
There will be more happening before the call to runCIRToCIRPasses() later, so I prefer not to combine the 'ifs'. Even as it is, I think the code is more readable like this since the second 'if' is a call to a function performing substantial action, and I don't like hiding the conditional nature of that call in the 'if' short-circuit semantics. That is, it's not just calculating a second condition. The first 'if' is determining if we want to perform and action, and the second 'if' is checking the result of having performed that action.
| cir.return | ||
| } | ||
| // CHECK: cir.func @redundant_br() { | ||
| // CHECK-NOT: ^ |
There was a problem hiding this comment.
Instead of the CHECK-NOT, could we just do a CHECK-NEXT in these few places?
clang/tools/driver/cc1as_main.cpp
Outdated
| @@ -326,6 +326,7 @@ bool AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts, | |||
| } | |||
| } | |||
| Opts.LLVMArgs = Args.getAllArgValues(OPT_mllvm); | |||
|
|
|||
| return success(); | ||
| } | ||
|
|
||
| Region *region = &(op.getScopeRegion()); // getRegions().front(); |
There was a problem hiding this comment.
This is VERY suspicious. Address-of a thing resulting in nullptr? I think we'd better ensure that there IS a scope region, then make region here a reference instead.
There was a problem hiding this comment.
Yeah, sorry. That's sloppy cleanup on my part. The incubator had the code that was commented out here, but an upstream MLIR change made getRegions() inaccessible here, and the function I found to replace it returned a reference. There will always be a region here. The old code was using a different interface for which that isn't true in general but still is for ScopeOp. I'll fix this.
bcardosolopes
approved these changes
Mar 19, 2025
erichkeane
approved these changes
Mar 19, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/7443 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/2533 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change introduces the cir-canonicalize pass. This is a simple cir-to-cir transformation that eliminates empty scopes and redundant branches. It will be expanded in future changes to simplify other redundant instruction sequences.
MLIR verification and mlir-specific command-line option handling is also introduced here.