(~True) instead of (not True) can break a design

Issue by whitequark Wednesday Jul 24, 2019 at 20:28 GMT Originally opened as https://github.com/m-labs/nmigen/issues/159

I just discovered this bug in some oMigen code:

--- bad/software/glasgow/gateware/i2c.py
+++ good/software/glasgow/gateware/i2c.py
@@ -142,7 +142,7 @@ class I2CMaster(Module):
                 If(stb,
                     NextValue(pin_o, 1)
                 ).Elif(pin_o == 1,
-                    If(~stretch | (pin_i == 1),
+                    If((not stretch) | (pin_i == 1),
                         NextState(next_state),
                         *exprs
                     )

It translates to this Verilog:

--- good.v      2019-07-24 20:20:50.179566232 +0000
+++ bad.v       2019-07-24 20:21:05.291656753 +0000
@@ -1190,7 +1190,7 @@
                                i2cmastersubtarget_sda_o_i2cmaster_next_value_ce4 <= 1'd1;
                        end else begin
                                if ((i2cmastersubtarget_sda_o == 1'd1)) begin
-                                       if ((1'd1 | (i2cmastersubtarget_sda_i == 1'd1))) begin
+                                       if ((1'sd1 | $signed({1'd0, (i2cmastersubtarget_sda_i == 1'd1)}))) begin
                                                i2cmaster_next_state <= 2'd3;
                                        end
                                end

Given that (n)Migen signals can be negated only with ~ and booleans can be negated only with not, this seems to be a potential source of serious bugs. Unfortunately I'm not sure what can be done to fix it (since by the time the value is coerced to an nMigen constant, its boolean-ness is long lost), and nMigen is indeed still susceptible to this issue:

>>> C(~True)
(const 2'sd-2)
>>> C(~True) | (1==0)
(| (const 2'sd-2) (const 1'd0))
>>> (C(~True) | (1==0)).shape()
(2, True)

amaranth-lang / amaranth

(~True) instead of (not True) can break a design #160