Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do incorrectly writen XDC files generate errors from the toolchain? #151

Closed
WhiteNinjaZ opened this issue Jun 3, 2021 · 10 comments
Closed

Do incorrectly writen XDC files generate errors from the toolchain? #151

WhiteNinjaZ opened this issue Jun 3, 2021 · 10 comments
Labels
bug Something isn't working

Comments

@WhiteNinjaZ
Copy link
Contributor

WhiteNinjaZ commented Jun 3, 2021 •
edited

@acomodi
I was trying to modify the symbiflow counter test to take an input from one of the switches, and ran into an interesting error where the input pin for a switch was not being routed correctly during the build process. The bit-stream was generated but my project was not functioning as expected (specifically flipping sw[0] had no effect on the counter I was running). After some debugging I realized that the pin constraints for my switch were written slightly differently from the symbiflow example declarations for other I/O pins. Although, my pin deceleration for the switch is supported by Vivado's tool chain apparently it is not recognized by symbiflow. After I changed my switch constraint to match the format of the symbiflow examples, my design worked properly.

My concern is that, there was no warning from the build process indicating that there was a part of my XDC file that was written in a way that was not supported. Since errors caused by xdc files are some of the most difficult to debug, shouldn't this type of error generate some sort of warning or return a fail from the tool chain?

This is my XDC file:

# Clock pin
set_property PACKAGE_PIN W5 [get_ports {clk}]
set_property IOSTANDARD LVCMOS33 [get_ports {clk}]

# Switch (Modified from original example to include sw[0])
set_property -dict { PACKAGE_PIN V17   IOSTANDARD LVCMOS33 } [get_ports { sw }]; 

# LEDs
set_property PACKAGE_PIN U16 [get_ports {led[0]}]
set_property PACKAGE_PIN E19 [get_ports {led[1]}]
set_property PACKAGE_PIN U19 [get_ports {led[2]}]
set_property PACKAGE_PIN V19 [get_ports {led[3]}]
set_property IOSTANDARD LVCMOS33 [get_ports {led[0]}]
set_property IOSTANDARD LVCMOS33 [get_ports {led[1]}]
set_property IOSTANDARD LVCMOS33 [get_ports {led[2]}]
set_property IOSTANDARD LVCMOS33 [get_ports {led[3]}]

# Clock constraints
create_clock -period 10.0 [get_ports {clk}]

This is my modified example code:

module top (
    input clk,
    input sw,
    output [3:0] led
);
  localparam BITS = 4;
  localparam LOG2DELAY = 22;

  wire bufg;
  BUFG bufgctrl (
      .I(clk),
      .O(bufg)
  );

  reg [BITS+LOG2DELAY-1:0] counter = 0;

  always @(posedge bufg) begin
  //This if/else block pauses the state of the leds when switch[0] is high
    if(~sw)
    counter <= counter + 1;
    else 
    counter <= counter;
  end

  assign led[3:0] = counter >> LOG2DELAY;
endmodule

And this is the output from the tool chain.
@issuelabeler issuelabeler bot added the bug Something isn't working label Jun 3, 2021
@mithro
Copy link
Contributor

mithro commented Jun 4, 2021

FYI - @acomodi @tmichalak

@acomodi
Copy link
Contributor

acomodi commented Jun 7, 2021

Hi @WhiteNinjaZ, I think you are right and this kind of behaviour should get caught earlier, at least with a warning. I guess that something might have been reported in the counter_synth.json.log file, which holds all the output from the yosys synthesis and the yosys-plugins. I believe this should also get solved in the XDC and SDC yosys-plugins

@WhiteNinjaZ
Copy link
Contributor Author

WhiteNinjaZ commented Jun 7, 2021 •
edited

@acomodi That is good to know. Out of curiosity, why does symbiflow-examples not use the xdc parser in symbiflow-xc-fasm2bels/fasm2bels/lib/parse_xdc.py? If I understand correctly, it looks like that parser fixes this kind of issue by supplying better error handling as well as providing support for a few other features.

@acomodi
Copy link
Contributor

acomodi commented Jun 7, 2021

The XDC plugin in symbiflow-yosys-plugins does actually more than te one in fasm2bels, as it accesses the netlist information and annotates relevant attributes required for VPR. For instance it binds the pin to the actual physical location, as well as exploring the clock network to annotate all the clock constraints which are output in the .sdc output.

@WhiteNinjaZ
Copy link
Contributor Author

That makes sense. Thank you for the clarification.

@mithro
Copy link
Contributor

mithro commented Jun 7, 2021

@acomodi - We should never be issuing warnings. It should either be something you don't care about or an error.

@nelsobe
Copy link
Contributor

nelsobe commented Jun 7, 2021

@mithro Thanks for the clarification. We thought that silently ignoring an XDC pin mapping due to an XDC file syntax error should result in an error (and it was not really a syntax error - just using a feature the parser doesn't grok). Otherwise, it is nearly impossible to figure out why the circuit doesn't work... It was only a bit of guessing on our part that led us to find the culprit in this case...

@WhiteNinjaZ
Copy link
Contributor Author

WhiteNinjaZ commented Jun 8, 2021 •
edited

@acomodi Is there documentation showing how to install the XDC plugin and merge it with the tool chain? I was looking through the internet and also in the REDMES and couldn't seem to find it. Is the XDC plugin already part of the tool chain you install with symbiflow examples?

@acomodi
Copy link
Contributor

acomodi commented Jun 9, 2021

@WhiteNinjaZ At the moment, the symbiflow-examples pulls all the tools from conda, and those versions are pre-built. I think one option would be to:

  1. build the XDC plugin locally (note that the version of yosys used to build the plugin need to be the same as the one installed in conda)
  2. replace the xdc.so library from conda/envs/xc7/share/yosys/plugins/ with the one locally compiled.
  3. test the flow

In general, it is useful to have a reduced test case to test directly in the yosys-plugins project.

@WhiteNinjaZ
Copy link
Contributor Author

Issue Fixed with Merge #152.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mithro @nelsobe @acomodi @WhiteNinjaZ