Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluesim divide-by-zero behavior is inconsistent on arm64 #688

Open
quark17 opened this issue Apr 6, 2024 · 2 comments
Open

Bluesim divide-by-zero behavior is inconsistent on arm64 #688

quark17 opened this issue Apr 6, 2024 · 2 comments
Labels
bluesim Relates to Bluesim

Comments

@quark17
Copy link
Collaborator

quark17 commented Apr 6, 2024

In testsuite/bsc.misc/divmod/ there are tests for division-by-zero behavior in Bluesim and Verilog backends. The Bluesim tests are expected to raise SIGFPE. For wide bit vectors, this is achieved by defining operator/ for the WideData type that Bluesim uses to represent such vectors. Smaller types are represented in Bluesim with uint64_t, uint32_t, uint8_t, and Bluesim relies on the operator for those types to raise SIGFPE. This happens on x86_64 architectures, but does not happen on arm64 (which allows the division to return zero). So on arm64, the behavior is different for small vectors than for wide vectors.

I'm guessing that we probably want to have Bluesim raise SIGFPE on all architectures. We could do that by replacing the / operation with a macro or something, which can be defined differently for arm64 (to check for zero and raise SIGFPE). Alternatively, I guess we could change the operator for WideData to return zero when on ARM, and thus make the behavior on ARM consistent that way (which might have less overhead than guarding every division?). Any opinions?

In the meantime, I'm going to change the testsuite to expect no error on ARM, so that the testsuite doesn't fail. GitHub now offers a macOS VM on arm64 hardware (macos-14) and I want to add a CI job that builds and tests BSC on this, but currently the testsuite has this failure.

FYI, the WideData representation (and operloaded operators) are declared in bsc/src/bluesim/bs_wide_data.h and the implementations are in bsc/src/bluesim/wide_data.cxx. It is the implementation of wide_quot_rem() that raises SIGFPE (line 1221 in wide_data.cxx).

@quark17 quark17 added the bluesim Relates to Bluesim label Apr 6, 2024
@mieszko
Copy link
Collaborator

mieszko commented Apr 6, 2024

I'm guessing that we probably want to have Bluesim raise SIGFPE on all architectures.

Why? That certainly does not respond to anything you'd see in hardware, right?

And it's not just ARM that doesn't trap for integer division. RISC-V returns all bits set, no trap. In PowerPC the result is undefined, no trap. The C standard leaves the result undefined.

Perhaps more relevantly, SystemVerilog defines division by 0 to result is 'x, which if you squint you could argue means undefined in some sense. But either way, no trap.

I think there are two rational things to do: either match Verilog behaviour (which we can't b/c we have no x) or leave the behaviour of things like this explicitly undefined in the language spec. I'd argue for the latter.

@rsnikhil
Copy link
Collaborator

rsnikhil commented Apr 6, 2024

I agree with @mieszko 's suggestion to leave the behaviour of things like this explicitly undefined in the language spec.

But an intermediate point would be to wrap it in something that $displays a warning in simulation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bluesim Relates to Bluesim
Projects
None yet
Development

No branches or pull requests

3 participants