8000 [LLVM] integrate upstream SMT by makslevental · Pull Request #8408 · llvm/circt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[LLVM] integrate upstream SMT #8408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

makslevental
Copy link
Contributor
@makslevental makslevental commented Apr 10, 2025

This PR bumps LLVM to the main commit associated with llvm/llvm-project#131480. The only non-upstreaming-SMT change is the small change in test/Dialect/Moore/types-errors.mlir.

Note, the upstream PR was only for the dialect itself. Shortly I will send subsequent PRs to move all of the other SMT stuff (ExportSMITLib, CAPI, Python bindings).

@makslevental makslevental force-pushed the makslevental/integrate-upstream-smt branch 4 times, most recently from 3a81a31 to 05a2409 Compare April 11, 2025 21:16
@makslevental makslevental marked this pull request as ready for review April 11, 2025 21:19
@makslevental makslevental requested a review from maerhart as a code owner April 11, 2025 21:19
@makslevental makslevental force-pushed the makslevental/integrate-upstream-smt branch from 05a2409 to 3b4189b Compare April 12, 2025 19:47
Copy link
Contributor
@TaoBi22 TaoBi22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks!

@@ -34,8 +34,8 @@ struct CombReplicateOpConversion : OpConversionPattern<ReplicateOp> {
LogicalResult
matchAndRewrite(ReplicateOp op, OpAdaptor adaptor,
ConversionPatternRewriter &rewriter) const override {
rewriter.replaceOpWithNewOp<smt::RepeatOp>(op, op.getMultiple(),
adaptor.getInput());
rewriter.replaceOpWithNewOp<mlir::smt::RepeatOp>(op, op.getMultiple(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to not just using namespace mlir in this file?

Copy link
Contributor Author
@makslevental makslevental Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I did that but then I couldn't tell what convention you guys are following - whether code outside of CIRCT should be fully qualified. If you say using namespace mlir is ok that's good enough for me!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely not an authority on our style conventions, but it looks like plenty of conversion passes use it!

Copy link
Member
@maerhart maerhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for doing the integration!

@makslevental makslevental force-pushed the makslevental/integrate-upstream-smt branch from e75ef0e to c930fe2 Compare April 14, 2025 17:45
@makslevental makslevental force-pushed the makslevental/integrate-upstream-smt branch from c930fe2 to e32b67b Compare April 14, 2025 18:25
@makslevental makslevental merged commit 7ea4014 into llvm:main Apr 14, 2025
@makslevental makslevental deleted the makslevental/integrate-upstream-smt branch April 14, 2025 18:34
RonxBulld pushed a commit to RonxBulld/hs-circt that referenced this pull request Apr 15, 2025
KelvinChung2000 pushed a commit to KelvinChung2000/circt that referenced this pull request Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0