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

Warn about supported datatypes #1202

Open
j2kun opened this issue Dec 16, 2024 · 2 comments
Open

Warn about supported datatypes #1202

j2kun opened this issue Dec 16, 2024 · 2 comments
Assignees

Comments

@j2kun
Copy link
Collaborator

j2kun commented Dec 16, 2024

A potential new contributor was going through the getting started docs, and accidentally used dot_product_8f.mlir instead of dot_product_8.mlir with the mlir-openfhe-to-bgv pipeline. The lowering failed for obvious reasons (bgv doesn't lower float ops) but the failure was a generic message about failing to lower secret.generic when it was marked illegal.

We should have a pass that validates the pipeline supports the input before trying to lower anything.

@j2kun j2kun self-assigned this Dec 16, 2024
@AlexanderViand-Intel
Copy link
Collaborator

AlexanderViand-Intel commented Dec 16, 2024

I noticed this similar pattern a few times in other settings, too*. I think rather than doing this with a dedicated pass (which would only work for certain pipelines and only for the input parts of that pipeline), we should add a "fail on unknown stuff" pattern to all the relevant passes.

For example, in secret-to-whatever, this pattern should match (with lower priority, if we can do that in a dialect conversion?) any secret.generic op and produce a nice error message about how the op inside wasn't recognized by the pass and is therefore likely not in the set of supported inputs.

Kind of related to #1143

.* Specifically, I noticed that if you lower something unsupported to openfhe, it'll also just give you an error message about failing to lower illegal ops, rather than admitting that it's unsupported. However, this issue is pretty much universal.

(another) EDIT: I'm also wondering if we can return the InflightDiagnostic (emitError) directly instead of a "failure" in order to surpress the default DialectConversion error about failing to lower an op marked as illegal.

@j2kun
Copy link
Collaborator Author

j2kun commented Dec 16, 2024

I generally tend to prefer input validation over JIT-style error checking, but you're right that the diagnostic system is supposed to make this more pleasant. So I'll try the low-priority pattern idea.

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

No branches or pull requests

2 participants