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.

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

Inout can't be read with constant value #4370

Open
alchitry opened this issue May 3, 2024 · 3 comments
Open

Inout can't be read with constant value #4370

alchitry opened this issue May 3, 2024 · 3 comments

Comments

@alchitry
Copy link

alchitry commented May 3, 2024

Version

Yosys 0.40+45 (git sha1 dd21955, g++ 13.2.1 -march=x86-64 -mtune=generic -O2 -fstack-protector-strong -fPIC -Os)

On which OS did this happen?

Linux

Reproduction Steps

Here's a minimal example.

module MI_alchitryTop (
    output reg [7:0] P_led,
    inout [4:0] P_button
  );
  
  reg [4:0] IO_P_button;
  assign P_button = IO_P_button;
  
  always @* begin
    IO_P_button = 5'bzzzzz;
    P_led = P_button;
  end
  
endmodule

I'm targeting the iCE40HX8K-CB132

Expected Behavior

The P_button signal to be read and output on P_led.

Actual Behavior

The P_button signal is treated as a constant and the LEDs never turn on.

@alchitry alchitry added the pending-verification This issue is pending verification and/or reproduction label May 3, 2024
@KrystalDelusion
Copy link
Member

I don't have any experience with using inout ports or tristate logic, but having a quick play with the show command and stepping through the synth command (cough cough) it looks like any semblance of tristate logic is being dropped as soon as it reaches proc. Looking at a few examples online for tristate logic it seems like you need to assign it with conditional logic, i.e. assign P_button = P_button ? P_button : 'bz; for it to correctly identify the tristate logic, rather than simply writing z's because then it will optimise to always read back z's (which, again you can see with show). This is probably also why your other issue, #4371, works with IO_P_button = D_flip_q ? 5'bzzzzz : 5'h0;.

@alchitry
Copy link
Author

alchitry commented May 6, 2024

For what it's worth, this works correctly in both iCEcube2 and Vivado.

I understand this case is kind of pointless since the inout is always z but it should act just like an input at that point and have its value driven externally.

@KrystalDelusion
Copy link
Member

I agree, but given that read_verilog does report back Warning: Yosys has only limited support for tri-state logic at the moment. for the relevant line, I think this is more of a feature request than a bug.

@KrystalDelusion KrystalDelusion added feature-request and removed pending-verification This issue is pending verification and/or reproduction labels May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants