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

Disable Bit-width change for modules with external functions. #14114

Closed
wants to merge 2 commits into from

Conversation

bviyer
Copy link
Contributor

@bviyer bviyer commented Jun 14, 2023

This patch will disable bit-width changes for modules that have external functions. Since the body of the function is compiled elsewhere and just linked, any modification will cause linker issues.

Fixes: #12987

This patch will disable bit-width changes for modules that have
external functions. Since the body of the function is compiled
elsewhere and just linked, any modification will cause linker
issues.
@bviyer bviyer requested a review from benvanik as a code owner June 14, 2023 18:46
Copy link
Collaborator

@benvanik benvanik left a comment

Choose a reason for hiding this comment

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

This is too broad - the only thing that shouldn't be changed is the external function declarations and calls to those external functions. Disabling the entire pass should only be done with the command line flags for toggling the passes.

To do it at the appropriate scope you can gather all extern function declarations into a set and then in the FuncOp/CallOp patterns test if the target is in the set before bailing on that one particular pattern.

But really, if disabling the pass entirely works for you the best thing to do is just pass the compiler flags to disable the pass.

Copy link
Contributor

@ezhulenev ezhulenev left a comment

Choose a reason for hiding this comment

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

Updating markUnknownOpDynamicallyLegal should do it, just don't touch funcs that are external because it's unsafe.

// pattern match and does not have the full concept of control/data flow.
// Thus, doing it in finer-granularity causes type-mismatches.
this->getOperation().walk([&](mlir::CallOpInterface callOp) {
foundExternalFunc |= (callOp->getRegions().empty());
Copy link
Contributor

Choose a reason for hiding this comment

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

callOp always have no regions, you need to find referenced FuncOp. Or just walk func::FuncOp ops.

@benvanik
Copy link
Collaborator

Closing stale PR.

@benvanik benvanik closed this May 14, 2024
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.

DemoteF64ToF32Pass corrupts signature of imported functions
3 participants