hir installation log

[paili0628]

Below are the steps and the final error message when building hir on havarti:

1. clone repository from source:

   git clone https://github.com/mcl-csa/hir-dev.git

2. Go to hir-dev directory:

   cd hir-dev

3. Add vivado to path:

   source /scratch/opt/Xilinx/Vitis/2020.2/settings64.sh

4. cmake:

   /usr/bin/cmake .

At this point the cursor shows that the build was successful:

-- multiplier_f32
-- adder_f32
-- subtract_f32
-- div_f32_ip
-- gt_f32
-- lt_f32
-- Configuring done (0.0s)
-- Generating done (0.0s)
-- Build files have been written to: /scratch/pai/hir-dev

5. Then enter benchmarks directory:

   cd benchmarks

6. cmake

   /usr/bin/cmake .

cmake gives the following error message:

CMake Warning (dev) in CMakeLists.txt:
  No project() command is present.  The top-level CMakeLists.txt file must
  contain a literal, direct call to the project() command.  Add a line of
  code such as

    project(ProjectName)

  near the top of the file, but after cmake_minimum_required().

  CMake is pretending there is a "project(Project)" command on the first
  line.
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) in CMakeLists.txt:
  cmake_minimum_required() should be called prior to this top-level project()
  call.  Please see the cmake-commands(7) manual for usage documentation of
  both commands.
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Error at common/CMakeLists.txt:1 (add_vivado_ip_target):
  Unknown CMake command "add_vivado_ip_target".

CMake Warning (dev) in CMakeLists.txt:
  No cmake_minimum_required command is present.  A line of code such as

    cmake_minimum_required(VERSION 3.26)

  should be added at the top of the file.  The version specified may be lower
  if you wish to support older CMake versions for this project.  For more
  information run "cmake --help-policy CMP0000".
This warning is for project developers.  Use -Wno-dev to suppress it.

7. pass in the -Wno-dev flag:

   /usr/bin/cmake . -Wno-dev

   Below is the error message shown:

   CMake Error at common/CMakeLists.txt:1 (add_vivado_ip_target):
   Unknown CMake command "add_vivado_ip_target".

[sampsyo]

Hi! Thanks for the detailed walkthrough!

There are a couple of confusing things about this, both having to do with CMake being weird:

1. cmake . doesn't actually build anything; it generates the build scripts to do stuff. So the fact that build . succeeds in the root directory doesn't mean the build succeeded; it just means that CMake successfully generated a Makefile. To actually build stuff, you then have to type make. (This also fails; more on this below.)
2. The CMakeLists.txt file in the benchmarks directory is not meant to be used as a standalone build; it's just meant to be included with a "complete" build from the root directory. That's why you're getting an error about missing cmake_minimum_required; this declaration is in the root instead of here. Note that the root CMakeLists.txt contains add_subdirectory(benchmarks). It also includes cmake/tclFunctions.cmake, which defines add_vivado_ip_target.

So to build stuff, what we need to do is roughly this:

$ mkdir build
$ cd build
$ /usr/bin/cmake ..
$ make

(While it's not required, it makes things a little cleaner to use an empty build directory for CMake builds. It's just easier to delete that and start fresh if you need to.)

For me, I get this error when trying to make:

Unknown argument circt-lsp-server
Usage: cmake --build <dir> [options] [-- [native-options]]
[…]

I tracked this down to some weird CMake hackery they're using to try to build the CIRCT tools. I applied this patch:

diff --git a/cmake/tclFunctions.cmake b/cmake/tclFunctions.cmake
index 86045b1..39b5cb6 100644
--- a/cmake/tclFunctions.cmake
+++ b/cmake/tclFunctions.cmake
@@ -5,7 +5,7 @@ function (add_circt_project part_name rel_constraint_file rel_circt_path)
   set(circt_build_path ${circt_path}/build)
   add_custom_target(
     circt ALL
-    COMMAND cmake --build ${circt_build_path} --target circt-opt circt-lsp-server
+    COMMAND /usr/bin/cmake --build ${circt_build_path} --target circt-opt
   )
   set_property(GLOBAL PROPERTY part_name ${part_name})
   set_property(GLOBAL PROPERTY constraint_file ${CMAKE_CURRENT_SOURCE_DIR}/${rel_constraint_file})

…which then revealed that I didn't have the submodules for the hir-dev repo. It's an easy fix with git submodule init ; git submodule update. But you can also just use the --recursive flag on your initial git clone to avoid this problem. (In fact, that would be better because the circt subrepo has its own subrepositories!)

Anyway, the next step here is to (as indicated by the errors!) build CIRCT, which involves building MLIR, which is kind of a slow process. Hopefully this unblocks you, @paili0628; I'm happy to help continue down the garden path here to see what trouble comes up next!

