[AIC] Fix the sext cost operands in tryToFPToSat

[dmgreen]

Summary

As pointed out in D125755 the operand of a call to getCastInstrCost had the Src and Dst the wrong way around.

This fixes that, but alters one of the tests in a way that may not be profitable, possibly due to inaccuracies in the costmodel for X86 fptosi.sat instructions that are scalarized? https://godbolt.org/z/cnKzzTKcr

[RKSimon]

I'm not certain its the costmodel - the generic expanded cost in BasicTTIImplBase looks about right - the problem seems to be the backend scalarizing when it shouldn't need to.

[RKSimon]

@e-kud is looking at the poor x86 codegen for fptosi/ui.sat here - D150372

[dmgreen]

That does look like quite an improvement! Thanks

[dnsampaio]

Thanks for looking into this, this LGTM. I imagine it's best to wait the ok from @RKSimon as well for pushing.

[RKSimon]

LGTM - let's get this fix in now and I'll assist @e-kud getting D150372 done soon

llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp

static bool tryToFPToSat(Instruction &I, TargetTransformInfo &TTI) {
   if (auto *VecTy = dyn_cast<VectorType>(IntTy))
     SatTy = VectorType::get(SatTy, VecTy->getElementCount());
 
   // Get the cost of the intrinsic, and check that against the cost of
   // fptosi+smin+smax
   InstructionCost SatCost = TTI.getIntrinsicInstrCost(
       IntrinsicCostAttributes(Intrinsic::fptosi_sat, SatTy, {In}, {FpTy}),
       TTI::TCK_RecipThroughput);
-  SatCost += TTI.getCastInstrCost(Instruction::SExt, SatTy, IntTy,
+  SatCost += TTI.getCastInstrCost(Instruction::SExt, IntTy, SatTy,
                                   TTI::CastContextHint::None,
                                   TTI::TCK_RecipThroughput);
 
   InstructionCost MinMaxCost = TTI.getCastInstrCost(
       Instruction::FPToSI, IntTy, FpTy, TTI::CastContextHint::None,
       TTI::TCK_RecipThroughput);
   MinMaxCost += TTI.getIntrinsicInstrCost(
       IntrinsicCostAttributes(Intrinsic::smin, IntTy, {IntTy}),

llvm/test/Transforms/AggressiveInstCombine/X86/fptosisat.ll

;
   %conv = fptosi half %in to i32
   %min = call i32 @llvm.smin.i32(i32 %conv, i32 127)
   %max = call i32 @llvm.smax.i32(i32 %min, i32 -128)
   ret i32 %max
 }
 
 define <4 x i64> @v4f32_i32(<4 x float> %in) {
 ; CHECK-LABEL: @v4f32_i32(
-; CHECK-NEXT:    [[CONV:%.*]] = fptosi <4 x float> [[IN:%.*]] to <4 x i64>
-; CHECK-NEXT:    [[MIN:%.*]] = call <4 x i64> @llvm.smin.v4i64(<4 x i64> [[CONV]], <4 x i64> <i64 2147483647, i64 2147483647, i64 2147483647, i64 2147483647>)
-; CHECK-NEXT:    [[MAX:%.*]] = call <4 x i64> @llvm.smax.v4i64(<4 x i64> [[MIN]], <4 x i64> <i64 -2147483648, i64 -2147483648, i64 -2147483648, i64 -2147483648>)
-; CHECK-NEXT:    ret <4 x i64> [[MAX]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call <4 x i32> @llvm.fptosi.sat.v4i32.v4f32(<4 x float> [[IN:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = sext <4 x i32> [[TMP1]] to <4 x i64>
+; CHECK-NEXT:    ret <4 x i64> [[TMP2]]
 ;
   %conv = fptosi <4 x float> %in to <4 x i64>
   %min = call <4 x i64> @llvm.smin.v4i64(<4 x i64> %conv, <4 x i64> <i64 2147483647, i64 2147483647, i64 2147483647, i64 2147483647>)
   %max = call <4 x i64> @llvm.smax.v4i64(<4 x i64> %min, <4 x i64> <i64 -2147483648, i64 -2147483648, i64 -2147483648, i64 -2147483648>)
   ret <4 x i64> %max
 }
 
 define <8 x i32> @v8f16_i16(<8 x half> %in) {