Thorough DSP Tests about dsptools HOT 14 OPEN

@shunshou Thanks I'll work my way through these

from dsptools.

@shunshou seems to me that UInt ring stuff should error on subtraction. Firrtl Spec says that UInt - UInt => SInt but Ring says minus returns UInt. So following, where io.a and io.b are UInts

  val regSubWrap = RegNext(DspContext.withOverflowType(Wrap) { io.a - io.b })

gives

firrtl.passes.CheckTypes$InvalidConnect:  @[Reg.scala 14:44:@23.4]: [module OverflowTypeCircuit]  Type mismatch. Cannot connect regSubGrow to _T_16.

Seems like a better error up front, saying to cast to SInt would be more reasonable

from dsptools.

This is maybe where @grebe states his opinion. Inside the Ring (so the user doesn't have to do it), we can do an explicit cast back to UInt. I think the types should frankly stay consistent w/ their input types (no surprises!).

from dsptools.

@shunshou @grebe Something like this then?

  override def minus(f: UInt, g: UInt): UInt = {
    val diff = context.overflowType match {
      case Grow => f.asSInt -& g.asSInt
      case Wrap => f.asSInt -% g.asSInt
      case _ => throw DspException("Saturating subtractor hasn't been implemented")
    }
    ShiftRegister(diff.asUInt, context.numAddPipes)
  }

from dsptools.

I'm fine with that. @grebe?

from dsptools.

@shunshou @grebe So with above logic subtracting 4-bit UInts from each other gives

15 - 1 => 30

Is that ok?

from dsptools.

Oh for wrap it should be trimmed back to 4 bits. I thought that that was the whole point of -% ?

from dsptools.

Is there no -% on UInt that returns a 4 bit UInt (or SInt that should then be cast as UInt)? b/c if so, IMO, that functionality isn't "Wrap"

from dsptools.

Also, re -&, what should be the behavior? That's one of those weird cases where, to always be mathematically correct, you should convert to an SInt... Or in that case, should you only make guarantees on outputs when they're >=0? @grebe ?

from dsptools.

Sorry, I fell very behind on my github mentions. I think we agreed to throw an exception for -&

from dsptools.

@shunshou So what is
io.sAbsWrap := DspContext.withOverflowType(Wrap) { io.sIn.abs() }
Supposed to do for an SInt(4.W) with value -8

from dsptools.

I believe it'll be -8, right? Wrap has 0 guarantees on mathematical correctness, and is just intended to do whatever the standard bit operation does. You'd have a separate function that might pull it to 7 or something. Or maybe Wrap is a bad descriptor? @grebe, opinions?

from dsptools.

from dsptools.

Reiterating what I said in the new issue -- test that a sequence of typeclass ops handles dspcontext correctly (i.e. does some op, b/c of the way it's implemented, make you lose the Real/Ring, etc. context).

from dsptools.