This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] InstCombine llvm.aarch64.sve.sel to select
ClosedPublic

Authored by MattDevereau on Mar 16 2022, 5:01 AM.

Details

Summary

InstCombine llvm.aarch64.sve.sel to select. This allows an existing InstCombine added in 20b0fa91c9ee to fire.

Diff Detail

Event Timeline

MattDevereau created this revision.Mar 16 2022, 5:01 AM
Herald added a project: Restricted Project. · View Herald Transcript
MattDevereau requested review of this revision.Mar 16 2022, 5:01 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 16 2022, 5:01 AM
sdesmalen accepted this revision.Mar 16 2022, 6:49 AM
sdesmalen added a subscriber: sdesmalen.
sdesmalen added inline comments.
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
530–531

nit: Does this constructor avoid doing this in two steps?

This revision is now accepted and ready to land.Mar 16 2022, 6:49 AM

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?

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?

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.

  1. You're right, and we've been discussing the possibility removing these intrinsics where they're unnecessary.
  2. The intrinsics exist largely to support the ACLE.
  3. There are a significant number of them, at the moment there is a straightforward correspondence between the C level and IR level.
  4. Because of the correspondence, there is currently very little code/complexity per-intrinsic required in clang to support the ACLE.
  5. Therefore, removing these intrinsics requires increasing the complexity of the frontend.
  6. 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.

This revision was landed with ongoing or failed builds.Mar 17 2022, 9:21 AM
This revision was automatically updated to reflect the committed changes.