Skip to content

Conversation

@slyubomirsky
Copy link
Contributor

@slyubomirsky slyubomirsky commented Jan 24, 2024

This is a temporary fix that prevents FuseTIR from removing PrimFuncs that are used in in-place calls. Previously, FuseTIR removed all PrimFuncs that are not marked public other than those that can be handled in fusion. The permanent solution to this issue would be to handle these PrimFuncs in FuseTIR and FuseOps, which I am planning to attempt, but that should not be a blocker on using in-place PrimFuncs with those passes (which are part of the compilation pipeline).

@Lunderberg
Copy link
Contributor

Would it be easier to fix the underlying issue in FuseTIR? It looks like it should be doable by having this loop maintain a copy of every function in funcs_to_keep, rather than a subset, then adding DeadCodeElimination() to this list of passes. That way, FuseTIR keeps around all the original unfused functions, and it gets cleaned up by the dedicated pass afterwards.

@slyubomirsky
Copy link
Contributor Author

I'm working on getting fusion for in-place cases working in PrimFuncs. These PrimFuncs will not be dead code either, since they will actually be called in the final program. I'll see if it will be just as easy as putting them in the funcs_to_keep set, though.

@slyubomirsky
Copy link
Contributor Author

Now this checks for the in-place calls in FuseTIR (only temporary, I hope, as I plan to handle fusion for in-place PrimFuncs correctly soon). We can decide which way we prefer.

@slyubomirsky slyubomirsky changed the title [Unity][Relax] Add global symbol to PrimFuncs introduced by in-place transformation [Unity][Relax] Avoid removing PrimFuncs that are used for in-place calls in FuseTIR Jan 24, 2024
@Lunderberg
Copy link
Contributor

These PrimFuncs will not be dead code either, since they will actually be called in the final program.

Hmm. My impression for the funcs_to_keep was that it forwarded all PrimFuncs that were called by the user-facing functions, and discarded all the rest. Any functions that were discarded could instead have been discarded by DeadCodeElimination.

I'll test out the idea and make a separate PR if it works. For now, I think this PR is good to go.

@slyubomirsky
Copy link
Contributor Author

No longer necessary thanks to 16487

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.

2 participants