fusion draft PR commit #135

jeffnye-gh · 2023-12-28T21:37:36Z

Contains the draft version of the fusion API and start of a test bench. Doxygen file provided also. README.md has the gist.
fusion/test/testbench.cpp has some usage.

The API is more general, simpler, better alignment with other source and existing packages (mavis).

See the README.md or front page of doxygen. API uses functor's at the top level and within the inner transform operations for generality. There are examples of each, including a lambda example.

FieldExtractor (mavis) and MachineInfo (future CoreExtensions ?) are template parameters.

To run the test bench and generate docs @ fusion/html/index.html :

cd fusion
mkdir -p release; cd release
cmake ..
make -j32 regress
make docs

jeffnye-gh · 2023-12-28T21:43:29Z

fusion/test/CMakeLists.txt

+
+
+
+#cmake_minimum_required(VERSION 3.17)


these dead directives are not intended.

refactor long testbench file Added special field extract method, fieldextractor test FSL: domain language has a name, fusion script (or spec) language Add fsl grammar and lexer, fsl.l/.y Add symbol table support Added syntax check tests Local copies of json and test files Reapplied clang-format

klingaard · 2024-01-21T20:56:41Z

A few general comments (nits):

The header files should be .hpp
The header files in olympia are named exactly (case included) as the class they represent. However, this rule isn't documented in the CodingStyleGuide
Noticed some files copied from Mavis. Mavis didn't follow a lot of the styles adopted by Olympia (for example using #ifdef instead of #pragma once for header guards). Do you mind making them consistent with Olympia's style?
Function names do not match the coding style guide -- they lower_lower for example...
Any single line conditionals will need to be encapsulated in {}

Started looking at the implementation. Have some comments pending.

klingaard

A few comments for now -- some mostly nits. I'd like to run the tester and step through it with the debugger to understand the flow a bit. But, this a awesome.

klingaard · 2024-01-21T20:56:17Z

fusion/test/testbench.cpp

