Improve Bmi2.MultiplyNoFlags codegen#22047
Improve Bmi2.MultiplyNoFlags codegen#22047pentp wants to merge 2 commits intodotnet:masterfrom pentp:mulx
Conversation
|
All the MultiplyNoFlags tests failed. Could you please address them at first? |
|
@dotnet-bot test Windows_NT x64 min_opt ryujit CoreCLR Perf Tests Correctness |
|
@dotnet-bot test Windows_NT x86 full_opt ryujit CoreCLR Perf Tests Correctness |
1 similar comment
|
@dotnet-bot test Windows_NT x86 full_opt ryujit CoreCLR Perf Tests Correctness |
|
Fixed a problem with tier0, this is ready for review now. |
|
@fiigii could you take a look at this? |
|
I will take a look tonight. |
|
I'd wait for @CarolEidt's feedback before going further with this. The "magic" way wasn't exactly the favored approach. |
|
It's likely also worth keeping in mind that there are other APIs (such as |
|
I explored the MultiRegOp path used in 32-bit mode also, but that looked more complicated and most importantly, would not be pay-for-play as this magic load solution. |
|
@CarolEidt could you take a look at this? I know this solution feels like a hack, but at least it's pretty contained and I'd like to use MULX in |
|
I'm still not keen on this approach, though it is cleaner than I feared. |
|
Thank you for your contribution. As announced in dotnet/coreclr#27549 this repository will be moving to dotnet/runtime on November 13. If you would like to continue working on this PR after this date, the easiest way to move the change to dotnet/runtime is:
|
|
Given the lack of activity in this PR, I'm going to close it. Feel free to re-open if work resumes. Note that the repo will move soon, so it would be best to wait until the new repo opens to create a new PR. |
I went with the magic load route in lowering. It works out quite well (codegen is ideal, if a local var is used for the low result). Fixes #21899.
@mikedn, @fiigii, @CarolEidt do you think this might be acceptable, as all changes are limited to BMI specific functions?