More code review changes

Adding docs that pipelining is for future plumbing work

Fixing sqrt output to be more sensical

Removing magic number from random number test

Fixing error signal for floating point sqrt logic

Alligning names for floating_point_sqrt.dart

Catching and properly reporting error on DeNorm

Updating docs to mention we do not support DeNorms

fixing fixed sqrt test

Author: sweeksBigBlue

doc/components/fixed_point.md

@@ -28,4 +28,4 @@ This component converts an 8-bit floating-point (FP8) representation ([FloatingP
 
 ## FixedPointSqrt
 
-This component computes the square root of a 3.x fixed-point value, returning a result in the same format. The square root value is rounded to the ordered number of bits. The integral part must be 3 bits, and the fractional part may be any odd value <= 51. Even numbers of bits are currently not supported, integral bits in numbers other than 3 are currently not supported.
+This component computes the square root of a 3.x fixed-point value, returning a result in the same format. The square root value is rounded to the ordered number of bits. The integral part must be 3 bits, and the fractional part may be any odd value <= 51. Even numbers of bits are currently not supported, integral bits in numbers other than 3 are currently not supported.

doc/components/floating_point.md

@@ -86,7 +86,7 @@ A second [FloatingPointAdderRound](https://intel.github.io/rohd-hcl/rohd_hcl/Flo
 ## FloatingPointSqrt
 
 A very basic [FloatingPointSqrtSimple] component is available which does not perform any
-rounding. It also only operates on variable mantissas of an odd value (1,3,5,etc) but these odd mantissas can be of variable length up to 51. It takes one
+rounding and does not support DeNorm numbers. It also only operates on variable mantissas of an odd value (1,3,5,etc) but these odd mantissas can be of variable length up to 51. It takes one
 [FloatingPoint](https://intel.github.io/rohd-hcl/rohd_hcl/FloatingPoint-class.html) [LogicStructure](https://intel.github.io/rohd/rohd/LogicStructure-class.html) and
 performs a square root on it, returning the [FloatingPoint](https://intel.github.io/rohd-hcl/rohd_hcl/FloatingPoint-class.html) value on the output.
 

lib/src/arithmetic/fixed_sqrt.dart

@@ -23,9 +23,9 @@ abstract class FixedPointSqrtBase extends Module {
   late final FixedPoint a;
 
   /// getter for the computed output.
-  late final FixedPoint sqrtF = a.clone(name: 'sqrtF')..gets(output('sqrtF'));
+  late final FixedPoint sqrt = a.clone(name: 'sqrt')..gets(output('sqrt'));
 
-  /// Square root a fixed point number [a], returning result in [sqrtF].
+  /// Square root a fixed point number [a], returning result in [sqrt].
   FixedPointSqrtBase(FixedPoint a,
       {super.name = 'fixed_point_square_root', String? definitionName})
       : width = a.width,
@@ -34,7 +34,7 @@ abstract class FixedPointSqrtBase extends Module {
                 definitionName ?? 'FixedPointSquareRoot${a.width}') {
     this.a = a.clone(name: 'a')..gets(addInput('a', a, width: a.width));
 
-    addOutput('sqrtF', width: width);
+    addOutput('sqrt', width: width);
   }
 }
 
@@ -62,8 +62,8 @@ class FixedPointSqrt extends FixedPointSqrtBase {
     subtractionValue = Const(0, width: aLoc.width);
     aLoc = [Const(0), a, Const(0)].swizzle();
 
-    final outputSqrt = a.clone(name: 'sqrtF');
-    output('sqrtF') <= outputSqrt;
+    final outputSqrt = a.clone(name: 'sqrt');
+    output('sqrt') <= outputSqrt;
 
     // loop once through input value
     for (var i = 0; i < ((width + 2) >> 1); i++) {

lib/src/arithmetic/floating_point/floating_point_sqrt.dart

@@ -6,8 +6,8 @@
 //
 // 2025 March 3
 // Authors: James Farwell <james.c.farwell@intel.com>,
-//Stephen Weeks <stephen.weeks@intel.com>,
-//Curtis Anderson <curtis.anders@intel.com>
+//          Stephen Weeks <stephen.weeks@intel.com>,
+//          Curtis Anderson <curtis.anders@intel.com>
 
 import 'package:meta/meta.dart';
 import 'package:rohd/rohd.dart';
@@ -23,16 +23,19 @@ abstract class FloatingPointSqrt<FpType extends FloatingPoint> extends Module {
 
   /// The [clk] : if a non-null clock signal is passed in, a pipestage is added
   /// to the square root to help optimize frequency.
+  /// Plummed for future work for pipelining, currently unsupported.
   @protected
   late final Logic? clk;
 
   /// Optional [reset], used only if a [clk] is not null to reset the pipeline
   /// flops.
+  /// Plummed for future work for pipelining, currently unsupported.
   @protected
   late final Logic? reset;
 
   /// Optional [enable], used only if a [clk] is not null to enable the pipeline
   /// flops.
+  /// Plummed for future work for pipelining, currently unsupported.
   @protected
   late final Logic? enable;
 
@@ -41,16 +44,16 @@ abstract class FloatingPointSqrt<FpType extends FloatingPoint> extends Module {
   late final FpType a;
 
   /// getter for the computed [FloatingPoint] output.
-  late final FloatingPoint sqrtR = (a.clone(name: 'sqrtR') as FpType)
-    ..gets(output('sqrtR'));
+  late final FloatingPoint sqrt = (a.clone(name: 'sqrt') as FpType)
+    ..gets(output('sqrt'));
 
   /// getter for the [error] output.
   late final Logic error = Logic(name: 'error')..gets(output('error'));
 
   /// The internal error signal to pass through
   late final Logic errorSig;
 
-  /// Square root a floating point number [a], returning result in [sqrtR].
+  /// Square root a floating point number [a], returning result in [sqrt].
   /// - [clk], [reset], [enable] are optional inputs to control a pipestage
   /// (only inserted if [clk] is provided)
   FloatingPointSqrt(FpType a,
@@ -71,10 +74,8 @@ abstract class FloatingPointSqrt<FpType extends FloatingPoint> extends Module {
     this.a = (a.clone(name: 'a') as FpType)
       ..gets(addInput('a', a, width: a.width));
 
-    addOutput('sqrtR', width: exponentWidth + mantissaWidth + 1);
-    errorSig = Logic(name: 'error');
+    addOutput('sqrt', width: exponentWidth + mantissaWidth + 1);
     addOutput('error');
-    output('error') <= errorSig;
   }
 
   /// Pipelining helper that uses the context for signals clk/enable/reset

lib/src/arithmetic/floating_point/floating_point_sqrt_simple.dart

@@ -16,7 +16,7 @@ import 'package:rohd_hcl/rohd_hcl.dart';
 class FloatingPointSqrtSimple<FpType extends FloatingPoint>
     extends FloatingPointSqrt<FpType> {
   /// Square root one floating point number [a], returning results
-  /// [sqrtR] and [error]
+  /// [sqrt] and [error]
   FloatingPointSqrtSimple(super.a,
       {super.clk,
       super.reset,
@@ -28,14 +28,17 @@ class FloatingPointSqrtSimple<FpType extends FloatingPoint>
     final outputSqrt = FloatingPoint(
         exponentWidth: exponentWidth,
         mantissaWidth: mantissaWidth,
-        name: 'sqrtR');
-    output('sqrtR') <= outputSqrt;
+        name: 'sqrt');
+    output('sqrt') <= outputSqrt;
+    late final error = output('error');
 
     // check to see if we do sqrt at all or just return a
     final isInf = a.isAnInfinity.named('isInf');
     final isNaN = a.isNaN.named('isNan');
     final isZero = a.isAZero.named('isZero');
-    final enableSqrt = ~((isInf | isNaN | isZero) | a.sign).named('enableSqrt');
+    final isDeNormal = ~a.isNormal.named('isDenorm');
+    final enableSqrt =
+        ~((isInf | isNaN | isZero | isDeNormal) | a.sign).named('enableSqrt');
 
     // debias the exponent
     final deBiasAmt = (1 << a.exponent.width - 1) - 1;
@@ -44,8 +47,9 @@ class FloatingPointSqrtSimple<FpType extends FloatingPoint>
     final deBiasExp = a.exponent - deBiasAmt;
 
     // shift exponent
-    final shiftedExp =
-        [deBiasExp[-1], deBiasExp.slice(a.exponent.width - 1, 1)].swizzle();
+    final shiftedExp = [deBiasExp[-1], deBiasExp.slice(a.exponent.width - 1, 1)]
+        .swizzle()
+        .named('deBiasExp');
 
     // check if exponent was odd
     final isExpOdd = deBiasExp[0];
@@ -61,7 +65,7 @@ class FloatingPointSqrtSimple<FpType extends FloatingPoint>
 
     // mux to choose if we do square root or not
     final fixedSqrt = aFixedAdj.clone()
-      ..gets(mux(enableSqrt, FixedPointSqrt(aFixedAdj).sqrtF, aFixedAdj)
+      ..gets(mux(enableSqrt, FixedPointSqrt(aFixedAdj).sqrt, aFixedAdj)
           .named('sqrtMux'));
 
     // convert back to floating point representation
@@ -70,14 +74,14 @@ class FloatingPointSqrtSimple<FpType extends FloatingPoint>
 
     // final calculation results
     Combinational([
-      errorSig < Const(0),
+      error < Const(0),
       If.block([
         Iff(isInf & ~a.sign, [
           outputSqrt < outputSqrt.inf(),
         ]),
         ElseIf(isInf & a.sign, [
           outputSqrt < outputSqrt.inf(negative: true),
-          errorSig < Const(1),
+          error < Const(1),
         ]),
         ElseIf(isNaN, [
           outputSqrt < outputSqrt.nan,
@@ -89,7 +93,13 @@ class FloatingPointSqrtSimple<FpType extends FloatingPoint>
         ]),
         ElseIf(a.sign, [
           outputSqrt < outputSqrt.nan,
-          errorSig < Const(1),
+          error < Const(1),
+        ]),
+        ElseIf(isDeNormal, [
+          outputSqrt.sign < a.sign,
+          outputSqrt.exponent < a.exponent,
+          outputSqrt.mantissa < a.mantissa,
+          error < Const(1),
         ]),
         Else([
           outputSqrt.sign < a.sign,

test/arithmetic/fixed_sqrt_test.dart

@@ -45,7 +45,7 @@ void main() {
         fixed.put(FixedPointValue.ofDouble(test,
             signed: fixed.signed, m: fixed.m, n: fixed.n));
 
-        final fpvResult = dut.sqrtF.fixedPointValue;
+        final fpvResult = dut.sqrt.fixedPointValue;
 
         final fpvExpected = FixedPointValue.ofDouble(sqrt(test),
             signed: fixed.signed, m: fixed.m, n: fixed.n);

test/arithmetic/floating_point/floating_point_sqrt_test.dart

@@ -52,7 +52,7 @@ void main() {
         final Logic expError = Const(0);
 
         fp.put(fv);
-        final fpOut = sqrtT.sqrtR;
+        final fpOut = sqrtT.sqrt;
         final eOut = sqrtT.error;
         expect(fpOut.floatingPointValue, equals(expSqrt),
             reason: '\t${fp.floatingPointValue} '
@@ -132,7 +132,7 @@ void main() {
 
         fp.put(fv);
 
-        final compResult = sqrtDUT.sqrtR;
+        final compResult = sqrtDUT.sqrt;
         final compError = sqrtDUT.error;
 
         final expResult = FloatingPointValue.populator(
@@ -158,13 +158,14 @@ void main() {
   test('FP: random number sqrt', () {
     const exponentWidth = 5;
     const mantissaWidth = 9;
+    final systemTestIter = pow(2, exponentWidth) * pow(2, mantissaWidth);
 
     final fp = FloatingPoint(
         exponentWidth: exponentWidth, mantissaWidth: mantissaWidth);
 
     final sqrtDUT = FloatingPointSqrtSimple(fp);
     final rand = Random(513);
-    for (var i = 0; i < 4096; i++) {
+    for (var i = 0; i < systemTestIter; i++) {
       final fv = FloatingPointValue.populator(
               exponentWidth: exponentWidth, mantissaWidth: mantissaWidth)
           .random(rand, normal: true);
@@ -176,7 +177,7 @@ void main() {
           fp.sign.value.toBool()) {
         continue;
       }
-      final compResult = sqrtDUT.sqrtR;
+      final compResult = sqrtDUT.sqrt;
       final compError = sqrtDUT.error;
 
       final expResult = FloatingPointValue.populator(