[MLIR] Fix printing of switch case for negative value

[Jezurko]

This patch fixes the printer for the llvm.switch operation with negative values in a case.

The previous behaviour printed the value as an unsigned integer, as the getLimitedValue() returns unsigned value. This caused the roundtrip to fail (assertion in APInt), as the printed unsigned integer could not be parsed into the same amount of bits in a signed integer.
I don't see a good reason for keeping any restriction on the printed value, as LLVMIR switch afaik does not have a limit on the bitwidth of the values and APInt handles printing just fine.

[llvmbot]

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Robert Konicar (Jezurko)

Changes

This patch fixes the printer for the llvm.switch operation with negative values in a case.

The previous behaviour printed the value as an unsigned integer, as the getLimitedValue() returns unsigned value. This caused the roundtrip to fail (assertion in APInt), as the printed unsigned integer could not be parsed into the same amount of bits in a signed integer.
I don't see a good reason for keeping any restriction on the printed value, as LLVMIR switch afaik does not have a limit on the bitwidth of the values and APInt handles printing just fine.

---

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

2 Files Affected:

• (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+1-1)
• (modified) mlir/test/Dialect/LLVMIR/roundtrip.mlir (+4-2)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index ccf8f72b2b230..fb9236fcc640d 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -634,7 +634,7 @@ static void printSwitchOpCases(OpAsmPrinter &p, SwitchOp op, Type flagType,
       llvm::zip(caseValues, caseDestinations),
       [&](auto i) {
         p << "  ";
-        p << std::get<0>(i).getLimitedValue();
+        p << std::get<0>(i);
         p << ": ";
         p.printSuccessorAndUseList(std::get<1>(i), caseOperands[index++]);
       },
diff --git a/mlir/test/Dialect/LLVMIR/roundtrip.mlir b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
index 09a0cd57e2675..e0a17308af828 100644
--- a/mlir/test/Dialect/LLVMIR/roundtrip.mlir
+++ b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
@@ -144,12 +144,14 @@ func.func @ops(%arg0: i32, %arg1: f32,
   // CHECK:      llvm.switch %0 : i32, ^[[BB3]] [
   // CHECK-NEXT:   1: ^[[BB4:.*]],
   // CHECK-NEXT:   2: ^[[BB5:.*]],
-  // CHECK-NEXT:   3: ^[[BB6:.*]]
+  // CHECK-NEXT:   3: ^[[BB6:.*]],
+  // CHECK-NEXT:   -3: ^[[BB6:.*]]
   // CHECK-NEXT: ]
   llvm.switch %0 : i32, ^bb3 [
     1: ^bb4,
     2: ^bb5,
-    3: ^bb6
+    3: ^bb6,
+    -3: ^bb6
   ]
 
 // CHECK: ^[[BB3]]

[gysit]

Thanks for the fix!

LGTM

As the parser expects a signed integer the printer should also print an signed integer.

[Jezurko]

Thanks! @gysit Could you please merge the PR on my behalf? :) I do not have write access