olympia (riscv-perf-model) HEAD fails regression

[jeffnye-gh]

The error is master_sb != nullptr: Didn't find the scoreboard 'integer' for scoreboard 'alu0' for parent 'top.cpu.core0.execute.alu0': in file: '[redacted]/map/sparta/src/Scoreboard.cpp', on line: 350

to duplicate:

conda activate sparta git clone --recursive https://@github.com/riscv-software-src/riscv-perf-model.git cd riscv-perf-model mkdir release; cd release cmake .. -DCMAKE_BUILD_TYPE=Release make olympia

make regress

15% tests passed, 55 tests failed out of 65

Log file attached regress.log

Can anyone confirm this?

[aarongchan]

I made a change recently in map that changes how the scoreboard view is interfaced with, due to a bug/error in always assuming there is a core node. The burden is now on the caller to pass the node that can view the scoreboard to the ScoreboardView.

For a quick fix you could alter the ScoreboardView code when it gets setup for LSU and ExecutePipe (should be in both their setup functions). An example of this is in my PR: https://github.com/riscv-software-src/riscv-perf-model/blob/fe146aedc68daf9ae033515876c133ede07496cb/core/ExecutePipe.cpp#L41.

Alternatively, you could use an older map version 2.0.12 instead of the current one until I merge my PR in. Sorry for not making this map change known during our meeting.

[jeffnye-gh]

thanks aaron

[jeffnye-gh]

See below for what I've tried so far. But I really need to get our regressions back up and running. Is there a combination of map/sparta sha's/branches and riscv-perf-model branches/shas which were tagged with known working regressions? Otherwise it is trial and error.

I cant build 2.0.12 from the tar ball out of the box, CMakeList is broken because of the git repo check. I did a quick patch on getting the rc var and git repo version set but something I can look into more eventually.

I also made the change suggested but I get similar failures. And I'll look into that also.

And I'm wondering if there also an issue with the miniconda sparta environment requiring it to be built from scratch. I got some unexpected errors with cmake

CMake Warning at example/Documentation/communication/CMakeLists.txt:7 (add_executable): Cannot generate a safe runtime search path for target Events_dual_example_test because files in some directories may conflict with libraries in implicit directories:

runtime library [libz.so.1] in /usr/lib/x86_64-linux-gnu may be hidden by files in:
  /nfshome/randy/miniconda3/lib
runtime library [libcrypto.so.3] in /usr/lib/x86_64-linux-gnu may be hidden by files in:
  /nfshome/randy/miniconda3/lib
runtime library [libcurl.so.4] in /usr/lib/x86_64-linux-gnu may be hidden by files in:
  /nfshome/randy/miniconda3/lib

[aarongchan]

Hey Jeff, I tried on the current master branch of olympia (with a clean olympia repo) and with the current version of map (2.0.13) that has the scoreboard fix. I fixed both the LSU and ExecutePipe areas with the following code:

std::vector<core_types::RegFile> reg_files = {core_types::RF_INTEGER, core_types::RF_FLOAT};
// Setup scoreboard view upon register file
auto cpu_node = getContainer()->findAncestorByName("core.*");
if (cpu_node == nullptr)
{
    cpu_node = getContainer()->getRoot();
}
for (const auto rf : reg_files)
{
    // alu0, alu1 name is based on exe names, point to issue_queue name instead
    scoreboard_views_[rf].reset(
        new sparta::ScoreboardView(getContainer()->getName(), core_types::regfile_names[rf],
                                   cpu_node)); // name needs to come from issue_queue
}

and the simulator was able to run without any of the errors or issues. Regressions also ran without any errors.

[jeffnye-gh]

i'll give this a try. Thanks.

[klingaard]

Wow, lots of headaches with this latest branch.

I'll admit that I usually build/install the latest sparta from the branch map_v2 and not from the tar ball. In the future when I make a release, I'll use that instead to make sure it's working.

But what patch did you need to make to CMakeList to get it working? Curious minds...

Another colleague discovered this week that the environment.yml file in Olympia is way out of date. I'm thinking about removing this and just requiring users follow the conda setup directions on map instead since that's regressed.

Finally, I'm a bit confused -- did you update sparta and not Olympia and saw the assertion? Or the other way around? I don't know why you need to patch Olympia if you updated sparta and attempted a regression on the latest sparta...

[jeffnye-gh]

Let me preface,

• regressions are working again, Thanks very much Aaron
• it's possible there could be user error, or missing steps that caused the problems

• The 1st CMake issue is related to these lines in the Sparta CMakeLists.txt and the fact that the tarball I grabbed failed the git describe command. There is no .git in the tarball, of course. Maybe I skipped an assumed step when using the tarball.

execute_process (COMMAND bash "-c" "git describe --tags --always"
  WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
  OUTPUT_VARIABLE GIT_REPO_VERSION RESULT_VARIABLE rc
)

if (NOT rc EQUAL "0")
  message (FATAL_ERROR "could not run git command 'git describe --tags --always', rc=${rc}")
endif ()

• We never use the environment.yml file. We use the process as described in the map/sparta read me.

• I have never seen the " may be hidden by files in" problem before. I do not have a guess at the moment.

• And the original source fixes I made were incorrect. When I aligned map/sparta to olympia and made the fixes properly it worked.

Our issue is that we use olympia as a reference, but we have our own model with changes that diverge related to our microarchitecture. We still share copies of code. If the assumption made about map/sparta versions change then things get out of sync.

I think we need to change our process to more firmly link map/sparta SHA to our model. I raised a question about this in the discussions asking what others do in these cases.

[klingaard]

The 1st CMake issue is related to these lines in the Sparta CMakeLists.txt and the fact that the tarball I grabbed failed the git describe command.

Dobt! Didn't think about sparta being build outside of the git environment. Will have to think about how to handle that...

[klingaard]

With Aaron's latest commit, HEAD of Olympia now builds/runs with Map v2.0.13. I think this can be closed.