+    unordered_map<string, fusion::HashType> & exp,
+    const FusionType::FusionGroupCfgListType & in)
+{
+    using FBASE = fusion::FusionGroupBase;


Doesn't seem to be used...

klingaard · 2024-01-21T20:58:48Z

fusion/dsl/qparser.cpp

+
+// ----------------------------------------------------------------
+// ----------------------------------------------------------------
+bool QParser::parse(string fn)


Probably be const &

fixed in 2024.1.22 push

klingaard · 2024-01-21T22:11:21Z

fusion/README.md

+
+The currently considered procedure for Fusion is an implementation in a 
+Sparta unit typically in the Decode. The primary input to the 
+fusionOperator() is a form of the InstQueue, sparta::Queue<InstPtr>.


I would guess that the operator is templatized (unless a lambda) that can take any type of iterable.

klingaard · 2024-01-21T22:13:32Z

fusion/README.md

+ 4.   uarch oly1
+ 5.   input_seq in_seq
+ 6.
+ 7.   sequence seq1(in_seq,rv64g) {


I wonder if the rv64g here is redundant since the fusion defines this on line "3"

It may be, rv64g is a reference to the isa decl object on line 162. I've kept the arg list for this version so that when a sequence decl is a top level object it is consistent. This is not well documented.

I think we should have a discussion on the dsl syntax. I threw something out there as a strawman.

klingaard · 2024-01-21T22:14:58Z

fusion/dsl/fsl.l

+
+[\.]*[_a-z\.A-Z0-9]+  {
+                 //E("ID")
+                 yylval.sval = strdup(yytext);


Curious, how will this mess with valgrind support once we've turned it on...

fingers crossed? i have not run valgrind on this parser.

klingaard · 2024-01-21T22:30:50Z

fusion/dsl/README.md

+10.    gpr g1,g2
+11.    s20 c1
+12.    s12 c2
+13.    u6 c3
+14.  
+15.    //The constraints declarations
+16.    g1 != g2                 // C3, the contrived constraint
+17.                             // In this version of the syntax the
+18.    transform t1             // scope of sequence and constraints is
+19.                             // within this fusion_decl


Is this missing the constraints keyword?

Yes, this text is old, needs an update.

I've remove this file for now. When we've had time to discuss the syntax/grammar there would be an update to this text.

klingaard · 2024-01-21T22:32:03Z

fusion/fusion/fieldextractor.h

+    };
+
+    //!  \brief placeholder
+    void info(std::ostream & os = std::cout)


method can be const

Suggest making the method a template on the ostreamT

I removed the placeholder method

klingaard · 2024-01-21T22:35:06Z

fusion/fusion/fieldextractor.h

+    //!
+    //! handles field name and immediate checking
+    //! \note RS_MAX is overloaded to identify get's for immediate fields
+    uint32_t getField(InstPtrType inst, FieldName f) const


So, outside of complaining about missing consts ( 😆 ), I have a huge issue with single letter variable names for one main reason: search for f in this function. 😱

klingaard · 2024-01-21T22:38:18Z

fusion/fusion/fusion.h

+        Fusion() :
+            Fusion({}, {}, {}, FusionGroupTypeAlloc(),
+                   MachineInfoTypeAlloc(), FieldExtractorType())
+        {
+        }


Curious, does this ctor have any value?

A bare FusionType was instantiated in the testbench compile checks test. Removing the instance eliminates the need for it.

//Removed FusionType f1;

klingaard · 2024-01-21T22:38:42Z

fusion/fusion/fusion.h

+        }
+
+        //! \brief future file type detector
+        bool isJsonFile(std::string fn) { return false; }


method can be const. probable want to (void) out fn or drop it.

coding style and file name changes functional changes are minimal, symbol table expect vs actual comparison code removed dated dsl/README

…nye-gh/fusion_dpr update top level README.md

jeffnye-gh · 2024-01-29T21:00:36Z

I have posted a PR for the Fusion API alone. We will continue the DSL syntax discussion.

fusion draft PR commit

8d40b08

jeffnye-gh commented Dec 28, 2023

View reviewed changes

Jeff Nye added 4 commits January 11, 2024 17:03

Merge branch 'master' into jeffnye-gh/fusion_dpr

f1abf7f

fusion draft PR commit

f176661

klingaard reviewed Jan 21, 2024

View reviewed changes

Jeff Nye added 2 commits January 22, 2024 09:57

changes per knute's comments 2024.1.21

994f9e0

coding style and file name changes functional changes are minimal, symbol table expect vs actual comparison code removed dated dsl/README

Merge remote-tracking branch 'origin/jeffnye-gh/fusion_dpr' into jeff…

d0a399c

…nye-gh/fusion_dpr update top level README.md

jeffnye-gh marked this pull request as ready for review January 29, 2024 20:31

jeffnye-gh closed this Jan 29, 2024

klingaard mentioned this pull request Feb 8, 2024

Implement micro op fusion in decode stage. #113

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fusion draft PR commit #135

fusion draft PR commit #135

jeffnye-gh commented Dec 28, 2023

jeffnye-gh Dec 28, 2023

jeffnye-gh Jan 22, 2024

klingaard commented Jan 21, 2024 •

edited

Loading

klingaard left a comment

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

klingaard Jan 21, 2024

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

jeffnye-gh Jan 22, 2024

klingaard Jan 21, 2024

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

klingaard Jan 21, 2024

jeffnye-gh Jan 22, 2024

jeffnye-gh commented Jan 29, 2024




		#cmake_minimum_required(VERSION 3.17)

fusion draft PR commit #135

fusion draft PR commit #135

Conversation

jeffnye-gh commented Dec 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klingaard commented Jan 21, 2024 • edited Loading

klingaard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffnye-gh commented Jan 29, 2024

klingaard commented Jan 21, 2024 •

edited

Loading