TorchToStablehlo constant integer type fixes #1
Conversation
…t types to singless integer element type
| @@ -883,13 +884,29 @@ LogicalResult ConvertAtenOp<ValueTensorLiteralOp>::matchAndRewrite( | |||
|
|
|||
| DenseElementsAttr valueAttr = | |||
| elements.mapValues(builtinTensorElemTy, [&](const APInt &v) { | |||
| return APInt(bitWidth, v.getSExtValue()); | |||
| return APInt(bitWidth, | |||
| bitWidth == 1 ? v.getZExtValue() : v.getSExtValue()); | |||
| @@ -883,13 +884,29 @@ LogicalResult ConvertAtenOp<ValueTensorLiteralOp>::matchAndRewrite( | |||
|
|
|||
| DenseElementsAttr valueAttr = | |||
| elements.mapValues(builtinTensorElemTy, [&](const APInt &v) { | |||
| return APInt(bitWidth, v.getSExtValue()); | |||
| return APInt(bitWidth, | |||
| bitWidth == 1 ? v.getZExtValue() : v.getSExtValue()); | |||
There was a problem hiding this comment.
The more I think about this, the weirder it seems
signed and unsigned +1 share the same bit representation 0b'000....01, so the SignExtend64 function in LLVM actually does convert a +1 into a -1 if the bit width = 1 when using getSExtValue. I think that can be considered a bug in LLVM that we're working around through this change.
There was a problem hiding this comment.
Well, if you have a single bit and you want to consider it a signed integer, then that bit must be the sign. And so you can basically just represent a magnitude-less "positive" or "negative". The issue is that for booleans 1 is true and 0 is false. But for a signed integer 1 as the sign bit means the integer is "negative"
There was a problem hiding this comment.
Ah right, so this 'issue' stems from the fact that, for bit width 1 it is impossible to store a boolean true = +1; and the only possible values are 0b'0 = 0 and 0b'1 = -1.
v.getZExtValue()when copyingAPIntwith bit-width of 1.DenseResourceElementsAttrwith integer element type to signless.