chipsalliance / chipsalliance/chisel

Bits UInt type review

Open
#1,743 5 comments 2 reactions 0 assignees View on GitHub
Dominant language
Scala
Stars
4.8k
Forks
658
Avg merge
18h 59m
Merged PRs (30d)
14

Description

These days I did some code review on `Elements` operators.
The original idea was trying to solve the the Intellij highlight issue.
But I encountered some strange designs which need to be recorded and discussed later.

## Question 1: What is `Bits`?
I think `Bits` corresponds to these data structure in [BlueSpec](http://wiki.bluespec.com/Home/Data-Types/Bit--n-), [SMT](http://smtlib.cs.uiowa.edu/theories-FixedSizeBitVectors.shtml)
However when I take a look at operator at `Bits`, there are some strange functions.
```
tail(n: Int) -> UInt
head(n: Int) -> UInt
apply(x: BigInt, y: BigInt) -> UInt
pad(that: Int) -> this.type
##(that: Bits) -> UInt
```
- Firstly, return type of `tail`, `head`, `apply(x: BigInt, y: BigInt)`, `##(that: Bits)` are certainly wrong.
They should returns `Bits`, rather than a `UInt`.
- Secondly, what is `pad(that: Int) -> this.type`?
`this.type` is defined [here](https://www.scala-lang.org/files/archive/spec/2.12/03-types.html#singleton-types)
I don't know why it returns `this.type`, it's kind of dependent type thinking?

## Question 2: `UInt` are used too much?
Basically, I think there exists a customary abuse to `UInt`: using `UInt` as `Bits`. I think if a user need a `UInt`, they are using `+`, `-`, `*`, `/` and other numerical related operators. But these operator only exists in `UInt`, while not exists in `Bits`:
```
&(that: UInt) -> UInt
|(that: UInt) -> UInt
^(that: UInt) -> UInt
orR -> Bool
andR -> Bool
xorR -> Bool
bitSet(off: UInt, dat: Bool) -> UInt
=/=(that: UInt) -> Bool
===(that: UInt) -> Bool
```
All of these operator are not related to number, while they should belong to `Bits`, and even the return type should be `Bits`:
what's the purpose of `(a: UInt | b: UInt) + c: UInt`? It's kind of strange to merge the usage between Bit Vector and UInt.

So, as the result, I think we may need to fix this to provide a more safe and concrete types to users.

## Solution
1. Change the wrong return type in Bits.
1. Add missing operators to Bits.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.