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

Final changes for BlackParrot integration #7

Open
wants to merge 12 commits into
base: black-parrot
Choose a base branch
from

Conversation

Shashank-Vijay
Copy link
Collaborator

No description provided.

@Shashank-Vijay Shashank-Vijay requested a review from Jbalkind July 8, 2021 23:36
@@ -2900,6 +2901,9 @@ sub parse_args
#
push (@{$opt{sim_run_args}}, "-gui") if ($opt{gui}) ;
push (@{$opt{vcs_build_args}}, "-debug_all") if ($opt{debug_all}) ;
push (@{$opt{vcs_build_args}}, "-debug_pp") if ($opt{debug_pp}) ;
push (@{$opt{vcs_build_args}}, "-CFLAGS \"-I/mnt/users/ssd1/homes/svijay97/BlackParrot/black-parrot-sdk/include -std=c++14\"") ;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change the path to a generic one

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do. It's also not being conditionally added?

@@ -33,7 +33,7 @@ import pyhplib
import os
from pyhplib import *

if PITON_ARIANE:
if PITON_BLACKPARROT:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably just be the PITON_RV64_PLATFORM macro, right?

@@ -1066,8 +1066,10 @@ ciop_fake_iob ciop_fake_iob(

<%
text = r'''
.spc0_inst_done (`PITON_CORE0_INST_DONE),
.pc_w0 (`PITON_CORE0_PC_W0),
`ifndef DISABLE_ALL_MONITORS

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remove the indent on this so it's a 2 line patch here

// .uart_timeout_en(uart_timeout_en),
.uart_timeout_en ( 1'b1 ),
.uart_timeout_en(uart_timeout_en),
// .uart_timeout_en ( 1'b1 ),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the original for this? Would be good if this weren't changed versus upstream

@@ -58,6 +58,9 @@ set_false_path -from [get_ports trst_ni]
set_max_delay -datapath_only -from [get_clocks -include_generated_clocks chipset_clk_clk_mmcm] -to [get_clocks tck_i] 15.000
set_max_delay -datapath_only -from [get_clocks tck_i] -to [get_clocks -include_generated_clocks chipset_clk_clk_mmcm] 15.000

# Retiming
set_property BLOCK_SYNTH.RETIMING 1 [get_cells -hierarchical *pipe_fma*]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't end up helping much, right? Should we keep this out and wait for the manually retimed FPU?

@@ -27,6 +27,7 @@
# create mem.image
#riscv64-unknown-elf-objcopy --reverse-bytes 4 -I elf32-littleriscv -O binary diag.exe diag.o
${RV64_TARGET_TRIPLE}-objcopy -I elf64-littleriscv -O binary diag.exe diag.o
cp diag.exe prog.elf

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for?

@@ -25,7 +25,7 @@
# Format:
# BlockID BlockPath Supported Board,Frequency(MHz),DDRSize(Mbytes)
piton_aws ../../build/f1/piton_aws/design f1,62.5,4096
system . vc707,60,1024;genesys2,66.667,1024;nexysVideo,30,512;vcu118,100,2048;xupp3r,60,32768
system . vc707,60,1024;genesys2,25,1024;nexysVideo,30,512;vcu118,100,2048;xupp3r,60,32768

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not excited about this - do we have a sense of the manually retimed FPU's timing?

@@ -2676,6 +2676,7 @@ sub parse_args
'gui!',
'log_all!',
'debug_all!',
'debug_pp!',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this one vs debug_all? Can you just use it via -vcs_build_args=-debug_pp rather than adding a new flag?

@@ -2899,7 +2901,11 @@ sub parse_args

#
push (@{$opt{sim_run_args}}, "-gui") if ($opt{gui}) ;
push (@{$opt{vcs_build_args}}, "-assert svaext") ;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this needed for? It's not conditionally added

@@ -2900,6 +2901,9 @@ sub parse_args
#
push (@{$opt{sim_run_args}}, "-gui") if ($opt{gui}) ;
push (@{$opt{vcs_build_args}}, "-debug_all") if ($opt{debug_all}) ;
push (@{$opt{vcs_build_args}}, "-debug_pp") if ($opt{debug_pp}) ;
push (@{$opt{vcs_build_args}}, "-CFLAGS \"-I/mnt/users/ssd1/homes/svijay97/BlackParrot/black-parrot-sdk/include -std=c++14\"") ;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do. It's also not being conditionally added?

,.frd_data_i(scheduler.fwb_pkt_cast_i.rd_data)
);
`endif // ENABLE_COSIM

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work?

Jbalkind and others added 6 commits October 4, 2021 00:48
Base and size configurable with parameters (using defaults for now)
Compile with -config_rtl=PITON_SIM_MEMORY
Use by taking compiled diag.o and running:
xxd -g 8 -c 64 diag.o | awk '{printf("%s%s%s%s%s%s%s%s\n", $9, $8, $7, $6, $5, $4, $3, $2);}' > sim_memory.memh
sim_memory.memh should be put in the same location that mem.image would normally live
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants