/ amaranth Public

# nMigen thinks -5 / 2 = 3#621

Closed
opened this issue Jul 14, 2021 · 7 comments
Closed

# nMigen thinks -5 / 2 = 3 #621

opened this issue Jul 14, 2021 · 7 comments
Labels
Milestone

### BracketMaster commented Jul 14, 2021

nMigen thinks `-5 / 2 = 3` , but verilator disagrees.
I suspect this is because `-5 // 2` evaluates to -3 in Python.

# Commands

```\$python3 div.py
quotient = -3
\$verilator -Wno-lint --cc --exe --build sim_main.cpp div.v
\$obj_dir/Vdiv
quotient = -2```

# FIles

## div.py

```from nmigen import Signal, Module
from nmigen.back import verilog
from nmigen.hdl.ast import signed

## make a module
dividend = Signal(signed(8))
divisor  = Signal(8)
quotient = Signal(signed(8))
counter  = Signal(8)

m = Module()
m.d.comb += quotient.eq(dividend // divisor)
m.d.sync += counter.eq(counter + 1)

## write module to verilog
top = m
with open(f"{__file__[:-3]}.v", "w") as f:
f.write(verilog.convert(top, ports=[dividend, divisor, quotient, counter]))

# simulate module in python
from nmigen.sim import Simulator
sim = Simulator(m)

def process():
yield dividend.eq(-5)
yield divisor.eq(2)
for tick in range(2):
yield

print(f"quotient = {(yield quotient)}")

sim.run()```

## sim_main.cpp

```#include "Vdiv.h"
#include "verilated.h"
#include <iostream>
#include <iomanip>
#include <chrono>
#include <cstdint>

// inspired heavily by code from:
// https://veripool.org/guide/latest/example_cc.html#example-c-execution
int main(int argc, char** argv, char** env) {
VerilatedContext* contextp = new VerilatedContext;
contextp->commandArgs(argc, argv);
Vdiv* top = new Vdiv{contextp};

top->dividend = 5;
top->divisor = -2;

top->clk = 0;
for (int i = 0; i < 20; i++) {
top->clk = ~(top->clk);
top->eval();
}

printf("quotient = %hhd\n", top->quotient);

delete top;
delete contextp;
return 0;
}```

### alanvgreen commented Jul 15, 2021

 It took me a minute or two to work out what is going on here: the same nMigen code evaluates differently in the nmigen simulator than it does in Verilator.

### whitequark commented Jul 15, 2021

 Yes. It's an issue with the emitted Verilog code, I believe; so not specific to Verilator.

### BracketMaster commented Jul 15, 2021

 Obviously up to you @whitequark what is canonical nMigen behavior. But I'd argue that in this case, concerning negative integer division, verilog's behavior seems a little more reasonable to me: ```module arithmetic_operators(); initial begin \$display (" -5 / 2 = %d", -5/2); #10 \$finish; end endmodule``` ` -5 / 2 = -2`

### BracketMaster commented Jul 15, 2021

 negative integer division behavior in Python strikes me as less common

### whitequark commented Jul 15, 2021

 Obviously up to you @whitequark what is canonical nMigen behavior. Consider the three snippets: `Const(-5 / 2)`, `Const(-5) / 2`, and `Const(-5) / Const(2)`. Do you think it is acceptable for any of them to evaluate to different values?

### BracketMaster commented Jul 15, 2021

added this to the 0.3 milestone Dec 11, 2021
closed this as completed in ``` b452e0e ``` Dec 11, 2021

### whitequark commented Dec 11, 2021

 Thanks for the report! This is now fixed by using the upstream Yosys support for flooring division and modulo operators, added in YosysHQ/yosys#1885. As a result, Yosys >=0.10 is now required.

mentioned this issue Feb 3, 2023