[PATCH] D125755: [AggressiveInstcombine] Conditionally fold saturated fptosi to llvm.fptosi.sat

[Diogo N. Sampaio via Phabricator via llvm-commits]

dnsampaio added inline comments.


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:405-407
+  SatCost += TTI.getCastInstrCost(Instruction::SExt, SatTy, IntTy,
+                                  TTI::CastContextHint::None,
+                                  TTI::TCK_RecipThroughput);
----------------
Hi  @dmgreen, long time.
I'm updating our downstream compiler from llvm 12 to llvm 15 and this getCastInstrCost call is doing a rather strange thing, by asking the cost of  a sign extend from `i32` to `i8`, which seems awkward.
Instead of using the fixed Instruction::SExt, shouldn't it be using `CastInst::getCastOpcode(IntTy, true, SatTy, true)` here?

Reproducer:
```
@a = global float 0.000000e+00
@b = global i32 0

define i32 @c() {
  %1 = load float, ptr @a
  %2 = fptosi float %1 to i32
  %3 = call i32 @llvm.smin.i32(i32 %2, i32 127)
  %4 = call i32 @llvm.smax.i32(i32 %3, i32 -128)
  store i32 %4, ptr @b
  ret i32 undef
}

declare i32 @llvm.smin.i32(i32, i32)

declare i32 @llvm.smax.i32(i32, i32)
```

Coming from a creduced code:
```
float a;
b;
c() {
  b = a;
  if (b > 127)
    b = 127;
  if (b < -128)
    b = -128;
}
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125755