sparcians / map

Modeling Architectural Platform

MAP - Modeling Architectural Platform

This is a framework designed and built by expert modeling/simulation engineers in the industry. Its purpose is to provide a set of classes, tools, and flows to aid in modeling/simulation of complex hardware for the purpose of performance analysis and better hardware designs.

These classes and tools are also designed to work with existing platforms like Gem5 and SystemC while providing more abstract and flexible methodologies for quick analysis and study.

MAP is broken into two parts:

1. Sparta -- A set of C++ classes (C++17) used to construct, bind, and run full simulation designs and produce performance analysis data in text form, database form, or HDF5. It's a modeling framework.
2. Helios -- A set of python tools used to visualize, analyze, and deep dive data generated for a Sparta-built simulator. It's a visualization toolset.

Current Regression Status

Building MAP

Building MAP is done in two parts

1. Sparta, the modeling framework: build sparta only in the sparta folder
2. Argos, the transaction viewer in Helios in the helios folder. Note that to build and use helios, you will need sparta built and installed somwehere on your system.

The MAP repository has numerous dependencies, which are listed in a conda recipe, and the versions of these libraries continuously change.

However, with the use of the conda recipe, users can set up a conda environment that will build and run the tools found in this repository.

This guide assumes the user is not familiar with conda nor has it installed and would like to build everything (not just sparta).

1. If conda is not installed, install it
   • Get miniconda and install: https://docs.conda.io/en/latest/miniconda.html
   • You can install miniconda anywhere
2. Activate conda conda activate
3. Go to the root of MAP
   • cd map
4. Install JSON and YAML parsers
   • conda install -c conda-forge jq
   • conda install -c conda-forge yq
5. Create a sparta conda development environment
   • ./scripts/create_conda_env.sh sparta dev
6. Activate the environment
   • conda activate sparta
7. To build Sparta framework components:
   • cd sparta && mkdir release && cd release
   • cmake -DCMAKE_BUILD_TYPE=Release ..
   • make
   • cmake --install . --prefix $CONDA_PREFIX
8. To build Helios/Argos transaction viewer:
   • cd helios && mkdir release && cd release
   • cmake -DCMAKE_BUILD_TYPE=Release ..
   • make
   • cmake --install . --prefix $CONDA_PREFIX

A few interesting cmake options to help resolve dependencies are:

For both Sparta and Helios:

• -DBOOST_ROOT=<BOOST_LOCATION>: Custom Boost location
• -DCMAKE_INSTALL_PREFIX=: Install prefix, defaults to a system wide location normally so you can use this for a local install in a home folder for example.

Helios only:

• -DSPARTA_SEARCH_DIR=<SPARTA_INSTALLED_LOCATION>: Use this to ensure helios finds Sparta, when you installed it in a non-default location Not providing this will try and find sparta in the map source tree, but this might fail, if you did not build in a folder named release
• -DPython3_ROOT_DIR=<PYTHON_LOCATION>: Not often needed but useful to point to the right python if you are not in a conda env)

Updating Regression/Build Environments for CI

CI files are generated when the command conda smithy rerender is run inside a MAP clone. That command uses the following files to control the generation of the CI-specific control files:

• conda-forge.yml - defines which platforms you want to support and some other higher-level things
• conda.recipe/conda_build_config.yaml - defines lists of values for variables that are used in meta.yaml
• conda.recipe/meta.yaml - uses variables (stuff inside {{ varname }} double curlies)

To update versions of OSes, edit the following file: https://github.com/sparcians/map/blob/master/conda.recipe/conda_build_config.yaml and then run conda smithy rerender.

Install conda smithy instructions:

conda install -n root -c conda-forge conda-smithy
conda install -n root -c conda-forge conda-package-handling

map's Issues

Enable pipeline caching for the CI

See https://docs.microsoft.com/en-us/azure/devops/pipelines/release/caching?view=azure-devops and Ray's work on using ccache in conda-build

sparta::QueueIterator -> operator incorrect?

Queue.hpp has:

/// Dereferencing operator
DataReferenceType operator* ()
{
    sparta_assert(getIndex()<attatched_queue_->total_valid_, "Not a valid Iterator");
    return getAccess_(std::integral_constant<bool, is_const_iterator>());
}

///support -> operator
DataReferenceType operator->()
{
    return operator*();
}

Should operator-> return a pointer type?

DAG cycles have extraneous info

A DAG exception where GOPs are being used have LOTS of useless graph information in the dot as well as the print. Need to figure out how this information is getting into the cycle information.

Create install target for cmake ExternalProject builds

One technique developers can use is ExternalProject from CMake. If the user uses Ninja, everything just works. If the user uses make, install targets are missing. Need to add support for this.

One the thing to note in my experiments with ExternalProject is that the cmake tool runs at the top level of Sparta and the GNUmakefile that is currently at the top level is catching the build and terminating. Will need to update that too.

sparta ReportHeader is clumsy with stdlib types

ReportHeaderWriter::header_position_ needs to store the result of os_.tellp() (i.e. decltype(os_.tellp()) for the lazy like me or std::streampos or std::ostreampos for the less lazy) but it is typed as unsigned long. Not only is this incorrect (most systems use long long for streampos) it's super confusing because it makes an error return value from os_.tellp() (i.e. streampos(-1)) look like a crazy large offset.

This is an example where code that interacts with the stdlib needs to correctly use the stdlib types for stored state. Doing so would remove the need for two static_casts (see line 195 and 204 in ReportHeader.hpp) and also make it easier to see that tellp() was returning an error.

