harvard-acc / ALADDIN

A pre-RTL, power-performance model for fixed-function accelerators
Other
161 stars 54 forks source link

Write CMake scripts #6

Closed rgly closed 8 years ago

rgly commented 8 years ago

The CMake scripts finds and build with LLVM-Tracer. It adds unit-test and SHOC/ C examples as CTest and supports CMake options from LLVM-Tracer. I like CTest for it supports regular expression to choose the wanted tests to run.

Note that the in-source-build is forbidden and BUILD_ON_SOURCE=FALSE by default in Aladdin.

ysshao commented 8 years ago

Thanks for putting together the cmake scripts for Aladdin. It looks really nice. One comment I have here is whether we can separate the entire design space run for all the benchmarks from ctest. Can we only do the unit-test run instead of running everything there?

rgly commented 8 years ago

CTest is the test driver of CMake. By default it runs all tests, but actually CTest supports regular expression to pick the wanted tests.

Here are some tests available, with test commands:

ctest name command
triad triad-instrumented
triad_p2_u16_P2_1ns triad.sh 2 16 2 1
triad_p2_u2_P1_6ns triad.sh 2 2 1 6
triad_p4_u8_P2_1ns triad.sh 4 8 2 1
triad_data_plot triad_plot.sh
fft_p4_u8_P2_1ns fft.sh 4 8 2 1
bb_gemm_p4_u8_P2_1ns bb_gemm.sh 4 8 2 1
unit_test_init_base_address unit_test_init_base_address
unit_test_store_buffer unit_test_store_buffer

If you use "ctest -R triad", only test names containing "triad" will be executed. If you only interest in triad with 6ns config , put "ctest -R 'triad[A-Za-z_0-9]+6ns' " If you are not sure which test are invoked, add "--name-only" to print the test list instead of execution. CTest also supports -E to exclude some tests from execution.

Or you can just run the generated shell scripts manually. For example runs "/aladdin/build-directory/SHOC/triad/triad.sh 2 2 1 6" to will be same as running "ctest -R triad_p4_u8_P2_1ns". The generated shell scripts are coded in absolute path, so calling them from other place is safe.

ysshao commented 8 years ago

Hi Chia-Wei,

After extensive thoughts, I still find myself having several doubts of integrating the CMake flow to the Aladdin compilation.

  1. I do like the CMake flow with LLVM-Tracer as it enables users to install LLVM and Clang with the CMake flow. It's unclear what the benefit is for Aladdin in this case.
  2. The Aladdin build and simulation flow described in the original README file serves as a way for users to familiarize themselves with the way Aladdin runs so that they can run Aladdin with their own applications. The CMake flow encapsulates all the details of running Aladdin, which is really nice for Aladdin developers to run the existing tests. But it makes it harder for new users to understand the Aladdin flow and then extend it to their own cases.
  3. LLVM-Tracer generates input traces to Aladdin, but it is also a standalone tool on itself. We do have users that are just using LLVM-Tracer for some profiling purposes without using Aladdin. Having the inter-compilation dependences between Tracer and Aladdin also seems to be an overhead in some sense. I understand it's good for Aladidn developers like you and me who need to constantly change the code and want to make sure the code is updated. But it also unnecessarily increase the code size.

These are my thoughts so far. Feel free to let me know what you think. I really think these scripts are really useful, but I'm hesitate to merge them into the main distribution at this point.

Thanks, Sophia

rgly commented 8 years ago

For the unnecessary CMake code size issue. I keep the same idea. Checking the existence of Aladdin is truly unnecessarily complex in LLVM-Tracer. Like the following code. snippet.

  if (${HAS_ALADDIN})
    if(${BUILD_ON_SOURCE})
      set(TRACE_LOGGER "${TRACER_DIR}/${LOGGER_PATH}")
    else()
      set(TRACE_LOGGER "${CMAKE_BINARY_DIR}/LLVM-Tracer/${LOGGER_PATH}")
    endif()
  else()
    if(${BUILD_ON_SOURCE})
      set(TRACE_LOGGER "${CMAKE_SOURCE_DIR}/${LOGGER_PATH}")
    else()
      set(TRACE_LOGGER "${CMAKE_BINARY_DIR}/${LOGGER_PATH}")
    endif()
  endif()

How about moving unnecessary code to Aladdin and only check whether the CMake variable is defined in LLVM-Tracer. Like this:

  if (NOT DEFINED TRACE_LOGGER)
    if(${BUILD_ON_SOURCE})
      set(TRACE_LOGGER "${CMAKE_SOURCE_DIR}/${LOGGER_PATH}")
    else()
      set(TRACE_LOGGER "${CMAKE_BINARY_DIR}/${LOGGER_PATH}")
  endif()
rgly commented 8 years ago
  1. The reason I developed CMake flow of LLVM-Tracer is mainly for a substitution of llvm_compile.py. A build script should stop and show message when error occurs, but I don't see this behaviour on llvm_compile.py while I import more complex case to LLVM-Tracer. Aladdin suffers the same issue, so I make Aladdin reuse CMake scripts of LLVM-Tracer. I believe that all users frequently modify their C language design while using Aladdin.
  2. For users who wants to know the simulation flow of Aladdin, they can use CTest with argument -V, which shows the used shell commands to run the test. Most of tests are actually CMake-generated shell scripts. I think learning working flow from shell scripts is not hard, and I try to keep the generated files are almost the same position as original flow. Using CTest brings two advantages: job-parallelism and regular expression. I think it is worth for suffering CMake script complexity.
  3. I admit that the build flow of LLVM-Tracer instrumented-code is hidden by CMake, All the clang, opt and llvm-link details are invisible from users. A better way is to write a LLVM program instead of LLVM library like full_trace.so, which takes all the LLVM assembly files and doing linking, instrumenting and code generation internally. It should be more robust and easier to use. I have personally maintained an ad-hoc LLVM-Tracer using this method, but I need some time to organize it.

Here is what directory look like after running CTest:

rgly@rgly-Linux-Mint ~/Source/aladdin/build/SHOC/triad $ ls
CMakeFiles           full.llvm    sim                 triad-opt.llvm
cmake_install.cmake  full.s       triad-instrumented  triad_plot.sh
CTestTestfile.cmake  Makefile     triad.llvm          triad.sh
dynamic_trace.gz     output.data  triad.obj.llvm      triad.unopt.llvm
rgly@rgly-Linux-Mint ~/Source/aladdin/build/SHOC/triad/sim/p64_u4_P2_6ns $ ls
config_p64_u4_P2_6ns  out.csv  triad_stats  triad_summary

And here is triad.sh generated by CMake, which is used by CTest.

rgly@rgly-Linux-Mint ~/Source/aladdin/build/SHOC/triad $ cat triad.sh 
#!/bin/bash

PART_CONFIG=$1
UNROLL_CONFIG=$2
PIPE_CONFIG=$3
CLOCK_CONFIG=$4

CURRENT_DIR=`pwd`
if ! [ -f "/home/rgly/Source/aladdin/build/SHOC/triad/dynamic_trace.gz" ]
then
  echo "finds no /home/rgly/Source/aladdin/build/SHOC/triad/dynamic_trace.gz, generate it!"
  cd /home/rgly/Source/aladdin/build/SHOC/triad
  /home/rgly/Source/aladdin/build/SHOC/triad/triad-instrumented
  cd $CURRENT_DIR
fi

PARAEMETER_STRING="p${PART_CONFIG}_u${UNROLL_CONFIG}_P${PIPE_CONFIG}_${CLOCK_CONFIG}ns"
RUN_TEST_DIRECTORY="/home/rgly/Source/aladdin/build/SHOC/triad/sim/${PARAEMETER_STRING}"
CONFIG_FILE="${RUN_TEST_DIRECTORY}/config_${PARAEMETER_STRING}"

python /home/rgly/Source/aladdin/aladdin/SHOC/scripts/config.py /home/rgly/Source/aladdin/build/SHOC/triad triad \
    $PART_CONFIG $UNROLL_CONFIG $PIPE_CONFIG $CLOCK_CONFIG && \
cd $RUN_TEST_DIRECTORY && \
/home/rgly/Source/aladdin/build/common/aladdin triad /home/rgly/Source/aladdin/build/SHOC/triad/dynamic_trace.gz $CONFIG_FILE && \
cd $CURRENT_DIR

Changing work flow is always painful. I suggest not to merge if you and your team don't need these new features. I will still maintain it on my repository.

Thanks you for the code review. They are really useful to me.

Chia-Wei Chang

ysshao commented 8 years ago

Hi Chia-Wei,

Okay, let's do that. I'll close this pull request for now, and you can keep your flow in your repository since it works pretty well for you. The llvm_compile.py flow that doesn't stop when a compilation error occurs is a good point. Let's revisit the workflow issue later.

At the same time, feel free to send me review/pull requests whenever you have updates to share. Happy to do that.

Thanks, Sophia