[mlir][arith] Align shift Ops with LLVM instructions on allowed shift amounts

[ubfx]

This patch aligns the documentation of the shift Ops in arith with respective LLVM instructions. Specifically, it is now stated that shifting by an amount equal to the operand bitwidth returns poison.

Relevant discussion: https://discourse.llvm.org/t/some-question-on-the-semantics-of-the-arith-dialect/74861/10
Relevant issue: #80960

[llvmbot]

@llvm/pr-subscribers-mlir-arith

@llvm/pr-subscribers-mlir

Author: Felix Schneider (ubfx)

Changes

This patch aligns the documentation of the shift Ops in arith with respective LLVM instructions. Specifically, it is now stated that shifting by an amount equal to the operand bitwidth returns poison.

---

Full diff: https://github.com/llvm/llvm-project/pull/82133.diff

1 Files Affected:

• (modified) mlir/include/mlir/Dialect/Arith/IR/ArithOps.td (+5-5)

diff --git a/mlir/include/mlir/Dialect/Arith/IR/ArithOps.td b/mlir/include/mlir/Dialect/Arith/IR/ArithOps.td
index 4babbe80e285f7..c9df50d0395d1f 100644
--- a/mlir/include/mlir/Dialect/Arith/IR/ArithOps.td
+++ b/mlir/include/mlir/Dialect/Arith/IR/ArithOps.td
@@ -788,7 +788,7 @@ def Arith_ShLIOp : Arith_IntBinaryOpWithOverflowFlags<"shli"> {
     The `shli` operation shifts the integer value of the first operand to the left 
     by the integer value of the second operand. The second operand is interpreted as 
     unsigned. The low order bits are filled with zeros. If the value of the second 
-    operand is greater than the bitwidth of the first operand, then the 
+    operand is greater or equal than the bitwidth of the first operand, then the
     operation returns poison.
 
     This op supports `nuw`/`nsw` overflow flags which stands stand for
@@ -818,8 +818,8 @@ def Arith_ShRUIOp : Arith_TotalIntBinaryOp<"shrui"> {
     The `shrui` operation shifts an integer value of the first operand to the right 
     by the value of the second operand. The first operand is interpreted as unsigned,
     and the second operand is interpreted as unsigned. The high order bits are always 
-    filled with zeros. If the value of the second operand is greater than the bitwidth
-    of the first operand, then the operation returns poison.
+    filled with zeros. If the value of the second operand is greater or equal than the
+    bitwidth of the first operand, then the operation returns poison.
 
     Example:
 
@@ -844,8 +844,8 @@ def Arith_ShRSIOp : Arith_TotalIntBinaryOp<"shrsi"> {
     and the second operand is interpreter as unsigned. The high order bits in the 
     output are filled with copies of the most-significant bit of the shifted value 
     (which means that the sign of the value is preserved). If the value of the second 
-    operand is greater than bitwidth of the first operand, then the operation returns 
-    poison.
+    operand is greater or equal than bitwidth of the first operand, then the operation
+    returns poison.
 
     Example:
 

[joker-eph]

In the context of the bug, shouldn't the folder be updated as well to return poison?

[joker-eph]

I think this is the line to change?

llvm-project/mlir/lib/Dialect/Arith/IR/ArithOps.cpp

Line 2404 in a6df347

bounded = b.ule(b.getBitWidth());

Also any other op affected in a similar way?

[kuhar]

In the context of the bug, shouldn't the folder be updated as well to return poison?

Right now we don't have any folders/canon patterns that produce poison; the rationale is here: https://discourse.llvm.org/t/some-question-on-the-semantics-of-the-arith-dialect/74861/2.

The shift folders should be updated not to fold when the shit amount equals the bitwidth.

[ubfx]

Also any other op affected in a similar way?

This issue #81228 seems to hint to something similar, but I'm still trying to find the root cause.

[kuhar]

LGTM