InstCombine llvm.aarch64.sve.sel to select. This allows an existing InstCombine added in 20b0fa91c9ee to fire.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
Why do we have llvm.aarch64.sve.sel if we are always going to replace it with a select? Why not remove llvm.aarch64.sve.sel entirely?
Comment Actions
I'm not the key decisionmaker here, but hopefully I can summarize the position and others can chime in if I'm mistaken or misdirecting.
- You're right, and we've been discussing the possibility removing these intrinsics where they're unnecessary.
- The intrinsics exist largely to support the ACLE.
- There are a significant number of them, at the moment there is a straightforward correspondence between the C level and IR level.
- Because of the correspondence, there is currently very little code/complexity per-intrinsic required in clang to support the ACLE.
- Therefore, removing these intrinsics requires increasing the complexity of the frontend.
- Some intrinsics will always be required.
Given these considerations, our current thinking is to have a future effort to see if removing intrinsics like this and lowering directly to clean IR is a net win in total. And to commit to doing this with a holistic view of the situation rather than doing it bit-by-bit and making a mess.
nit: Does this constructor avoid doing this in two steps?