Skip to content

Inconsistent results for mlir arith shift operations #80960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
AnonymousBugreporter1 opened this issue Feb 7, 2024 · 2 comments
Closed

Inconsistent results for mlir arith shift operations #80960

AnonymousBugreporter1 opened this issue Feb 7, 2024 · 2 comments
Assignees

Comments

@AnonymousBugreporter1
Copy link

I have the following MLIR program:
test.mlir:

func.func @func1() {
  %true = arith.constant true
  %false = arith.constant false

  %c1_i64 = arith.constant 1 : i64

  %17 = scf.if %true -> (i1) {
    scf.yield %true : i1
  } else {
    scf.yield %false : i1 // this line can not be changed to %true
  }
  
  // can be changed to arith.shli, arith.shrsi and arith.shrui
  %22 = arith.shrui %17, %true : i1

  %30 = scf.if %true -> (i64) {  // this scf.if can not be removed
    scf.yield %c1_i64 : i64
  } else {
    scf.yield %c1_i64 : i64   // but can yield same value.
  }

  vector.print %22 : i1
  return
}

The above MLIR program can be lowered and executed using the following commands:

mlir-opt --convert-scf-to-cf --convert-cf-to-llvm --convert-vector-to-llvm --convert-arith-to-llvm --convert-func-to-llvm test.mlir > 1.mlir

mlir-translate --mlir-to-llvmir 1.mlir > 2.ll

sed s/@func1/@main/ 2.ll > 3.ll

bin/clang 3.ll lib/libmlir_c_runner_utils.so lib/libmlir_float16_utils.so lib/libmlir_runner_utils.so /usr/lib/gcc/x86_64-linux-gnu/9/libstdc++.so -o out.out

./out.out

However, it has inconsistent results over multiple runs.
Is there some problem in the above arith shift operations?
My git version is adbf21f.

@github-actions github-actions bot added the mlir label Feb 7, 2024
@ubfx
Copy link
Member

ubfx commented Feb 17, 2024

There is an unfortunate inconsistency between the llvm Support lib (APInt::lshr()), the LLVMIR instruction lshr and the arith dialect Operation lshr (and probably the same for the other shift operations although I didn't check:
The llvm instruction doesn't allow shift amount = bitwidth, however both the arith Operation and the llvm Support lib allow it and define the result as zero.

So your code gets lowered to undefined behavior even though the arith dialect should theoretically allow it.
I will try to figure out a way to fix this.

@ubfx ubfx self-assigned this Feb 17, 2024
ubfx added a commit that referenced this issue Feb 18, 2024
… amounts (#82133)

This patch aligns the shift Ops in `arith` with respective LLVM instructions.
Specifically, shifting by an amount equal to the bitwidth of the operand
is now defined to return poison.

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

ubfx commented Feb 19, 2024

We have now updated the documentation to align it with the underlying LLVM instruction, which means that a shift amount equal to the bitwidth of the operand will return poison. This doesn't "fix" the issue that the results are inconsistent but it explains why these inconsistent results are to be expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants