You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PECOS's QASM classical expressions have no representation of signedness: values are bare BitVecs and every operation guesses sign from the bit pattern (resize_to_same_width sign-extends any multi-bit value from its MSB, bitvec::divide treats both MSBs as signs, as_i64 reinterprets any pattern as two's complement, is_negative_expression looks only at the outermost AST node). #864 is the comparison symptom and its PR fixes comparisons and assignment widening under the rule "registers are unsigned; only an explicitly negated expression is negative", using expression shape for signedness. Everything below is the rest of the same confusion and needs one deliberate design (value-carried signedness) rather than more shape heuristics.
Reproduced on dev at 86cd2b1 (through sim_builder, 4-bit registers unless stated)
b = 15; a = b / 2; gives a = 0. bitvec::divide reads b's MSB as a sign and divides -1 by 2. (Expected 7.)
Found by review, not yet reproduced (read the cited code before acting)
Unary minus keeps the operand's width: negating a 2-bit unsigned 11 yields 01 (+1), not -3 (bitvec_expression.rsevaluate_unary_op, arithmetic.rs::negate). The negation width, and the policy for negating the signed minimum, are undefined.
as_i64 on an unsigned value with its top bit set returns a negative number. Shift counts use it, so shifting by a register holding 3 (2-bit 11) clamps the count to 0. WASM arguments use it too. Right shift is logical; whether a signed right shift exists is undefined.
analyze_comparison treats "outermost node is unary minus" as "negative", so -0 < 0 is true, --1 > 0 is false, and -1 < (-2 + 0) is true. The shortcut runs before operands are evaluated, in both runtime evaluation and constant folding.
Constant folding complements a literal at its parsed width while runtime zero-extends to the default width first, so ~1 folds to 14 but evaluates to 254 at default width 8.
Mixed signed/unsigned bitwise operations: 15 & -1 at width 4 is a signed 1111 (-1) and 15 ^ -1 a signed 0 under any propagation rule; whatever the rule, it needs tests.
Indexed assignment c[0] = 2 stores 1 (truthiness), not the low bit 0.
Signedness and width of RNG and WASM function results, and of the void-result placeholder, are unstated; a function result compared directly is not supported by the recursive evaluator.
Carry signedness on the evaluated value (ExpressionValue::BitVec { bits, signed } or a signed variant), produced only by unary minus, propagated through operators, and consumed by widening, division, shifts and the i64 conversion. Registers are unsigned. Define negation width, division rounding, shift semantics and the conversion overflow policy in one place and test each with a bit-exact table. Delete the MSB-based heuristics (resize_to_same_width's sign guess, is_negative_expression's AST-shape guess) once the value carries the answer.
The review that produced this list is on the #864 PR.
Summary
PECOS's QASM classical expressions have no representation of signedness: values are bare
BitVecs and every operation guesses sign from the bit pattern (resize_to_same_widthsign-extends any multi-bit value from its MSB,bitvec::dividetreats both MSBs as signs,as_i64reinterprets any pattern as two's complement,is_negative_expressionlooks only at the outermost AST node). #864 is the comparison symptom and its PR fixes comparisons and assignment widening under the rule "registers are unsigned; only an explicitly negated expression is negative", using expression shape for signedness. Everything below is the rest of the same confusion and needs one deliberate design (value-carried signedness) rather than more shape heuristics.Reproduced on
devat 86cd2b1 (throughsim_builder, 4-bit registers unless stated)b = 15; a = b / 2;gives a = 0.bitvec::dividereads b's MSB as a sign and divides -1 by 2. (Expected 7.)creg a[2]; creg b[4]; a = 3; b = a;gives b = 15 (fixed by the QASM if conditions sign-extend multi-bit registers, so d == 3 is false when d holds 3 #864 PR for the assignment path, listed here for the record).c= 1:if (c > 0)is false (fixed by the QASM if conditions sign-extend multi-bit registers, so d == 3 is false when d holds 3 #864 PR).Found by review, not yet reproduced (read the cited code before acting)
11yields01(+1), not -3 (bitvec_expression.rsevaluate_unary_op,arithmetic.rs::negate). The negation width, and the policy for negating the signed minimum, are undefined.as_i64on an unsigned value with its top bit set returns a negative number. Shift counts use it, so shifting by a register holding 3 (2-bit11) clamps the count to 0. WASM arguments use it too. Right shift is logical; whether a signed right shift exists is undefined.analyze_comparisontreats "outermost node is unary minus" as "negative", so-0 < 0is true,--1 > 0is false, and-1 < (-2 + 0)is true. The shortcut runs before operands are evaluated, in both runtime evaluation and constant folding.~1folds to 14 but evaluates to 254 at default width 8.15 & -1at width 4 is a signed1111(-1) and15 ^ -1a signed 0 under any propagation rule; whatever the rule, it needs tests.c[0] = 2stores 1 (truthiness), not the low bit 0.qasm_to_phir.rstypes everycregasi64whileqasm_to_phir_json.rstypes itu32/u64(QASM-to-PHIR lowerings disagree on the classical register type: i64 in the Rust path, u32/u64 in the JSON path #868).Proposed shape
Carry signedness on the evaluated value (
ExpressionValue::BitVec { bits, signed }or a signed variant), produced only by unary minus, propagated through operators, and consumed by widening, division, shifts and thei64conversion. Registers are unsigned. Define negation width, division rounding, shift semantics and the conversion overflow policy in one place and test each with a bit-exact table. Delete the MSB-based heuristics (resize_to_same_width's sign guess,is_negative_expression's AST-shape guess) once the value carries the answer.The review that produced this list is on the #864 PR.