Other bug is that in the `Toplevel` module, the port of instantiated hardware is different:

[zzy666666zzy]

          Other bug is that in the `Toplevel` module, the port of instantiated hardware is different:

The ports' name of the my hardware module is:

  input logic [31:0] ext_mem0_read_data,
  input logic ext_mem0_done,
  input logic [31:0] ext_mem1_read_data,
  input logic ext_mem1_done,
  input logic [31:0] ext_mem2_read_data,
  input logic ext_mem2_done,
  input logic clk,
  input logic reset,
  input logic go,
  output logic [31:0] ext_mem0_write_data,
  output logic [3:0] ext_mem0_addr0,
  output logic ext_mem0_write_en,
  output logic [31:0] ext_mem1_write_data,
  output logic [3:0] ext_mem1_addr0,
  output logic ext_mem1_write_en,
  output logic [31:0] ext_mem2_write_data,
  output logic [3:0] ext_mem2_addr0,
  output logic ext_mem2_write_en,
  output logic done

In the top level, the instantiated ports' name is:

        .clk(ap_clk),
        .done(kernel_done),
        .ext_mem0__addr0(ext_mem0__addr0),
        .ext_mem0__clk(),
        .ext_mem0__done(ext_mem0__done),
        .ext_mem0__read_data(ext_mem0__read_data),
        .ext_mem0__write_data(ext_mem0__write_data),
        .ext_mem0__write_en(ext_mem0__write_en),
        .ext_mem1__addr0(ext_mem1__addr0),
        .ext_mem1__clk(),
        .ext_mem1__done(ext_mem1__done),
        .ext_mem1__read_data(ext_mem1__read_data),
        .ext_mem1__write_data(ext_mem1__write_data),
        .ext_mem1__write_en(ext_mem1__write_en),
        .ext_mem2__addr0(ext_mem2__addr0),
        .ext_mem2__clk(),
        .ext_mem2__done(ext_mem2__done),
        .ext_mem2__read_data(ext_mem2__read_data),
        .ext_mem2__write_data(ext_mem2__write_data),
        .ext_mem2__write_en(ext_mem2__write_en),
        .go(kernel_start),
        .reset(reset || memories_sent)

There are two underscores __. So the Toplevel cannot be synthesized as well.

Originally posted by @zzy666666zzy in https://github.com/cucapra/calyx/issues/1470#issuecomment-1567362877

[rachitnigam]

Not clear if this is necessarily fixable since the discover-external pass finds the prefix by matching with memories. The current fix is mentioned here: https://github.com/cucapra/calyx/pull/1534

[rachitnigam]

@zzy666666zzy can you confirm whether the solution works for you or not? Not sure if there is anything actionable for this issue currently

[zzy666666zzy]

Hi @rachitnigam ,

Thanks for solving the doubled underscore issue.

For the instantiated main module, ext_mem*_clk input clocks are not connected to 'ap_clk' signals. But I know these clock signals are connected inside the module.

To synthesis it without errors, could you modify the compiler source code to make ext_mem*_clk connect to ap_clk.

Many thanks.

[rachitnigam]

The compiler doesn't care about the name of the of the clk port. It only cares about the attribute. If you want a main component with ap_clk, you can write something like:

component main(@clk ap_clk: 1, ...) -> (...) { ... }

This will make the compiler thread the ap_clk signal to the memories

[rachitnigam]

@zzy666666zzy did the above fix for the ap_clk work for you?

[zzy666666zzy]

Hi @rachitnigam ,

Sorry for replying you late. gemm4x4_external.txt

I manually change the @clk clk: 1 with @clk ap_clk: 1,at line 74 in the attached file. And use the command

$PATH_calyx/calyx -l $Calyx_PATH -b xilinx --synthesis --disable-verify --nested $CURRENT_PATH/gemm4x4_external.mlir -o $CURRENT_PATH/gemm4x4_wrapper.v

Unfortunately ext_mem*_clk signals are still not connected with ap_clk at the output gemm4x4_wrapper.v file.

[rachitnigam]

Okay! That seems like a bug. I'll investigate!

[rachitnigam]

@zzy666666zzy okay, looks like this is a problem coming from #1056. If you look at the output of running the -p externalize pass which is responsible for externalizing the memories in Calyx, we see that the _clk and _reset ports of the memory are added to the signature:

calyx -p externalize <file> -m file

Generates the output:

import "primitives/binary_operators.futil";
import "primitives/core.futil";
component main<"toplevel"=1>(...) -> (
  ..., ext_mem0_clk: 1, ext_mem0_reset: 1, 
  ..., ext_mem1_clk: 1, ext_mem1_reset: 1, 
  ... ext_mem2_clk: 1, ext_mem2_reset: 1) { ... }

This is problematic because we probably don't want the component to provide the clk and reset signals to the memory. Instead, it should be the test harness that connects the memories with the component that should be in charge of handling these signals.

I'm going to change the compiler to not add these ports to the component when a cell is externalized and change the AXI generator to not add it either. Once done, I'll ping the thread so you can try out the solution.

(Also paging @sampsyo to get a sanity check on this)

[rachitnigam]

(Sorry, the correct issue in question is #1034)

[rachitnigam]

@zzy666666zz the changes have been merged. Can you try updating the repository and see if that works for you?

[zzy666666zzy]

Hi @rachitnigam ,

Thanks for your time investigate this issue. I have rebuilt the latest repo and successfully remove the unused clock ports.

    main kernel_inst (
        .clk(ap_clk),
        .done(kernel_done),
        .ext_mem0_addr0(ext_mem0_addr0),
        .ext_mem0_done(ext_mem0_done),
        .ext_mem0_read_data(ext_mem0_read_data),
        .ext_mem0_write_data(ext_mem0_write_data),
        .ext_mem0_write_en(ext_mem0_write_en),
        .ext_mem1_addr0(ext_mem1_addr0),
        .ext_mem1_done(ext_mem1_done),
        .ext_mem1_read_data(ext_mem1_read_data),
        .ext_mem1_write_data(ext_mem1_write_data),
        .ext_mem1_write_en(ext_mem1_write_en),
        .ext_mem2_addr0(ext_mem2_addr0),
        .ext_mem2_done(ext_mem2_done),
        .ext_mem2_read_data(ext_mem2_read_data),
        .ext_mem2_write_data(ext_mem2_write_data),
        .ext_mem2_write_en(ext_mem2_write_en),
        .go(kernel_start),
        .reset(reset || memories_sent)
    );

Thanks.

[rachitnigam]

Perfect! I'm going to close this and work on #1576 next!