[PATCH] D79753: [mlir][StandardToSPIRV] Fix signedness issue in bitwidth emulation.

[Han-Chung Wang via Phabricator via llvm-commits]

hanchung marked 3 inline comments as done.
hanchung added inline comments.


================
Comment at: mlir/lib/Conversion/StandardToSPIRV/ConvertStandardToSPIRV.cpp:665
+
+  // If the number is negative, we should do a sign extention.
+  IntegerAttr shiftValueAttr =
----------------
hanchung wrote:
> It seems that we should check the signedness semantics of dstType?
> 
> This looks like we should apply following sign extension if `srcType.isSignedInteger()` returns true, and shouldn't apply it if `srcType.isUnsignedInteger()` returns true. In case of it is signless, ie `srcType.isSignlessInteger()` returns true, I don't know what should we do. It seems an undef to me.
See commit message for the explanation.


================
Comment at: mlir/lib/Conversion/StandardToSPIRV/ConvertStandardToSPIRV.cpp:203-204
+    if (isUnsignedOp<SPIRVOp>() && dstType != operation.getType()) {
+      operation.emitError(
+          "bitwidth emunation is not implemented yet on unsigned op");
+    }
----------------
I'd like to return a failure with this statement, but it would cause an error and I don't know how to fix it in the test.

```
'std.divi_unsigned' op requires the same type for all operands and results

 note: see current operation: %0 = "std.divi_unsigned"(%arg0, %arg1) : (i32, i32) -> i8
```

Does anyone have a better thought here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79753