Skip to content

Conversation

@ecnelises
Copy link
Member

@ecnelises ecnelises commented Sep 5, 2023

VSX has 'Predicate Compare Instructions' to compare float values with specific predicates and save result into VSX register. They can be used to optimize select_cc of float types. For target CPUs without support for scalar comparisons, vector version will be used.

@ecnelises ecnelises changed the title [PowerPC] Prefer xxsel for vector selection [PowerPC] Prefer VSX compare in vector selection Sep 5, 2023
@bzEq
Copy link
Collaborator

bzEq commented Sep 9, 2023

Can you explain the rationale of the preference of VSX compare? We only see two cases improved in the test. Are there scenarios vector compare is better?

@ecnelises
Copy link
Member Author

This is splitted from https://reviews.llvm.org/D75895. Since VSX instructions can use twice registers than VMX, it does no harm to codegen.

Copy link
Member

@nemanjai nemanjai left a comment

Choose a reason for hiding this comment

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

Also, please add a description that will reflect what the eventual commit message would be.

@ecnelises ecnelises changed the title [PowerPC] Prefer VSX compare in vector selection [PowerPC] Optimize select_cc with VSX compare and xxsel Dec 4, 2023
@ecnelises
Copy link
Member Author

In previous plan, this patch adds patterns for xxsel and xscmp(eq|ge|gt)(s|d)p. I revised to make it a combine, because:

  1. The instruction does not touch CR, we can't use it to lower SETCC. It seems SETCC and SELECT_CC share table of setCondCodeAction. Implementing it in legaliztion requires large change to existing SETCC handling.
  2. The ideal select_cc f32/f64/f128 ... pattern won't appear in isel, instead, they are transformed in legalization and hand coded isel.
  3. Implementing in Select() cannot fallback easily if target CPU doesn't meet requirement.

@ecnelises
Copy link
Member Author

Gentle ping

1 similar comment
@ecnelises
Copy link
Member Author

Gentle ping

@nemanjai
Copy link
Member

Please note that I have just approved to unblock the review. I have not reviewed it yet, simply approved it to remove my blocking change request so others can review it and I will try to find time to review it myself next week.

@arsenm
Copy link
Contributor

arsenm commented Apr 2, 2025

Is this still relevant?

@nemanjai
Copy link
Member

@lei137 Can you please close this if it isn't relevant or rebase to make it current and ask for another review if it is relevant?

@lei137
Copy link
Contributor

lei137 commented Jun 26, 2025

Closing this PR as we are not certain if this is still relevant for us at this time.

@lei137 lei137 closed this Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants