[PATCH] D71550: [Intrinsic] Add fixed point saturating division intrinsics.

[Bevin Hansson via Phabricator via llvm-commits]

ebevhan added a comment.

In D71550#1835003 <https://reviews.llvm.org/D71550#1835003>, @leonardchan wrote:

> It seems that I run into `LLVM ERROR: Unsupported library call operation!` if I call `llvm.sdiv.fix.sat.i64` with a scale of 63:
>
>   define i64 @sdiv_sat_i64_s63(i64 %x, i64 %y) {
>     %tmp = call i64 @llvm.sdiv.fix.sat.i64(i64 %x, i64 %y, i32 63)
>     ret i64 %tmp;
>   }
>
>
> This seems to be the only issue I could find from running the code.


Ah, that's unfortunate... It's a consequence of being forced to handle the integer division overflow case gracefully.

With an i64, we get a sext+shl and an i65 after building. During the initial promotion we try to promote to i128. That gives us 63 bits of redundant sign (128-65), but in order to handle the case of MSB / ALL1 (MIN / -EPS) without invoking undefined behavior in the integer division, we require an extra bit. Since we don't have that bit to spare, we end up not being able to do the operation in i128. Then we get an i256 sdiv and there's no codegen for that.

I'm unsure what to do to make this look good. It would be convenient to skip the operation altogether in the overflow case, but we can't generate control flow here. I could check for the overflow case and fudge the inputs/result to prevent the overflow, but that feels sort of wrong. Might be the only viable option, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71550