[Mlir-commits] [llvm] [mlir] MathExtras: avoid unnecessarily widening types (PR #95426)

[Nikita Popov]

================
@@ -986,7 +986,9 @@ unsigned getMaxFlatWorkGroupSize(const MCSubtargetInfo *STI) {
 
 unsigned getWavesPerWorkGroup(const MCSubtargetInfo *STI,
                               unsigned FlatWorkGroupSize) {
-  return divideCeil(FlatWorkGroupSize, getWavefrontSize(STI));
+  // divideCeil will overflow, unless FlatWorkGroupSize is cast.
+  return divideCeil(static_cast<uint64_t>(FlatWorkGroupSize),
+                    getWavefrontSize(STI));
----------------
nikic wrote:

Hm, I think it's pretty unfortunate that we need these upcasts to avoid overflow. This would be a new footgun introduced by this change.

I think we should change the implementations to avoid overflow first (outside of this PR). Assuming this doesn't have measurable compile-time overhead (which I would not expect), it's best to make these function safe by default.

I think a typical way to do that is basically what the signed version does with `(X - 1) / Y + 1` -- it's just that this require an extra check for `X == 0` upfront. I guess a branchless variant of that would be `Bias = X != 0; return (X - Bias) / Y + Bias`.

https://github.com/llvm/llvm-project/pull/95426