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

[NFCI] Document division and the rational for the handling of divide by zero #6962

Merged
merged 5 commits into from
May 3, 2024

Conversation

darthscsi
Copy link
Contributor

Be explicit that divide by zero is undefined and show examples of how different semantics for source or target languages can be achieved.

docs/Dialects/Comb/RationaleComb.md Show resolved Hide resolved
docs/Dialects/Comb/RationaleComb.md Outdated Show resolved Hide resolved
docs/Dialects/Comb/RationaleComb.md Outdated Show resolved Hide resolved
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks for keeping the rationales up to date. 💯

@fzi-hielscher
Copy link
Contributor

Is there a reason to go straight for undefined behavior on division by zero, instead of just stating that the result is undefined (like the FIRRTL spec does)? IMHO invoking UB - and taking it seriously - puts us on a slippery slope regarding side effects and evaluation rules that we'd rather avoid in comb.

Also, modu / mods should probably be mentioned, too.

@darthscsi
Copy link
Contributor Author

Is there a reason to go straight for undefined behavior on division by zero, instead of just stating that the result is undefined

The main motivation is so that targets which produce runtime-traps in these ops don't need code to be generated to avoid them. VHDL I believe traps on divide by zero, so to implement comb semantics, you would have to codegenerate control flow (not data flow) which avoided the trap.

try undefined values
@darthscsi
Copy link
Contributor Author

Take a look again. I did like what I did for FIRRTL and took @fzi-hielscher 's suggestion to reuse that. The main thing is this precludes simulation-time traps and makes mapping to vhdl potentially more expensive.

Copy link
Contributor

@fzi-hielscher fzi-hielscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the update. I think it makes perfect sense to define division by zero as an arbitrary combinatorial function on the numerator. I guess this also means that division on i0 would produce a well-defined zero after all, since this would be the only possible function (ping #6959 ).

The situation on the VHDL front is actually a bit more nuanced. Assuming we are using the standard numeric_std library, a division by zero should cause an assertion violation of severity 'error'. Going by the 2008 version of the standard, execution is expected to continue after such a violation, but implementations may still chose to terminate.
So basically, we have no clue what is actually going to happen and IMHO we are better off not even trying to reason about non-dataflow semantics in comb. Otherwise, we"ll paint ourselves in a corner regarding the legality of transformations that end up changing the sequence of evaluation. Perhaps the 'sim' dialect could/should handle this?

docs/Dialects/Comb/RationaleComb.md Outdated Show resolved Hide resolved
@darthscsi
Copy link
Contributor Author

. I guess this also means that division on i0 would produce a well-defined zero after all, since this would be the only possible function

there are several: F(i0) = x, f(i0) = z, f(i0) = 0

@darthscsi darthscsi merged commit 79f0f81 into main May 3, 2024
2 checks passed
@darthscsi darthscsi deleted the dev/darthscsi/div0 branch May 3, 2024 19:15
@Moxinilian
Copy link
Member

Moxinilian commented May 3, 2024

. I guess this also means that division on i0 would produce a well-defined zero after all, since this would be the only possible function

there are several: F(i0) = x, f(i0) = z, f(i0) = 0

I'm really not sure about giving x and z semantics to i0. The x and z abstraction works well to say that a given bit is z or x, not really on a whole value. In particular i0 has no bits, so saying its value is x or z seems a bit weird. What meaning would you give to an i0 in being x when i0 carries no information?

This approach to undefined values feels a lot like undef to me and I'm still not sure on whether that's an issue or not.

@maerhart
Copy link
Member

maerhart commented May 4, 2024

This is also a bit unclear to me. The doc says: "return any value in the target's type system". If we assume that iN represents 4-valuedness, then for i1, the only valid return values would be 0, 1, x, and z. Combinatorially, for an N digit value, we have 4^N possible return values, i.e., for i0 it'd be 4^0=1. On discourse, we defined this singleton to be 0.

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

Successfully merging this pull request may close these issues.

5 participants