llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.82k stars 11.91k forks source link

RegionKind::Graph and 'applyPatternsAndFoldGreedily()' results in "Assertion `!rewriterImpl.currentConversionPattern && "already inside of a pattern rewrite"' failed." #47917

Open teqdruid opened 3 years ago

teqdruid commented 3 years ago
Bugzilla Link 48573
Version unspecified
OS Linux
Attachments Test case
CC @jpienaar,@River707

Extended Description

Hi All-

I've got a pair of custom ops which I can eliminate when they're back-to-back. I framed this transform as a canonicalization. When run with '-canonicalize', the application produces the error in the summary and in the stack trace at the end of this report. When I run the same pattern as a dialect conversion (via 'applyPartialConversion'), it works as expected. When I substitute that call with 'applyPatternsAndFoldGreedily', it crashes with with the stack trace at the end of this post.

A little more about the custom ops: they form a tight cycle and live in a 'RegionKind::Graph'. In other words, given op (A) and op (B): one of A's results is an operand on B and one of B's results is one of A's operands. Other than these tight cycles, there is a larger one between

If you need a more detailed look, this is the PR we're having trouble with: https://github.com/llvm/circt/pull/340.

circt-opt: /home/jodemme/circt/llvm/mlir/lib/Transforms/Utils/DialectConversion.cpp:1430: virtual mlir::LogicalResult mlir::ConversionPattern::matchAndRewrite(mlir::Operation *, mlir::PatternRewriter &) const: Assertion `!rewriterImpl.currentConversionPattern && "already inside of a pattern rewrite"' failed. PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace. Stack dump:

  1. Program arguments: /home/jodemme/circt/build/bin/circt-opt /home/jodemme/circt/test/esi/lowering.mlir --lower-esi-to-physical --lower-esi-to-rtl -verify-diagnostics

    ​0 0x00007ff7eac648aa llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/jodemme/circt/llvm/llvm/lib/Support/Unix/Signals.inc:563:11

    ​1 0x00007ff7eac64a7b PrintStackTraceSignalHandler(void*) /home/jodemme/circt/llvm/llvm/lib/Support/Unix/Signals.inc:630:1

    ​2 0x00007ff7eac6306b llvm::sys::RunSignalHandlers() /home/jodemme/circt/llvm/llvm/lib/Support/Signals.cpp:70:5

    ​3 0x00007ff7eac651cd SignalHandler(int) /home/jodemme/circt/llvm/llvm/lib/Support/Unix/Signals.inc:405:1

    ​4 0x00007ff7ed50e3c0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x153c0)

    ​5 0x00007ff7ea38a18b raise /build/glibc-ZN95T4/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1

    ​6 0x00007ff7ea369859 abort /build/glibc-ZN95T4/glibc-2.31/stdlib/abort.c:81:7

    ​7 0x00007ff7ea369729 get_sysdep_segment_value /build/glibc-ZN95T4/glibc-2.31/intl/loadmsgcat.c:509:8

    ​8 0x00007ff7ea369729 _nl_load_domain /build/glibc-ZN95T4/glibc-2.31/intl/loadmsgcat.c:970:34

    ​9 0x00007ff7ea37af36 (/lib/x86_64-linux-gnu/libc.so.6+0x36f36)

    ​10 0x00007ff7ec92e657 mlir::ConversionPattern::matchAndRewrite(mlir::Operation*, mlir::PatternRewriter&) const /home/jodemme/circt/llvm/mlir/lib/Transforms/Utils/DialectConversion.cpp:1432:7

    ​11 0x00007ff7ec823b70 mlir::PatternApplicator::matchAndRewrite(mlir::Operation*, mlir::PatternRewriter&, llvm::function_ref<bool (mlir::Pattern const&)>, llvm::function_ref<void (mlir::Pattern const&)>, llvm::function_ref<mlir::LogicalResult (mlir::Pattern const&)>) /home/jodemme/circt/llvm/mlir/lib/Rewrite/PatternApplicator.cpp:168:22

    ​12 0x00007ff7ec98e129 (anonymous namespace)::GreedyPatternRewriteDriver::simplify(llvm::MutableArrayRef, int) /home/jodemme/circt/llvm/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp:200:36

    ​13 0x00007ff7ec98db92 mlir::applyPatternsAndFoldGreedily(llvm::MutableArrayRef, mlir::FrozenRewritePatternList const&, unsigned int) /home/jodemme/circt/llvm/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp:258:8

    ​14 0x00007ff7ec98da78 mlir::applyPatternsAndFoldGreedily(mlir::Operation*, mlir::FrozenRewritePatternList const&, unsigned int) /home/jodemme/circt/llvm/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp:229:10

    ​15 0x00007ff7ec98da33 mlir::applyPatternsAndFoldGreedily(mlir::Operation*, mlir::FrozenRewritePatternList const&) /home/jodemme/circt/llvm/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp:223:10

    ​16 0x0000000000498ac5 (anonymous namespace)::ESItoRTLPass::runOnOperation() /home/jodemme/circt/lib/Dialect/ESI/ESIPasses.cpp:288:3

    ​17 0x00007ff7ebf1d49d mlir::detail::OpToOpPassAdaptor::run(mlir::Pass, mlir::Operation, mlir::AnalysisManager, bool) /home/jodemme/circt/llvm/mlir/lib/Pass/Pass.cpp:370:21

    ​18 0x00007ff7ebf1da1d mlir::detail::OpToOpPassAdaptor::runPipeline(llvm::iterator_range<llvm::pointee_iterator<std::unique_ptr<mlir::Pass, std::default_delete >, mlir::Pass> >, mlir::Operation, mlir::AnalysisManager, bool, mlir::PassInstrumentor, mlir::PassInstrumentation::PipelineParentInfo const) /home/jodemme/circt/llvm/mlir/lib/Pass/Pass.cpp:410:16

    ​19 0x00007ff7ebf1f8b7 mlir::PassManager::run(mlir::Operation*) /home/jodemme/circt/llvm/mlir/lib/Pass/Pass.cpp:815:13

    ​20 0x00007ff7ed4e9e69 performActions(llvm::raw_ostream&, bool, bool, llvm::SourceMgr&, mlir::MLIRContext*, mlir::PassPipelineCLParser const&) /home/jodemme/circt/llvm/mlir/lib/Support/MlirOptMain.cpp:74:17

    ​21 0x00007ff7ed4e9108 processBuffer(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete >, bool, bool, bool, bool, mlir::PassPipelineCLParser const&, mlir::DialectRegistry&) /home/jodemme/circt/llvm/mlir/lib/Support/MlirOptMain.cpp:117:3

    ​22 0x00007ff7ed4e8f06 mlir::MlirOptMain(llvm::raw_ostream&, std::unique_ptr<llvm::MemoryBuffer, std::default_delete >, mlir::PassPipelineCLParser const&, mlir::DialectRegistry&, bool, bool, bool, bool, bool) /home/jodemme/circt/llvm/mlir/lib/Support/MlirOptMain.cpp:145:10

    ​23 0x0000000000442110 main /home/jodemme/circt/tools/circt-opt/circt-opt.cpp:149:17

    ​24 0x00007ff7ea36b0b3 __libc_start_main /build/glibc-ZN95T4/glibc-2.31/csu/../csu/libc-start.c:342:3

    ​25 0x0000000000441b5e _start (/home/jodemme/circt/build/bin/circt-opt+0x441b5e)

River707 commented 3 years ago

You can't use a ConversionPattern as a canonicalization pattern. You can only use ConversionPatterns when using DialectConversion (i.e. via apply*Conversion). You will need to use a RewritePattern/OpRewritePattern for your pattern instead.

teqdruid commented 3 years ago

Actually, a blank, never-matches OpConversionPattern also fails. I'm guessing it's something in the greedy logic operating on the graph regionkind.

namespace { /// Eliminate back-to-back wrap-unwraps to reduce the number of ESI channels. struct RemoveWrapUnwrap : public OpConversionPattern { public: RemoveWrapUnwrap(MLIRContext *ctxt) : OpConversionPattern(ctxt) {} using OpConversionPattern::OpConversionPattern;

LogicalResult match(Operation *op) const final { return failure(); }

void rewrite(WrapValidReady wrap, ArrayRef operands, ConversionPatternRewriter &rewriter) const final { } }; } // anonymous namespace

void WrapValidReady::getCanonicalizationPatterns( OwningRewritePatternList &patterns, MLIRContext *ctxt) { patterns.insert(ctxt); }

teqdruid commented 3 years ago

This got submitted before I was done writing it... Must've mis-clicked while adding the attachment.

"Other than these tight cycles, there is a larger one between"... This is outdated. I was able to reduce the test case to the 'wrap-unwrap' (A and B) without another cycle.

I'm not 100% sure this is an MLIR bug vs. my bug but would appreciate some help on this one.

teqdruid commented 3 years ago

Here's the OpConversionPattern:

namespace { /// Eliminate back-to-back wrap-unwraps to reduce the number of ESI channels. struct RemoveWrapUnwrap : public OpConversionPattern { public: RemoveWrapUnwrap(MLIRContext *ctxt) : OpConversionPattern(ctxt) {} using OpConversionPattern::OpConversionPattern;

LogicalResult match(Operation *op) const final { auto wrap = dyn_cast(op); if (!wrap) return failure(); if (!wrap.chanOutput().hasOneUse()) return failure(); if (!isa(wrap.chanOutput().use_begin()->getOwner())) return failure(); return success(); }

void rewrite(WrapValidReady wrap, ArrayRef operands, ConversionPatternRewriter &rewriter) const final { UnwrapValidReady unwrap = dyn_cast(wrap.chanOutput().use_begin()->getOwner()); assert(unwrap && "Must be operating on wrap-unwrap pair");

// Find upstream uses of the wrap's ready value.
for (auto &use : llvm::make_early_inc_range(wrap.ready().getUses())) {
  // Update these uses in place to use the unwrap's ready value.
  rewriter.updateRootInPlace(use.getOwner(),
                             [&] { use.set(unwrap.ready()); });
}
rewriter.replaceOp(unwrap, operands);

// If I comment this in, valgrind emits a warning!
rewriter.eraseOp(wrap);

} }; } // anonymous namespace

teqdruid commented 3 years ago

assigned to @joker-eph