Implement floating-point Fast Fourier Transform by ethanuppal · Pull Request #210 · intel/rohd-hcl

ethanuppal · 2025-06-09T17:57:34Z

Description & Motivation

TODO

Related Issue(s)

Testing

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

mkorbel1 · 2025-06-09T20:58:34Z

lib/src/arithmetic/signals/floating_point_logics/complex_floating_point_logic.dart

+        name: name,
+      );
+
+  ComplexFloatingPoint adder(ComplexFloatingPoint other) =>


why not override operator + and *?

I don't want to hide the fact that it's expensive

the names of the functions seem unintuitive to me, maybe plus and times or addedTo and multipliedBy or something?

if we wanted to stay more consistent with other parts of the library, these could be their own components (classes, modules) e.g. ComplexFloatingPointMultiplier, perhaps with an argument that allows you to choose your own FloatingPointMultiplier implementation for the internals.

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

mkorbel1 · 2025-06-09T21:10:55Z

lib/src/arithmetic/floating_point/fft/butterfly.dart

+      required this.inB,
+      required this.twiddleFactor,
+      super.name = 'butterfly'}) {
+    addInput('inA', inA, width: inA.width);


heads up, you'll want to use the signal returned by addInput or input for internal computation rather than the one provided in the constructor

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

mkorbel1 · 2025-06-10T15:48:05Z

lib/src/arithmetic/signals/floating_point_logics/complex_floating_point_logic.dart

+        name: name,
+      );
+
+  ComplexFloatingPoint adder(ComplexFloatingPoint other) =>


the names of the functions seem unintuitive to me, maybe plus and times or addedTo and multipliedBy or something?

if we wanted to stay more consistent with other parts of the library, these could be their own components (classes, modules) e.g. ComplexFloatingPointMultiplier, perhaps with an argument that allows you to choose your own FloatingPointMultiplier implementation for the internals.

mkorbel1 · 2025-06-10T15:48:43Z

lib/src/arithmetic/floating_point/fft/fft_stage.dart

+import 'package:rohd_hcl/src/arithmetic/floating_point/fft/butterfly.dart';
+import 'package:rohd_hcl/src/arithmetic/signals/floating_point_logics/complex_floating_point_logic.dart';
+
+class BadFFTStage extends Module {


why "Bad"? :)

It's not fully pipelined

maybe "multi-cycle" is a better name or something (if that's what it is)?

mkorbel1 · 2025-06-10T15:50:10Z

lib/src/arithmetic/floating_point/fft/fft_stage.dart

+      required this.twiddleFactorROM,
+      super.name = 'badfftstage'})
+      : assert(ready.width == 1) {
+    addInput('clk', clk);


reminder: you'll want to do something like clk = addInput('clk', clk); instead of just taking this.clk as the argument, since the signals received as arguments to the constructor are not the input ports of this module.

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

mkorbel1 · 2025-08-12T21:47:11Z

lib/src/arithmetic/floating_point/fft/butterfly.dart

@@ -0,0 +1,35 @@
+// Copyright (C) 2025 Intel Corporation
+// SPDX-License-Identifier: BSD-3-Clause


please update headers on the files to match the recommended style, including the name of the file, date authored, purpose of the file, etc.

mkorbel1 · 2025-08-12T21:47:44Z

test/arithmetic/floating_point/bad_fft_stage_test.dart

probably worth naming this something better than bad?

mkorbel1 · 2025-08-12T21:47:59Z

test.dart

delete this file?

mkorbel1 · 2025-08-12T21:48:45Z

lib/src/arithmetic/floating_point/fft/fft_stage.dart

+
+  late final Logic done;
+
+  BadFFTStage({


please do write doc comments

mkorbel1 · 2025-08-12T21:49:01Z

lib/src/arithmetic/floating_point/fft/fft_stage.dart

remember to export components that are intended to be part of the public API for ROHD-HCL

mkorbel1 · 2025-08-12T21:49:23Z

lib/src/arithmetic/floating_point/fft/fft_stage.dart

Please add user guide descriptions on how to use new components in the doc/ directory (including links to the pages)

mkorbel1 · 2025-08-12T21:50:28Z

test/arithmetic/floating_point/butterfly_test.dart

+  return complex;
+}
+
+class Complex {


this class is defined in multiple files?

Is there a place for shared utilities in tests?

you can create a directory in either lib (not exported) or in the test directory and can reference other files that way

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

Add bit reversal component for 1D arrays

87ca92b

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

ethanuppal mentioned this pull request Jun 9, 2025

Implement fixed-point Fast Fourier Transform #207

Closed

Add complex floating point module

492150c

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

mkorbel1 reviewed Jun 9, 2025

View reviewed changes

Add butterfly module

b5e4895

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

mkorbel1 reviewed Jun 9, 2025

View reviewed changes

WIP butterfly stage

cc2059c

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

mkorbel1 reviewed Jun 10, 2025

View reviewed changes

ethanuppal added 12 commits June 18, 2025 15:47

backup

e2463a1

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

Fix butterfly test

468abe4

Fix typo in butterfly and add tags to connectIO

51e4e7c

Confused?

f2ee651

Remove extraneous file

7712bf8

For good measure

0b8c3bf

backup

7481def

Fix weird error

823c57b

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

Fix memory harness

f88400b

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

Rework interface

f634b18

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

switch to connectIntf

972fcb1

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

Finish fft test

2537fcb

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

mkorbel1 reviewed Aug 12, 2025

View reviewed changes

ethanuppal marked this pull request as ready for review August 14, 2025 22:47

ethanuppal added 4 commits August 14, 2025 16:48

print stuff

7c1ad04

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

stuff

dffb187

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

Merge remote-tracking branch 'upstream/main' into fft2

ca5422f

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

ok

bc4f828

Signed-off-by: Ethan Uppal <113849268+ethanuppal@users.noreply.github.com>

		@@ -0,0 +1,35 @@
		// Copyright (C) 2025 Intel Corporation
		// SPDX-License-Identifier: BSD-3-Clause

Conversation

ethanuppal commented Jun 9, 2025

Description & Motivation

Related Issue(s)

Testing

Backwards-compatibility

Documentation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants