Calyx Binary Floating Point AddF Operator

[jiahanxie353]

This patch stacks on #7086 .

Add support for floating point addition in Calyx

[rachitnigam]

This is interesting. I think we keep having to add ops because we don't have a good mechanism to talk about extern ops in the dialect. @cgyurgyik do you have thoughts on how best to encode external ops in the dialect?

[cgyurgyik]

This is interesting. I think we keep having to add ops because we don't have a good mechanism to talk about extern ops in the dialect. @cgyurgyik do you have thoughts on how best to encode external ops in the dialect?

By "extern", are you talking about an op that is defined outside of the Calyx standard primitive libary? Don't we provide support for this à la HW dialect?

[rachitnigam]

@cgyurgyik exactly! We should build support for something like this in the Calyx dialect. A tricky thing is that we want to specify paths to Verilog files which implement the specific extern we're talking about. The upside would be that we don't have to keep adding new primitives to CIRCT ops definition (especially because we're not defining any optimizations over them).

[cgyurgyik]

@cgyurgyik exactly! We should build support for something like this in the Calyx dialect. A tricky thing is that we want to specify paths to Verilog files which implement the specific extern we're talking about. The upside would be that we don't have to keep adding new primitives to CIRCT ops definition (especially because we're not defining any optimizations over them).

Got it. Would this back us into a corner from ever defining optimizations over them? I could imagine a world where Calyx would allow the following,

mul = std_mul_pipe(32);

...

@promotable(3) group do_mul {
  mul.lhs = reg.out;
  mul.rhs = 32'd3;
  ...
}

Could be rewritten to:

mul = std_mul_const(32);

...

static<1> group do_mul {
  mul.lhs = reg.out;
  mul.rhs = 32'd3;
}

These "peephole" optimizations would be easier to implement with different Operations and their typed input/outputs. (It isn't really peephole, since it would require some whole program analysis to ensure we aren't breaking some other constraints.)

Edit: I guess this also encompasses user-defined primitives, that may not be readily optimized by a Calyx compiler.

[rachitnigam]

@cgyurgyik finally catching up to your comment. This is a good question about which ops we want to perform peephole optimizations on. In general, I think we should define them only on "simple" operations like addition. Something like a floating-point operation should probably be implemented as an external because the most optimized version is going to be some black-box RTL anyways.

(FWIW, this argument should also extend to really large adders! For example, it is not obvious to me that a 128-bit adder is best implemented automatically. For now, however, we'll leave this dragon sleeping.)

@jiahanxie353 How critical is this PR in the stack that you're trying to get approved? For example, if it is not yet super critical, I would lean towards redesigning this to use externs instead so we can integrate with the HardFloat library.

[jiahanxie353]

A tricky thing is that we want to specify paths to Verilog files which implement the specific extern we're talking about. The upside would be that we don't have to keep adding new primitives to CIRCT ops definition

I would lean towards redesigning this to use externs instead so we can integrate with the HardFloat library.

I believe I'm using externs, because it emits import float/addFN here, which in turn uses extern here

Am I missing anything? Are you saying we should completely get rid of AddFNOp in TableGen here? I don't see an obvious way to introduce floating point add/mul without adding new ops.

[rachitnigam]

Am I missing anything? Are you saying we should completely get rid of AddFNOp in TableGen here? I don't see an obvious way to introduce floating point add/mul without adding new ops.

Yeah, @andrewb1999 has been working on a idea for defining operator libraries that might eventually allow us to decouple the specification of new primitives from new ops in the compiler. For now, this seems fine.

One nit about the new operator definitions (that you can address in a follow-on PR if it's a bigger change). Instead of calyx.addFN, we should be saying something like calyx.ieee754.fadd to make it explicit that this is not only a floating-point adder, but specifically an IEEE-754 compliant adder.

[jiahanxie353]

@andrewb1999 has been working on a idea for defining operator libraries that might eventually allow us to decouple the specification of new primitives from new ops in the compiler

sounds very neat, looking forward to it

One nit about the new operator definitions (that you can address in a follow-on PR if it's a bigger change). Instead of calyx.addFN, we should be saying something like calyx.ieee754.fadd to make it explicit that this is not only a floating-point adder, but specifically an IEEE-754 compliant adder.

Agree. I'll address when there's a need in the future.

For now, I'll click the button if it looks good

[cgyurgyik]

Some small nits, otherwise LGTM