Skip to content

Commit

Permalink
added sed_wor option to verilate function (#40)
Browse files Browse the repository at this point in the history 
Co-authored-by: Anvesh Nookala <[email protected]>
  • Loading branch information
@nookfoo
nookfoo and Anvesh Nookala authored Sep 12, 2024
1 parent 94bc253 commit 21d4ab6
Showing 1 changed file with 53 additions and 1 deletion.
54 changes: 53 additions & 1 deletion cmake/sim/verilator/verilate.cmake
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
function(verilate IP_LIB)
set(OPTIONS "COVERAGE;TRACE;TRACE_FST;SYSTEMC;TRACE_STRUCTS;MAIN")
set(OPTIONS "COVERAGE;TRACE;TRACE_FST;SYSTEMC;TRACE_STRUCTS;SED_WOR;MAIN")
set(ONE_PARAM_ARGS "PREFIX;TOP_MODULE;THREADS;TRACE_THREADS;DIRECTORY;EXECUTABLE_NAME")
set(MULTI_PARAM_ARGS "VERILATOR_ARGS;OPT_SLOW;OPT_FAST;OPT_GLOBAL")

Expand Down Expand Up @@ -71,6 +71,53 @@ function(verilate IP_LIB)
endforeach()

get_ip_rtl_sources(SOURCES ${IP_LIB})

# String replace "wor " with "wire " in TMR files, since Verilator does not support "wor"
# TODO: Remove this if Verilator ever supports "wor"
if(ARG_SED_WOR)
file(MAKE_DIRECTORY ${BINARY_DIR}/sed_wor)
set(MODIFIED_SOURCES "")

foreach(source ${SOURCES})
get_filename_component(source_name ${source} NAME)
if(source_name MATCHES "TMR")
set(output_file "${BINARY_DIR}/sed_wor/${source_name}")
list(APPEND MODIFIED_SOURCES ${output_file})

add_custom_command(
OUTPUT ${output_file}
COMMAND sed "s/wor /wire /g" ${source} > ${output_file}
DEPENDS ${source}
COMMENT "Replacing wor with wire in ${source_name}."
)
else()
list(APPEND MODIFIED_SOURCES ${source})
endif()
endforeach()

# Update sources to use modified sources
set(SOURCES ${MODIFIED_SOURCES})

# Create stamp file for sed command
set(STAMP_FILE "${BINARY_DIR}/sed_wor/${IP_LIB}_sed_wor.stamp")

add_custom_command(
OUTPUT ${STAMP_FILE}
COMMAND /bin/sh -c date > ${STAMP_FILE}
DEPENDS ${MODIFIED_SOURCES}
COMMENT "Generating stamp file after sed commands."
)

add_custom_target(
${IP_LIB}_sed_wor ALL
DEPENDS ${STAMP_FILE}
)

add_dependencies(${IP_LIB} ${IP_LIB}_sed_wor)
# unset, so argument is not further passed to verilator bin
unset(ARG_SED_WOR)
endif()

get_ip_sim_only_sources(SIM_SOURCES ${IP_LIB})
list(PREPEND SOURCES ${SIM_SOURCES})

Expand Down Expand Up @@ -173,6 +220,11 @@ function(verilate IP_LIB)
set(VLT_STATIC_LIB "${DIRECTORY}/lib${ARG_TOP_MODULE}.a")
set(INC_DIR ${DIRECTORY})

# TODO: Remove this if Verilator ever supports "wor"
if(TARGET ${IP_LIB}__sed_wor)
add_dependencies(${VERILATE_TARGET} ${IP_LIB}__sed_wor)
endif()

set(VERILATED_LIB ${IP_LIB}__vlt)
add_library(${VERILATED_LIB} STATIC IMPORTED)
add_dependencies(${VERILATED_LIB} ${VERILATE_TARGET})
Expand Down

4 comments on commit 21d4ab6

@Risto97
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @nookfoo .
Isn't this already implemented in tmrg() function? (https://github.com/HEP-SoC/SoCMake/blob/master/cmake/tmrg/tmrg/tmrg.cmake#L171)

Can you please revert the change and implement it in tmrg if its not there.

@Risto97
Copy link
Contributor

@Risto97 Risto97 commented on 21d4ab6 Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I see now what you wanted. You want to keep wor for implementation but not for simulation.
In that case I think it makes sense to create a new sed_wor SoCMake function, and create a target before simulation target.

I don't want to overcomplicate any of the targets, and verilator target is already quite complex

@nookfoo
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @Risto97, yeah that would be a more clean way to do it. We are working toward the tapeout, apologies for the forcefule merge.

Do you suggest to still revert it? Otherwise, I will make a new PR with an exracted sed_wor function.

@Risto97
Copy link
Contributor

@Risto97 Risto97 commented on 21d4ab6 Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Don't worry now, focus on tapeout.
I will just open an issue tonight so we can change once you have more time.

In any case I think the fact that the tmrg function is applying sed might not be problem for you.
If the structure of triglav is mostly the same as before, your verification and implementation directories are different.
Meaning when you do implementation you configure CMake there, and there is no verilator target.

In that case you can have a variable that will affect if tmrg function applies sed or not, ie. when you do implementation don't apply, when simulation apply...
Something like

if(SIMULATION)
    set(ARG_TMRG_SED_WOR SED_WOR)
endif()

tmrg(${IP} ${ARG_TMRG_SED_WOR)

Please sign in to comment.