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

Cranelift: make legalization groups smarter #1745

Closed
abrown opened this issue May 22, 2020 · 3 comments
Closed

Cranelift: make legalization groups smarter #1745

abrown opened this issue May 22, 2020 · 3 comments

Comments

@abrown
Copy link
Contributor

abrown commented May 22, 2020

I am trying to implement a legalization for imul.i64x2 that must be a custom legalization function. It needs to have logic to look at the ISA-specific flags to determine what legalization to use; I know, I know, this would be fixed by the new backend, but in the meantime I would like to implement this legalization. I cannot find a good transform group to put this custom legalization in: if I put it in x86_narrow, the legalization search stops too soon and things like i128 (in narrow) don't get legalized. If I put it in another transform group, e.g. expand, I have to add i64x2 as a type the group legalizes, which prevents other i64x2 operations from being legalized (all of these are currently in x86_narrow). What can I do?

  • I could move all of the SIMD operations to x86_expand but when I do this I run into conflicts with other operations (e.g. ineg)
  • I could duplicate the logic from the shared narrow imul legalizations to my custom function
  • I could make transform groups smarter somehow, e.g. by registering custom functions for an instruction AND a type (not just an instruction) or by allowing custom functions to return a result indicating if they succeeded (if they do, we stop; if they don't, we continue looking in other groups)

Would appreciate some help thinking through this, @bnjbvr. CCing @iximeow and @whitequark as well since it could be remotely related to #1743.

@bnjbvr
Copy link
Member

bnjbvr commented May 25, 2020

I could make transform groups smarter somehow, e.g. by registering custom functions for an instruction AND a type (not just an instruction) or by allowing custom functions to return a result indicating if they succeeded (if they do, we stop; if they don't, we continue looking in other groups)

There's already something like this: one can actually "chain" legalization groups when creating them, with this exact behavior. So i'd say, feel free to add as many legalization groups that you want, as long as they have a clear usefulness.

@abrown
Copy link
Contributor Author

abrown commented May 26, 2020

Ok, I think I have a clearer idea now... perhaps the title of this issue should be "make legalization groups clearer"? I added a new x86_narrow_avx group in #1759 but there must be a better name for that. As I looked at how the legalization graph is currently set up for x86, I thought of several improvements:

  • move the SIMD instructions into x86_expand--this is what those are legalizations are truly doing (they are not really "narrowing"); but, as I learned, this breaks other things, because other legalizations in x86_narrow are expected to be the default group so we might need to add another level or shift things around to make this work
  • put the construction of the legalization graph in one place (per ISA) so it is easier to visualize what is going on; e.g. move all of the chain_with calls to meta/src/isa/x86/mod.rs. Sure, this might mean some duplication for the various ISAs but I think it would be worth it for clarity.

@bjorn3
Copy link
Contributor

bjorn3 commented Feb 3, 2021

The new backend framework doesn't use legalizations as much as the old backend framework.

@abrown abrown closed this as completed Feb 3, 2021
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

3 participants