[PATCH] D158053: [Legalizer] Expand fmaximum and fminimum

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I don't believe any of the existing Arm/AArch64 tests should change with this patch. The SVE test looks like it has just regenerated check lines without any changes? Can you include this change to make the operations always legal when we have NEON, It will be better than expanding them:

  diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
  index 80a147666094..27f12f3a1cc8 100644
  --- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
  +++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
  @@ -1543,15 +1543,11 @@ ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
   
     if (Subtarget->hasNEON()) {
       // vmin and vmax aren't available in a scalar form, so we can use
  -    // a NEON instruction with an undef lane instead.  This has a performance
  -    // penalty on some cores, so we don't do this unless we have been
  -    // asked to by the core tuning model.
  -    if (Subtarget->useNEONForSinglePrecisionFP()) {
  -      setOperationAction(ISD::FMINIMUM, MVT::f32, Legal);
  -      setOperationAction(ISD::FMAXIMUM, MVT::f32, Legal);
  -      setOperationAction(ISD::FMINIMUM, MVT::f16, Legal);
  -      setOperationAction(ISD::FMAXIMUM, MVT::f16, Legal);
  -    }
  +    // a NEON instruction with an undef lane instead.
  +    setOperationAction(ISD::FMINIMUM, MVT::f32, Legal);
  +    setOperationAction(ISD::FMAXIMUM, MVT::f32, Legal);
  +    setOperationAction(ISD::FMINIMUM, MVT::f16, Legal);
  +    setOperationAction(ISD::FMAXIMUM, MVT::f16, Legal);
       setOperationAction(ISD::FMINIMUM, MVT::v2f32, Legal);
       setOperationAction(ISD::FMAXIMUM, MVT::v2f32, Legal);
       setOperationAction(ISD::FMINIMUM, MVT::v4f32, Legal);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158053