[calebmkim]

So based on these updates I tried to get hir working, and I'll post an update here.

So I tried git submodule init and git submodule update and tried to build CIRCT. I built llvm/mlir fine.

I then tried to bid CIRCT: While attempting to build the scripts to build CIRCT, I got this error:

CMake Error at llvm/build/lib/cmake/llvm/LLVMProcessSources.cmake:114 (message):
Found unknown source file OpFusionAnalysis.cpp

Please update
/scratch/cmk265/learning/hir-dev/circt/lib/Conversion/AffineToHIR/CMakeLists.txt

I noticed that for some reason, in this file, the line forOpFusionAnalysis.cpp was commented out, so I uncommented it. I then also uncommented the line in file where OpFusionPass.cpp was commented out as well.

This at least got rid of the error while building the scripts... however, when I tried to run ninja, I got:

fatal error: ortools/linear_solver/linear_solver.h: No such file or directory
    9 | #include "ortools/linear_solver/linear_solver.h" 

So I guess we have to install ortools now? (Edit. I just saw): CIRCT says you can optionally install Or Tools on the getting started page: https://circt.llvm.org/docs/GettingStarted/ (you have to scroll a bit to see where they talk about the or-tools)

[sampsyo]

Awesome; nice work pushing this forward!! As ever, there's "just one more thing" on the way to installation… fortunately, it looks like the CIRCT people have a utils/get-or-tools.sh script that is supposed to download and build it?? Hopefully that works?

[calebmkim]

So I ran the utils/get-or-tools.sh script (I had to change cmake to /ur/bin/cmake in the script). It builds the or-tools in a the directory hir-dev/circt/ext/include

However, when I try to build using ninja it isn't recognizing the #include "ortools/linear_solver/linear_solver.h" file.

fatal error: ortools/linear_solver/linear_solver.h: No such file or directory
    9 | #include "ortools/linear_solver/linear_solver.h"

I think I need to add something to so that the path hir-dev/circt/ext/include can be included as a directory to look in when we #include things, but I'm not that experienced with this sort of stuff so I'm not quite sure how. (I searched online and tried to find some StackOverflow answers like this one but I was still kind of confused).

