[PATCH] D153620: [X86] Combine MUL+SRL+TRUNC to MULX for i32 on 64-bit

Fri Jun 23 20:22:30 PDT 2023

pengfei added a comment.

In D153620#4444499 <https://reviews.llvm.org/D153620#4444499>, @craig.topper wrote:

> If not updating arithmetic flags is a requirement, the we should have an IR intrinsic. Relying on pattern matching that can be easily broken is not a good solution.

You are right. I just found it doesn't work under O0 due to we use fast ISel. `_mulx_u64` works because it falls back to DAGISel.
But I don't see much necessity to add a intrinsic. So maybe just need to roll back it D153681 <https://reviews.llvm.org/D153681>

> Most of these test are for fixed point intrinsics. Maybe we should change the lowering of those to what we want instead of using a DAGCombine to get there?

They are not affected by DAGCombine but changes in `Select`. Is this what you expected here?

> How much do we actually care about honoring this? A program written in C can't tell if an intrinsic overwrites arithmetic flags; I'd feel free to just pick the fastest lowering, regardless of what the guide says.

I don't know. But that might explain why we implemented these intrinsics in this way. I have no preference between this and D153681 <https://reviews.llvm.org/D153681>, either way is good to me.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153620/new/

https://reviews.llvm.org/D153620