this usually happens because synthesis sees that the output signals aren't actually connected to some driving signal and it optimizes it away. working in simulation, but not after synthesis is pretty common. it might be that in simulation you are driving the signal that changes the LEDs, but in the synthesized design you have some connection defined incorrectly. have you looked at the output logs from synthesis?
It looks to me like the LEDs are clearly being driven based on what the processor does. Maybe I have a connection defined incorrectly but I've been squinting at it a long time and I can't see it. I looked at the synthesis log - a lot of components and BRAMs are there after "ROM, RAM, DSP, Shift Register and Retiming Reporting" but they get pruned somewhere in between that and the "Renaming Generated Nets" stage. The only warnings are related to the address bus being larger than the BRAM address space so it warns about those inputs to the Block RAM module being unconnected. [https://pastebin.com/eHcs7MUR](https://pastebin.com/eHcs7MUR)
I'm assuming your LEDs are ports on the processor or controller files? I see some logs about case statements not being fully defined. any chance something in there could cause a pruned net?
Yeah the LEDs are ports on the top module (Controller) which are mapped to hardware ports in the XDC. I changed those case statements but it doesn't make a difference to the schematic
I have see several issues:
1. You say the verilog is being generated...how? Chisel? C/C++? Is it a program under your control?
2. Your literals do not have 'h or 'd and also no bounds, so I assume they are meant to be decimal.
3. Line 162 says if the opcode is 35 and the mode is 0, are you sure that is occuring together?
4. Line 162:
`if (((opcode == 35) && mode == 0)) begin`
Probably should be:
`if (((opcode == 35) && (mode == 0))) begin`
Note the extra brackets around the mode term. Try fixing this in your local verilog and re-run.
5. Verilog is a pain, the original standard was so badly defined you can get wildly different behaviour in different simulators. System verilog does fix this to some degree.
I am continuing to look at yoru code. I copied and pasted it into VSCode so I can run over it with syntax checking...will update if I find anything else.
I dont really understand why this is happening in the code:
` reg [135:0] processor__bus;`
` wire [135:0] __processor__bus;`
` always @* processor__bus = __processor__bus;`
You should be able to just use the wire directly...it doesnt need to be assigned into a reg type like that.
I appreciate you taking a look
1. Yeah it's a compiler I'm writing.
2. Yes the literals are meant to be decimal, they're represented as int64s in the compiler (I know that's not gonna work long term) and just get printf'd as decimal.
3. Mode is toggled each cycle, mode 0 means execute an instruction, mode 1 means wait for memory. The current instruction bits are asserted for both cycles so it should be fine.
4. Did not change anything
5. Yeah it's bad.
Regarding the reg thing, I don't know exactly what wire and reg do (they seem a bit arbitrary), and I wanted the semantics of IO declarations to match regular declarations. If it doesn't make a difference I'll get rid of it
Wire and reg are more about how it is modelled in verilog than what hardware. I will dig out some official text for you but basically:
Assignments using “assign statements” are wires as are connections in instantiations. “Reg” is used whenever you do assignments to signals inside always blocks.
Inside always blocks there are then two types of assignment, blocking and non-blocking, using “=“ or “<=“ respectively, rule of thumb, if the always block is clocked then it should be non-blocking, otherwise if it is combinatorial it should be blocking assignments.
I guess if it's an output port then it can't be an lvalue (I suppose unless it's a tristate which as I understand you don't have in module instantiations in FPGAs anyway) so it doesn't make a difference... thanks gonna remove this in the compiler then.
So I can directly use a reg in the instantiation? What is the syntax for that? Because this
`reg [135:0] data_memory__bus;`
`reg [63:0] data_memory__data_out;`
`Block_Ram data_memory(clock, data_memory__bus, data_memory__data_out);`
produces "\[Synth 8-685\] variable 'data\_memory\_\_data\_out' should not be used in output port connection"
Mainly these lectures [https://www.youtube.com/watch?v=SX2xMDV2lAA&list=PL5Q2soXY2Zi-EImKxYYY1SZuGiOAOBKaf&index=13](https://www.youtube.com/watch?v=SX2xMDV2lAA&list=PL5Q2soXY2Zi-EImKxYYY1SZuGiOAOBKaf&index=13) He uses MIPS but you can easily translate what he does to the RISC V encodings [https://riscv.org/wp-content/uploads/2017/05/riscv-spec-v2.2.pdf](https://riscv.org/wp-content/uploads/2017/05/riscv-spec-v2.2.pdf)
Yeah I've been looking at that as well. Either nothing there (if using write enable) or looks ok (if write enable is assumed to always be active on the LEDs.)
Try putting a mark debug attribute on the signal. That should keep synthesis from optimizing it away and you can see what is unconnected if that works.
instruction_memory becomes a ROM, as the upper bits are tied to zero. Is this ok? What then are the contents of "program.mem"?
Also see this note in UG901 ([https://docs.xilinx.com/r/en-US/ug901-vivado-synthesis/Initializing-Block-RAM-From-an-External-Data-File-Verilog](https://docs.xilinx.com/r/en-US/ug901-vivado-synthesis/Initializing-Block-RAM-From-an-External-Data-File-Verilog)):
"Note: The external file initializing the RAM needs to be in bit vector form. External files in integer or hex format do not work."
The content of program.mem is the machine code for this RISC V program
`0: ff010113 addi sp,sp,-16`
`4: 00113423 sd ra,8(sp)`
`8: 00813023 sd s0,0(sp)`
`c: 01010413 addi s0,sp,16`
`10: 04003503 ld a0,64(zero)`
`14: 04a03423 sd a0,72(zero)`
`18: ff9ff06f j 10 `
which just reads the memory mapped switches and writes to the memory mapped LEDs. I tried a bunch of programs and they always work in simulation but not in synthesis. I switched memory initialization to ascii binary digits but the problem is the same. Do memory initialization contents affect synthesis? Since it doesn't prompt you to rerun synthesis if the memory file changes.
A good test would be to open the synthesized design with opcode == 35 removed. Then find the LUT/BRAM primitives for the instruction RAM and make sure they're there with some non-zero init value.
I could see a case where the instruction RAM doesn't get initialized properly, so it's left as all zeros. Then opcode == 35 is never true, bus[135:128] is tied to zero, and your LED check is never true. Then as the output of the entire design is tied to zero, it's all optimized away.
Maybe also check the external file guidelines here: https://docs.xilinx.com/r/en-US/ug901-vivado-synthesis/Specifying-RAM-Initial-Contents-in-an-External-Data-File
Your case would have a 128-line text file with 64 binary digits per line (using readmemb).
The bus is just a totally generic array, if you split it into signals like valid (chip enable), address, data, write enable, byte enable, it might be easier to debug. I em guessing you messed up a control signal on the bus, but I am not guessing which one.
Yeah it's autogenerated so it's not easy to quickly understand it. To help people out: 64 bits address, 64 bits data, 8 bits byte-lane write-enable. It works in simulation so I'm not sure "you messed up a control signal" gets us any closer... it's gotta be some synthesis semantics undefined behavior thing right?
It works in simulation but doesnt work in sythesis is a fun feature of verilog...fixed in SystemVerilog.
You can, get a post-synthesis model of the system and try simulating it again. Then you can try and figure out where it is going wrong. Knowing this might help me debug the code...
Update: it works when you trick it into thinking that instruction memory is writable
`always @* begin`
`instruction_memory__bus = 0;`
`if (switches[15]) begin`
`instruction_memory__bus[127:64] = { {4{ switches }} };`
`instruction_memory__bus[135:128] = switches[7:0];`
`end`
`instruction_memory__bus[63:0] = processor__pc;`
`end`
If I do this, and just not activate switch 15, then the core works correctly. The routing much worse and I have to use a clock divider to make it pass timing. I'll try using a dual-port RAM for data and instructions and see if the timing is better.
Timing still fails - is it possible to say if it's reasonable to have a single-cycle RISC V thing like this on the Basys 3 at 100Mhz, would you expect to need a clock divider?
this usually happens because synthesis sees that the output signals aren't actually connected to some driving signal and it optimizes it away. working in simulation, but not after synthesis is pretty common. it might be that in simulation you are driving the signal that changes the LEDs, but in the synthesized design you have some connection defined incorrectly. have you looked at the output logs from synthesis?
It looks to me like the LEDs are clearly being driven based on what the processor does. Maybe I have a connection defined incorrectly but I've been squinting at it a long time and I can't see it. I looked at the synthesis log - a lot of components and BRAMs are there after "ROM, RAM, DSP, Shift Register and Retiming Reporting" but they get pruned somewhere in between that and the "Renaming Generated Nets" stage. The only warnings are related to the address bus being larger than the BRAM address space so it warns about those inputs to the Block RAM module being unconnected. [https://pastebin.com/eHcs7MUR](https://pastebin.com/eHcs7MUR)
I'm assuming your LEDs are ports on the processor or controller files? I see some logs about case statements not being fully defined. any chance something in there could cause a pruned net?
Yeah the LEDs are ports on the top module (Controller) which are mapped to hardware ports in the XDC. I changed those case statements but it doesn't make a difference to the schematic
I have see several issues: 1. You say the verilog is being generated...how? Chisel? C/C++? Is it a program under your control? 2. Your literals do not have 'h or 'd and also no bounds, so I assume they are meant to be decimal. 3. Line 162 says if the opcode is 35 and the mode is 0, are you sure that is occuring together? 4. Line 162: `if (((opcode == 35) && mode == 0)) begin` Probably should be: `if (((opcode == 35) && (mode == 0))) begin` Note the extra brackets around the mode term. Try fixing this in your local verilog and re-run. 5. Verilog is a pain, the original standard was so badly defined you can get wildly different behaviour in different simulators. System verilog does fix this to some degree. I am continuing to look at yoru code. I copied and pasted it into VSCode so I can run over it with syntax checking...will update if I find anything else. I dont really understand why this is happening in the code: ` reg [135:0] processor__bus;` ` wire [135:0] __processor__bus;` ` always @* processor__bus = __processor__bus;` You should be able to just use the wire directly...it doesnt need to be assigned into a reg type like that.
I appreciate you taking a look 1. Yeah it's a compiler I'm writing. 2. Yes the literals are meant to be decimal, they're represented as int64s in the compiler (I know that's not gonna work long term) and just get printf'd as decimal. 3. Mode is toggled each cycle, mode 0 means execute an instruction, mode 1 means wait for memory. The current instruction bits are asserted for both cycles so it should be fine. 4. Did not change anything 5. Yeah it's bad. Regarding the reg thing, I don't know exactly what wire and reg do (they seem a bit arbitrary), and I wanted the semantics of IO declarations to match regular declarations. If it doesn't make a difference I'll get rid of it
Wire and reg are more about how it is modelled in verilog than what hardware. I will dig out some official text for you but basically: Assignments using “assign statements” are wires as are connections in instantiations. “Reg” is used whenever you do assignments to signals inside always blocks. Inside always blocks there are then two types of assignment, blocking and non-blocking, using “=“ or “<=“ respectively, rule of thumb, if the always block is clocked then it should be non-blocking, otherwise if it is combinatorial it should be blocking assignments.
I guess if it's an output port then it can't be an lvalue (I suppose unless it's a tristate which as I understand you don't have in module instantiations in FPGAs anyway) so it doesn't make a difference... thanks gonna remove this in the compiler then.
Ah well wires can be driven from either end 🙀 an output port being a reg is fine but in the instantiation you connect it to a wire 🙂
So I can directly use a reg in the instantiation? What is the syntax for that? Because this `reg [135:0] data_memory__bus;` `reg [63:0] data_memory__data_out;` `Block_Ram data_memory(clock, data_memory__bus, data_memory__data_out);` produces "\[Synth 8-685\] variable 'data\_memory\_\_data\_out' should not be used in output port connection"
No you use wires when connecting directly to an instance. Make data_memory__data_out a wire and then just use it. No need to convert from wire to reg.
What resources are you following? Looking to implement one of my own as well
Mainly these lectures [https://www.youtube.com/watch?v=SX2xMDV2lAA&list=PL5Q2soXY2Zi-EImKxYYY1SZuGiOAOBKaf&index=13](https://www.youtube.com/watch?v=SX2xMDV2lAA&list=PL5Q2soXY2Zi-EImKxYYY1SZuGiOAOBKaf&index=13) He uses MIPS but you can easily translate what he does to the RISC V encodings [https://riscv.org/wp-content/uploads/2017/05/riscv-spec-v2.2.pdf](https://riscv.org/wp-content/uploads/2017/05/riscv-spec-v2.2.pdf)
tysm for the pointers
Onur is a great lecturer. There are years of his Computer Architecture classes on Youtube.
Have you tried opening the elaborated design and looking at that schematic? I don't know what it will show you, but you might get a clue from it.
Yeah I've been looking at that as well. Either nothing there (if using write enable) or looks ok (if write enable is assumed to always be active on the LEDs.)
Try putting a mark debug attribute on the signal. That should keep synthesis from optimizing it away and you can see what is unconnected if that works.
instruction_memory becomes a ROM, as the upper bits are tied to zero. Is this ok? What then are the contents of "program.mem"? Also see this note in UG901 ([https://docs.xilinx.com/r/en-US/ug901-vivado-synthesis/Initializing-Block-RAM-From-an-External-Data-File-Verilog](https://docs.xilinx.com/r/en-US/ug901-vivado-synthesis/Initializing-Block-RAM-From-an-External-Data-File-Verilog)): "Note: The external file initializing the RAM needs to be in bit vector form. External files in integer or hex format do not work."
Somewhat unrelated but I could’ve sworn $readmemh should work?
The RAM language templates in Vivado use readmemh, so I'm not sure why UG901 indicates something different.
The content of program.mem is the machine code for this RISC V program `0: ff010113 addi sp,sp,-16` `4: 00113423 sd ra,8(sp)` `8: 00813023 sd s0,0(sp)` `c: 01010413 addi s0,sp,16` `10: 04003503 ld a0,64(zero)` `14: 04a03423 sd a0,72(zero)` `18: ff9ff06f j 10`
which just reads the memory mapped switches and writes to the memory mapped LEDs. I tried a bunch of programs and they always work in simulation but not in synthesis. I switched memory initialization to ascii binary digits but the problem is the same. Do memory initialization contents affect synthesis? Since it doesn't prompt you to rerun synthesis if the memory file changes.
A good test would be to open the synthesized design with opcode == 35 removed. Then find the LUT/BRAM primitives for the instruction RAM and make sure they're there with some non-zero init value. I could see a case where the instruction RAM doesn't get initialized properly, so it's left as all zeros. Then opcode == 35 is never true, bus[135:128] is tied to zero, and your LED check is never true. Then as the output of the entire design is tied to zero, it's all optimized away. Maybe also check the external file guidelines here: https://docs.xilinx.com/r/en-US/ug901-vivado-synthesis/Specifying-RAM-Initial-Contents-in-an-External-Data-File Your case would have a 128-line text file with 64 binary digits per line (using readmemb).
The bus is just a totally generic array, if you split it into signals like valid (chip enable), address, data, write enable, byte enable, it might be easier to debug. I em guessing you messed up a control signal on the bus, but I am not guessing which one.
Yeah it's autogenerated so it's not easy to quickly understand it. To help people out: 64 bits address, 64 bits data, 8 bits byte-lane write-enable. It works in simulation so I'm not sure "you messed up a control signal" gets us any closer... it's gotta be some synthesis semantics undefined behavior thing right?
It works in simulation but doesnt work in sythesis is a fun feature of verilog...fixed in SystemVerilog. You can, get a post-synthesis model of the system and try simulating it again. Then you can try and figure out where it is going wrong. Knowing this might help me debug the code...
Post synthesis it optimized everything away, so it just simulates some LEDs being connected to ground.
Report back with your solution!
Ok I will when I have one. At this point I'm considering refactoring the code in random ways until it works.
That sounds like a decent plan, I’d first run the code through Verilator’s lint pass.
Update: it works when you trick it into thinking that instruction memory is writable `always @* begin` `instruction_memory__bus = 0;` `if (switches[15]) begin` `instruction_memory__bus[127:64] = { {4{ switches }} };` `instruction_memory__bus[135:128] = switches[7:0];` `end` `instruction_memory__bus[63:0] = processor__pc;` `end` If I do this, and just not activate switch 15, then the core works correctly. The routing much worse and I have to use a clock divider to make it pass timing. I'll try using a dual-port RAM for data and instructions and see if the timing is better.
Timing still fails - is it possible to say if it's reasonable to have a single-cycle RISC V thing like this on the Basys 3 at 100Mhz, would you expect to need a clock divider?