(fyi if anyone wants to check this out and doesn't want to waste time building llvm you can go to /scratch/cmk265/learning/hir-dev on havarti).

[sampsyo]

Always annoying. It is certainly true that CMake is supposed to be responsible for setting up all those include flags. But one simple/hacky way around this, which sometimes works, is to set the CXXFLAGS (or CFLAGS, depending) environment variable.

You may need this both when running cmake and when running ninja to do the actual build. So it would look like:

CXXFLAGS=-I`pwd`/circt/ext/include cmake -GNinja ...
CXXFLAGS=-I`pwd`/circt/ext/include ninja

…or something roughly along those lines.

[calebmkim]

Ok, sorry to keep on bringing up problems... but running CXXFLAGS=-I$PWD/../ext/include ninja is still not working.

I've attached the super long error message I'm getting below, but one thing I've noticed that's kind of weird:

For some of the commands, you can see that -I/scratch/cmk265/learning/hir-dev/circt/ext/include is part of the command, meaning that the path we're trying for is included... but for other commands, this include flag isn't included (and that's what's generating the error complaining the ortools aren't installed). Not sure what's going on.

I may just give up for today and try again tomorrow.

error.txt

[sampsyo]

Absolutely no problem; that's the way things go with this kind of complicated third-party software setup!! Just to confirm, I'm reproducing the relevant command from the log:

FAILED: lib/CAPI/Dialect/CMakeFiles/obj.CIRCTCAPIHWArith.dir/HWArith.cpp.o 
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -DMLIR_CAPI_BUILDING_LIBRARY=1 -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/scratch/cmk265/learning/hir-dev/circt/llvm/llvm/include -I/scratch/cmk265/learning/hir-dev/circt/llvm/build/include -I/scratch/cmk265/learning/hir-dev/circt/llvm/mlir/include -I/scratch/cmk265/learning/hir-dev/circt/llvm/build/tools/mlir/include -I/scratch/cmk265/learning/hir-dev/circt/include -I/scratch/cmk265/learning/hir-dev/circt/build/include -fno-exceptions -fno-rtti -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wimplicit-fallthrough -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wmisleading-indentation -fdiagnostics-color -g -std=gnu++17 -fvisibility=hidden  -fno-exceptions -fno-rtti -MD -MT lib/CAPI/Dialect/CMakeFiles/obj.CIRCTCAPIHWArith.dir/HWArith.cpp.o -MF lib/CAPI/Dialect/CMakeFiles/obj.CIRCTCAPIHWArith.dir/HWArith.cpp.o.d -o lib/CAPI/Dialect/CMakeFiles/obj.CIRCTCAPIHWArith.dir/HWArith.cpp.o -c /scratch/cmk265/learning/hir-dev/circt/lib/CAPI/Dialect/HWArith.cpp
In file included from /scratch/cmk265/learning/hir-dev/circt/include/circt/Conversion/SchedulingAnalysis.h:4,
                 from /scratch/cmk265/learning/hir-dev/circt/include/circt/Conversion/AffineToHIR.h:13,
                 from /scratch/cmk265/learning/hir-dev/circt/include/circt/Conversion/Passes.h:16,
                 from /scratch/cmk265/learning/hir-dev/circt/lib/CAPI/Dialect/HWArith.cpp:6:
/scratch/cmk265/learning/hir-dev/circt/include/circt/Conversion/SchedulingUtils.h:9:10: fatal error: ortools/linear_solver/linear_solver.h: No such file or directory
    9 | #include "ortools/linear_solver/linear_solver.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

And indeed, there are a lot of -I flags but not the one we tried to add. Annoying!!

Diving a little more into the CMakeLists.txt crud, I note that this is where they are setting up the ORTools dependency: https://github.com/mcl-csa/circt-dev/blob/6d23d18939dfce26de5259454aab0a4b36cf18b4/CMakeLists.txt#L386-L408

This makes me wonder if any of the following things could help:

• setting OR_TOOLS_DISABLE to true
• setting OR_TOOLS_PATH to the relevant path
• failing that, trying to log the value of ortools_FOUND and related variables during CMake-ninja-generation time to try to diagnose what's going on

[calebmkim]

Finally got hir to run! I just lowered one of their designs to the CIRCT hw dialect. Edit: not sure how well their affine-to-hir stuff works, but I'll see if it does.

(documenting what I needed to do for future reference) The main thing was that, in the hir-dev repo, the circt submodule leads you to their lab's fork of CIRCT as expected. But it checks out some random branch 972861b... but you can see that their fork's default branch is called hir, which has more recent changes to it. So once I checked out that branch things started to build properly.

Also needed to add an include_directories({or tools path}) command to the CMakelists.txt file.

[calebmkim]

Here's my best guess at what's going on with the HIR stuff.

• For the benchmarks in the paper, they generated the HIR code either by a) handwriting HIR or b) using something other than the Affine-to-HIR stuff. I'm basing this off how they describe the benchmarks in the paper and looking at the history of the AffineToHIR.cpp file, it appears they've been slowly working on it over the past year or so, well after the paper was written. They then lowered these HIR designs to Verilog using a pass they mention in their paper (but not HIR-to-HW in CIRCT.).
• My guess is that the Affine-to-HIR and HIR-to-HW passes (at least the stuff on the public Github Repo) is still a WIP. Of course, the errors when we try it could be a mistake on our end. But based on the code I've been looking at in the GH repo, this is my best guess.
• This means we probably can't do a direct comparison between our flows from the same Affine code.
• Of course, we could try to compare their HIR code against something we produce. In this case, we would probably want to find the HIR-to-Verilog pass that they mention in their paper and run that ... but we'd have to find that.

[sampsyo]

That's super interesting; thank you for the detailed report. It is too bad (although actually kind of understandable) that there is a gap between the "just make stuff work once" passes and the more polished stuff they are trying to upstream.

I agree with this bottomline assessment:

Of course, we could try to compare their HIR code against something we produce. In this case, we would probably want to find the HIR-to-Verilog pass that they mention in their paper and run that ... but we'd have to find that.

That is, we should give up on any prospects of getting the affine-to-HIR translation working. It seems like there could be some hope of getting the (old, not-upstreamed) HIR-to-Verilog translation going. IIUC, that would only admit any kind of comparison to benchmarks that they have already written, right? Do you have a sense of whether any of those benchmarks overlap with stuff we already have running?

IMO at this point, it is probably wise to make "plan A" be giving up on this comparison altogether and reporting that the open-source stuff doesn't actually work to lower affine programs to RTL. It would be nice to work around this but I think we have officially done our due diligence to conclude that straightforward comparisons are not currently possible.

[calebmkim]

Do you have a sense of whether any of those benchmarks overlap with stuff we already have running?

The benchmarks in their paper are Matrix transpose, stencil-1d, histogram, GEMM, Convolution, Fifo. So there's overlap w/ GEMM.

IMO at this point, it is probably wise to make "plan A" be giving up on this comparison altogether and reporting that the open-source stuff doesn't actually work to lower affine programs to RTL.

Yep, I agree with this. I think giving up might be an ok default.

[rachitnigam]

Agreed!