Skip to content
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

Dialect Conversion #20

Merged
merged 15 commits into from
Oct 21, 2023
Merged

Dialect Conversion #20

merged 15 commits into from
Oct 21, 2023

Conversation

j2kun
Copy link
Owner

@j2kun j2kun commented Sep 22, 2023

No description provided.

@j2kun j2kun force-pushed the dialect-conversion branch from db32c5a to e6893cf Compare September 22, 2023 22:43
@j2kun j2kun force-pushed the dialect-conversion branch 10 times, most recently from 216b314 to 6c40e28 Compare October 20, 2023 22:38
@j2kun j2kun force-pushed the dialect-conversion branch from 6c40e28 to c3a8384 Compare October 20, 2023 23:42
@j2kun j2kun merged commit fac42a3 into main Oct 21, 2023
2 checks passed
@@ -32,6 +32,20 @@ class PolyToStandardTypeConverter : public TypeConverter {
}
};

struct ConvertAdd : public OpConversionPattern<AddOp> {
ConvertAdd(mlir::MLIRContext *context)

Choose a reason for hiding this comment

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

I'm not sure of before but in MLIR 17+, I've had to instead implement this constructor,

ConvertAdd(TypeConverter& type_converter, MLIRContext* context)
    : OpConversionPattern<AddOp>(type_converter, context) {}

otherwise the call to patterns.add<ConvertAdd>(typeConverter, context); fails at compile time saying no matching constructor for initialization since the constructor we implemented here requires one argument but we provide two.

Also, it maybe just a minor preference thing but struct A : public B is equivalent to struct A : B since structs have public inheritance by default.

usainzg pushed a commit to usainzg/compdist that referenced this pull request Oct 27, 2024
* Add optional packer plugins hook

* Provide help in README and simplify module

* Fix spelling and tweak verbiage about after/plugin
// TODO: implement
arith::AddIOp addOp = rewriter.create<arith::AddIOp>(
op.getLoc(), adaptor.getLhs(), adaptor.getRhs());
rewriter.replaceOp(op.getOperation(), {addOp});

Choose a reason for hiding this comment

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

{addOp} will trigger ambiguous error in LLVM 20, so this can be implemented in this way:

rewriter.replaceOp(op.getOperation(), addOp.getOperation());

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