[PATCH] D81728: [InstCombine] Add target-specific inst combining

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> This combines instructions, so I think it belongs into the InstCombine pass. On the other hand, the f16 form of the intrinsics is not available on all targets, so this combination cannot be applied unconditionally but it needs to be gated depending on the target.

I don't think this is a great justification for doing anything here.  You can always reverse the transform in isel on targets where it isn't supported; adding more IR patterns increases the potential for missed optimizations.

That said, I think moving the handling for target intrinsics into the target makes sense as a cleanup.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:1444
+              *this, *II, DemandedElts, UndefElts, UndefElts2, UndefElts3,
+              simplifyAndSetOp, &V))
+        return V;
----------------
Is there some way we can check that an intrinsic is actually target-specific, to discourage people from handling generic intrinsics in target-specific ways?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81728