In addition, we should probably be setting the exceptions() mask on most streams to throw on std::failbit or std::badbit being set so that we don't have to remember to check for error return codes like we're writing C. std::eofbit usually only should throw on ostreams. Looking through the codebase, I see a few places where we calling exceptions( to set some streams to throw but it might make sense for Sparta to have stream classes that throw by default rather than requiring all usages of streams to correctly set the exceptions() throwing mask.

NOTE: I also see a few places where we have code like this:

            // Throw on write failure
            outfile_->exceptions(std::ostream::eofbit | std::ostream::badbit | std::ostream::failbit | std::ostream::goodbit);

The fact that std::ostream::goodbit is included in our arguments to exceptions() demonstrates that we have no clue what we're doing. std::ostream::goodbit is 0. Including it in the bitwise OR doesn't do anything but it's also not a confident look.

Remove pipeline collection functionalities from Resources

Reference #45
This task is about removing internal iterable collectors from resource classes.
Resource classes should not be lumped together with pipeline collection responsibility.
The modelers would now have to create external iterable collectors, register that with the proper tree node and the resource to carry on pipeline collection.

Should QueueIterator::isValid() be const?

Also, "attached" is misspelled as "attatched" in Queue.hpp.

Fix QueueIterator copy assignment

QueueIterator copy assignment is buggy. In this code:

            QueueIterator& operator=(const QueueIterator& rhs)
            {
                attached_queue_ = rhs.attached_queue_;
                physical_idx_ = rhs.physical_idx_;
                unique_id_ = rhs.physical_idx_;
                return *this;
            }

the line should read:

                unique_id_ = rhs.unique_id_;

SPARTA_FLATTEN on STL types

Curious, can I use SPARTA_FLATTEN on STL types that contain objects with PairDefinitions?

Port_test is segfaulting on some builds

Issue in Port tests, specifically the fix to issue #15

Include guards are not compliant

The include guards that Sparta uses are not compliant with the industry standard. Specifically, they should not start with underscores.

There are two solutions:

1. Use #pragma once
   There's a holy war regarding this on the net. First, this is a compile time directive and not a preprocessor one (possible slower compiles). Secondly, if there are multiple files with the same name (different directories), you'll drop both automatically. Now, in Sparta, I hope we don't have that kind of crappy design, so I'm not too worried about it. Third, it's not standard, but most compilers (if not all) support it
2. Define a standard format
   If we keep the #ifdef guards, Sparta should adopt a standard. I propose this: SPARTA_<function>_<filename>_H where function is kernel, simulation, events, etc.

I like the second one. Thoughts?

I'd be handy if `State` was copy constructible

Since the State class in simulation is used by dynamically created objects, it'd be handy to have this class copyable.

Sparta's memory allocator allows modeler's to shoot foot

Currently the SpartaSharedPointerAllocator will complain if it's being deleted and there are outstanding SpartaShared pointers hanging out in simulation. This is good, but bad. Two things happen:

1. The outstanding memory objects are now orphaned.
2. The outstanding memory objects memory is deleted (i.e. they're gutted!)

Need to figure out a better mechanism to handle this problem.

sparta::Queue performance improvements

sparta::Queue uses new[] (queue_data_.reset(new value_type[vector_size_]);). It would be nice to take an allocator parameter, and add an rvalue ref version of push to reduce copying.

For Global Ordering Points, create a convenience function

Right now to get a GOP, you do this:

getClock()->getScheduler()->getDAG()->getGOPoint

This requires a modeler to know TOO much about the FW. Need to add a function/macro or something to hide this mess.

Also, need a precedence operator for Vertex*

Cool map flipper

This would be nice to add to the Utils.hpp:

    // Function to invert a maps or an unordered_map, or any type of
    // class that has key/value semantics.  The variadic template
    // argument is necessary 'cause map/unordered_map don't have just
    // 2 template parameters.  ;)
    template <typename K, typename V, typename... TArgs, template<typename...> class MapT>
    MapT<V, K> flip_map(const MapT<K, V, TArgs...> & map)
    {
        MapT<V, K> inv;
        for(auto [key, value] : map) {
            inv.insert(std::make_pair(value, key));
        }
        return inv;
    }

Refactor TreeNode class

TreeNode class is one giant, monolithic class which needs to be heavily refactored to provide more flexibility.
This ticket is about making the TreeNode class leaner and cleaner by removing the member methods and making them free functions which visit tree nodes and apply their custom functionalities.

Define and document single source of truth for external requirements

Curious, is there a way to generate this file from the cmake configs? I worry that this file and the required minimums on the build would get out of sync...

_Originally posted by @klingaard in a comment on pr #34

Currently in map, we have:

1. conda.recipe/meta.yaml conforming to meta.yaml doc
2. stuff in CMakelists.txt in various places
3. hardcoded versions of stuff in other build files (python36 in pipeviewer build files)
4. other things I'm missing??

This needs to be cleaned up so that we have a single source of truth for external dependencies.

Disable pipeline collection on strings

Now that the core example and other examples use NVP for data collection, I want to remove ALL ability to collect data for pipeouts as strings.
This means the following:

1. Remove Collection methods from all of Sparta’s resources. Modelers can now create explicit sparta::Collectable objects if they want them collected.
2. Move sparta::Collectable to look only for NVP registrations. Annotations will be allowed, but … barely. Need to figure that out.

Extension just seem to not work right...

I've been playing around with extensions and I'm having a tough time getting them to do what I think they're supposed to do.

I'd like to set up some topology extensions in a subunit (like a core) using extensions, but the code in TreeNode just can't find it. Specifically, I'm trying to get the extension in createSubTree of the core's sparta::ResourceFactory, but I think it's "too soon" to get them.

I've re-written the code a bit and pushed to branch knutel/extension_changes and I am able to at least get extensions if supplied in a config yaml or an extensions file, but I cannot get them if I use the factory -- no matches.

Users are seeing segfaults while using loggers in Destructors

This is a feature that should work -- users can use loggers in their destructors. However, if the user uses a resource factory to build their simulation hierarchy, and stash that factory in Simulation's ResourceSet, they will see a crash if they use a logger during their Unit's destruction.

The issue is that in Simulation the resource set is being destructed after the Taps are destructed. Or to say it differently, the Taps are being destroyed before the resource sets. It's an older thang... and an easy fix.

Print resource name during port errors of disparate types

Example:

Error setting up simulator because of an exception:
ERROR: Attempt to bind DataInPort of a disparate types: 'in_data_read_req' to 'out_req'

Would it be possible to add the resource names and/or the hierarchy of both when this error occurs?

SpartaSharedPointer doesn't support move

Will look at adding this support. This need was sparked by the addition of move/forwarding semantics added to the Sparta resources.

Fix Queue test for failing Collectable template instantiation

Collectable class was instantiated for a template type std::unique_ptr even though no properties of Collectable was used.
Fixed this issue by not using std::unique_ptr and explicitly move instances into Queue.

SpartaSharedPointerAllocator Destructor prints out misleading message

The destructor of SpartaSharedPointerAllocator checks for outstanding objects and if successful, prints out a message with the number of allocated blocks and the number of returned blocks.
For the number of returned blocks, it prints out

free_blocks_.size()

I think it should print out

free_idx_

to show the number of blocks returned to pool.

SpartaSharedPointer is overzealous

I have asserts on checking for dead pointers. Turns out, that's not necessary. Going to removed them.

Pipe, Pipeline, and Array are sparta::Resource types -- why?

I think we had in our minds that these resources would show up in the tree one day. No one did that. Having them derive from Resource is problematic as Resource is restrictive on naming. Also, what's the point?

Memory_test.cpp fails regress target with compiler warning as error

Using the map-docker devel-env image based on fedora 31 (latest) (gcc (GCC) 9.2.1), the following warning fails the regress target. This is the only warning when removing "-Werror" from cmake.
cc: @bdutro @bdutro-sv

Scanning dependencies of target Memory_test
[ 51%] Building CXX object test/Memory/CMakeFiles/Memory_test.dir/Memory_test.cpp.o
In file included from /work/map/sparta/./sparta/simulation/ResourceTreeNode.hpp:9,
from /work/map/sparta/./sparta/sparta.hpp:17,
from /work/map/sparta/test/Memory/Memory_test.cpp:7:
/work/map/sparta/./sparta/functional/ArchData.hpp: In function ‘void testMemoryObjectPerformance()’:
/work/map/sparta/./sparta/functional/ArchData.hpp:381:23: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
381 | memcpy(data_ + offset, data, size);
| ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/work/map/sparta/./sparta/functional/ArchData.hpp:381:23: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
381 | memcpy(data_ + offset, data, size);
| ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/work/map/sparta/./sparta/functional/ArchData.hpp:381:23: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
381 | memcpy(data_ + offset, data, size);
| ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/work/map/sparta/./sparta/functional/ArchData.hpp:381:23: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
381 | memcpy(data_ + offset, data, size);
| ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[3]: *** [test/Memory/CMakeFiles/Memory_test.dir/build.make:63: test/Memory/CMakeFiles/Memory_test.dir/Memory_test.cpp.o] Error 1
make[2]: *** [CMakeFiles/Makefile2:3725: test/Memory/CMakeFiles/Memory_test.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:2579: test/CMakeFiles/regress.dir/rule] Error 2
make: *** [Makefile:274: regress] Error 2

simdb files are automatically generated quietly -- in the darkness

Apparently, the feature simdb is defaulted to 1 which creates a simdb file ALL THE TIME for each run of a simulator. The simdb file is saved to tmp. I think this was done to ensure simdb was working properly, but we need to disable this for now.

Queue, Buffer, and Pipeline should use in-place new

I think there is a bug in sparta Queue, Buffer, and Pipeline. These structures should use in-place new when allocating a new entry and call the destructor of an entry when it goes out of scope. This would ensure that an 18 entry queue, for example, even if it allocates space for something like 64 entries in the implementation will only have 18 entries alive at any one time. I tried to fix this in Queue (I believe it was Queue) but Knute said it was causing some issue. Knute had to back out my changes from Sparta. I never got back to this. In my situation the 10 entry allocator for my 5 entry queue was aborting.

From Knute:
Yeah, I had to back this out — there were random seg faults in the Sparta testers using Queue. Destruction of popped, out or reach elements seems to be a better solution than the in-place new (possibly).

InPort -> InPort precedence doesn't catch disparate phases

If a user does this:

   const uint32_t one_delay = 1;
   const uint32_t zero_delay = 0;
   DataInPort inp1{..., zero_delay};
   DataInPort inp2{..., one_delay};
   inp1.precedes(inp2);

The framework will quietly drop a DAG issue and confuse the poor modeler. What's happening is that a zero-cycle InPort will move itself from the PortUpdate phase to the Tick phase behind the scenes. The precedence being asked for in the example makes no sense. Inp1 will always follow inp2 since inp2 is on the PortUpdate phase: Update -> Tick.

An error message needs to be added to detect this precedence issue early before it gets to the DAG.

It'd be handy to allow use of `const` methods for CREATE_SPARTA_HANDLER

Now, I'd love to abandon the macro completely and just use some cool C++17 feature, but for now it'd be handy to be able to create a SpartaHandler with a const method:

class Foo
{
private:
    void myMethod_() const {}
};

Foo::Foo()
{
    auto func = CREATE_SPARTA_HANDLER(Foo, myMethod_);
}

You get this when you try this:

error: no matching function for call to 'from_member'
             CREATE_SPARTA_HANDLER(Foo, myMethod_)};
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 note: expanded from macro 'CREATE_SPARTA_HANDLER'
    sparta::SpartaHandler::from_member<clname, &clname::meth>            \
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 note: candidate template ignored: invalid explicitly-specified argument for template parameter 'TMethod'
        static SpartaHandler from_member(T* object_ptr, const char * name = "")

Release pipeViewer

Get the pipeViewer working on Linux and checked into sparcians/map

Open Source formatter for Sparta code

There is a high degree of code formatting inconsistency in Sparta codebase.
This task is about researching on several open source formatters and how to automate code formatting with git hooks (pre-commit) on every commit.

Improve Sparta Build time

This task is about improving the Sparta build time.

1. There are many header files with no separate implementation files and these files are included by a lot of source files.
2. Even within these header files, entire headers are included when forward declarations would suffice.
3. Look into precompiled header file of commonly used standard headers and rarely changing sparta headers.

SpartaSharedPointer refcount busted on nullptr assignment

There's a path through assignments and move operations where the refcount is in a borked state and a double delete occurs. I added a test and confirmed it.

New move operations in SpartaSharedPointer tanks performance

I thought I was being clever in the move operations of SpartaSharedPointer by setting the orig's internal refcounter object to a new one -- but this adds unnecessary allocations! Need to rethink SpartaSharedPointer a bit...

Vertex objects do not have intelligent labels

When a DAG exception occurs and the cycle is printed, the labels for the vertex are generic and NOT the labels of the actual Scheduleable objects.

Add rvalue reference overloads to push-like functions in all Resource containers

Enable codecov for the CI

Umbrella codecov is nice. I was thinking of adding coverage runs to the CI and reporting the results into Azure DevOps but codecov has a nice display back into PRs:

I may still also report the coverage results into Azure just to compare but I like the fact that codecov will comment on the PR with coverage changes.

--report-all only works with --show-options

Report all doesn't work unless the user specifies --show-options. The bug is that the reports are not copied unto the SimulationConfiguration anywhere but in the check for --show-options.

Registers cannot be reset unless initial_value is specified

If a Register is initialized without the initial_value specified, like in the following Register::Definition, the reset() method causes a segfault.
sparta::Register::Definition reg_def = { 0, "reg0", 0, "group", 0, "register 0", 4, {}, {}, nullptr, Register::INVALID_ID, 0, nullptr, 0, 0 }

When initial_value is not specified, a default initial value is used:
<top.dummy.regs.reg0 32 bits cc cc cc cc>

When calling reset() in this scenario I would expect the register to be reset to the default initial value.

--show-options is busted

Throws an exception. Shouldn't do that. Should show what is supported and what isn't. Not crash.

sparta::Vertex::unlink() uses iterator after std::List::erase()

When I build for Linux using the conda toolchain, I hit a segfault in DAG_test as mentioned in #34 (comment)

Program received signal SIGSEGV, Segmentation fault.
0x0000000000510053 in sparta::Vertex::unlink (this=0x1d821f0, efact=..., w=0x1d81820)
    at /home/conda/feedstock_root/build_artifacts/map_1583622171682/work/sparta/src/Vertex.cpp:50
50	                if (*el == ei->first) {
(gdb) p el
$1 = {_M_node = 0x10ec8348e5894855}
(gdb) p ei
$2 = {_M_node = 0x1d82810}
(gdb) p ei->first
$3 = (sparta::Vertex * const) 0x1d81820
(gdb) p *el
$4 = (sparta::Vertex *&) <error reading variable>
(gdb) list
45	        if (ei == outbound_edge_map_.end()) {
46	            // Edge not present -- just ignore
47	            return false;
48	        } else {
49	            for (auto el = outbound_edge_list_.begin(); el != outbound_edge_list_.end(); ++el) {
50	                if (*el == ei->first) {
51	                    outbound_edge_list_.erase(el);
52	                }
53	            }
54	            outbound_edge_map_.erase(ei);
(gdb) p (*el == ei->first)
Cannot access memory at address 0x10ec8348e5894865

Iterators passed to std::List::erase() are invalidated and std::List::erase() returns an iterator to the item following the one that was erased. So, you can't let the for-loop increment the iterator because it will try to increment an invalid iterator after the erasure.

I'll be submitting a PR for this shortly.

SpartaSharedPointerAllocator::hasOutstandingObjects() incorrect?

SpartaSharedPointerAllocator::hasOutstandingObjects prints a warning message during destruction if there are still in-flight object at that time.
The current check is

return (allocated_ != free_idx_);

I think the check should be

return (free_idx_ + memory_blocks_.size() == memory_blocks_.capacity());

Conditionally compile Collectable::collect on ostream operator availability

Most of the resource classes have internal iterable collectors which enable them for pipeline collection.
Currently, there is a strict restriction that the template type of resources must know how to respond to ostream operators.
This is cumbersome for cases where we need resources but do not care about pipeline collection.
We should not compile Collectable code that needs ostream operators if ostream operators are not defined by the template type.
This can be easily achieved with a mixture of constexpr if statement with has_ostream_operator metafunction in Utils.h.

No way to set a precedence between an InPort and an Outport and/or GOP

Somehow this got lost, but there used to be a way to set explicit precedence between a Port and a GOP or between an InPort and OutPort. There's still support for InPort<->InPort precedence support, and the ability to register a consuming/producing event for the Port, but you can't do this:

// My InPort comes before my GOP
my_inport_ >> my_gop_;

// My InPort will drive this outport:
my_outport_.registerProducingPort(my_inport_);

Need this added.

1. Use case for the outport -> inport:
   If you have an InPort whose handler immediately calls a zero-cycle OutPort, the receiver of that OutPort call cannot easily set precedence on the InPort in the original unit.
2. Use case for the InPort -> GOP.
   If an InPort's handler will schedule another event, but that event is in another block separated by a GOP, we need a way to have that InPort precede that GOP.

PairDefinition use requires a type named `type`. Let's change that.

How about SpartaPairDefinitionType or a (shutter) a macro?

SPARTA_PAIR_DEFINITION_TYPE(MyPairDefintion);

Add watermark callback to SpartaSharedPointerAllocator

To the allocator class, add the ability to add a SpartaHandler or std::function that allows the modeler to be notified when the watermark on their allocator was hit. This allows the developer to pause simulation and see what objects are outstanding for debug,

Simulator dies when '-h' or '--help' is specified on the command line

My command line simulator dies with:

libc++abi.dylib: terminating with uncaught exception of type sparta::SpartaException: Cannot setup simulation before parsing command line

when ‘-h’ or ‘--help’ is specified on the command line. ‘--help-tree’ works fine. I little spelunking in CommandLineSimulator.cpp makes be believe that ‘-h’ or ‘--help’ should set no_run_mode_, but it does not.