Benchmarks not currently mapping in master

HamasWaqar commented 4 weeks ago

As Omkar pointed out, Conv Balance benchmarks do not seem to be mapped on the CGRA even though the previous pull request from the "AddingConstant" does seem to map the benchmark on CGRA.

To reproduce this: ./run_gramm.sh Kernels_Modified/Conv_Balance/conv_nounroll_Balance.dot 8 8 RIKEN_with_pins > ./logs/log.txt

HamasWaqar commented 4 weeks ago

So, there is a bug in the current modified_benchmarks. What happened is that the store has one input pin labelled "inPinA". However, the benchmarks use the pin name "inPinB" for the store which does not exist, hence the mapping fails. I will need to modify the script that created this issue.

Note to Hamas: This will need to go as a DRC rule check for GRAMM Application graph

HamasWaqar commented 4 weeks ago

So, there is a bug in the current modified_benchmarks. What happened is that the store has one input pin labelled "inPinA". However, the benchmarks use the pin name "inPinB" for the store which does not exist, hence the mapping fails. I will need to modify the script that created this issue.

Note to Hamas: This will need to go as a DRC rule check for GRAMM Application graph

For now, I would say to verify and rename any store pins in the modified_kernel folder to "inPinA". This will fix the issue until I fix the current scripts and recompile the benchmarks.

HamasWaqar commented 4 weeks ago

So, I kinda figured out why we can not get the map of the DFG onto a 6x4 Riken architecture, and it comes to the constant. Earlier in GRAMM, Jason skipped over the mapping process of the constant. However, in my last push to master, I reinstated constant into the mapping process by removing the following code. In doing so, it is preventing GRAMM from mapping the DFG onto smaller CGRAs. I am currently looking into why this is occurring and hopefully come up with a fix. For now, I have noticed that most of the DFGs should work for 8x8 CGRAs architectures.

ombhilare999 commented 4 weeks ago

Hi @HamasWaqar,

I have couple of questions here.

https://github.com/ombhilare999/GRAMM/blob/aa5dd9305adc2c34c0044d03fa0c9e01b0706352/Kernels_Modified/Conv_Balance/conv_nounroll_Balance.dot#L57

I've been experimenting with the benchmark conv_nounroll_Balance, and I've noticed that the correct loadPin attribute seems to be set correctly on other branches as well for it. Specifically, when running the benchmarks on an 8x8 CGRA, everything works fine, but the 6x6 CGRA doesn't seem to work.

Here are the commands I've tried:

Works: ./run_gramm.sh Kernels_Modified/Conv_Balance/conv_nounroll_Balance.dot 8 8 RIKEN
Doesn't work: ./run_gramm.sh/Kernels_Modified/Conv_Balance/conv_nounroll_Balance.dot 6 6 RIKEN

Based on this, I don't believe the loadPin attribute is the issue.

You mentioned that the presence of constants might prevent GRAMM from mapping the DFG onto smaller CGRAs. However, I'm not sure I fully understand the relationship between constants and the difficulty in mapping larger DFGs onto smaller CGRAs. Isn't constant mapping relatively straightforward? I wouldn't expect it to impact global routing resources significantly.

I'm also working on debugging the exact cause of this issue. Let's discuss and pinpoint some possible errors together here in this thread.

HamasWaqar commented 3 weeks ago

Hi, @ombhilare999, sorry for the late reply. Please read the following to clarify the question asked earlier:

Load Pin Bug for Memport Operation: So, the script I created did randomly assigns load pins if the operand property from the testbench originally was designated to Any2Pins. However, this introduce a bug that I did not released. Memport in the Riken Architecture only have one load pin that is labeled "inPinA". So, if any store operation in the application DFG requires the used of load pin "inPinB", GRAMM will fail since the Memport does not have "inPinB" as a load pin. Please look that the following link in which this issue occurred in the Conv_nounroll.dot benchmark. For all the benchmarks I have compiled, I have fixed this issue, but fixing the python script such that we don't have this issue again. https://github.com/ombhilare999/GRAMM/blob/fe29900d4360d00fa81ed41d1b9048cdbc80b99d/Kernels_Modified/Conv_Balance/conv_nounroll.dot#L57

GRAMM not compiling the for smaller Riken architecture: As you pointed out, the current master branch does not seem to map benchmarks for smaller Riken architecture (i.e. 6x6) in which it should have. I did some investigation and found that this issue is occurring due to the resistance of the constant in GRAMM code.

To help better convay this Omkar, try commenting out lines 660 and 661 in the GRAMM.cpp in the "new_pin_layout_fix" branch. These lines, originally skipped over the constant in the GRAMM mapping hence we were able to mapped to small CGRA sizes. When you comment out these lines to allow Constant to be mapped, then GRAMM is now not able to map to smaller CGRA sizes. https://github.com/ombhilare999/GRAMM/blob/aeac7ffab2cbfc2c426d19900412fe1a0f9e610a/src/GRAMM.cpp#L660-L661

I am still trying to determine why did this occurring. I may be wrong here but based on my investigation, the cost for the const seems really high, ie (1000000) which does not seem reasonable. Trying to figure out why the cost is really high.

Also, I created a separate branch called "fix_pins" in which I am working on to fix this issue. Hamas

ombhilare999 commented 3 weeks ago

Hi @HamasWaqar,

I have fixed the small-CGRA mapping issue in the function_to_pin_mapping branch with commit #d74929a94ccebfceba72a355651ebe47bc4e86ac

The details about the fix can be found here:

before
- For the given application node, when there are no vertex-model present for either fan-in or fan-out nodes, a random suitable(same-opcode) node from device model is returned.
- Implemented in the code as follows:
```
do
{
chooseRand = (chooseRand + 1) % num_vertices(*G);
} while (((*gConfig)[chooseRand].opcode != (*hConfig)[y].opcode));
```

after
- For the given application node, when there are no vertex-model present for either fan-in or fan-out nodes, a random suitable (same-opcode+unused) node from device model is returned.
- Implementaed in the code as follows:

https://github.com/ombhilare999/GRAMM/blob/6bbe8a4b3df17ad52d582e70a1b3fcb96c0611bb/src/GRAMM.cpp#L508-L519

explanation
- During the smaller CGRA mapping process, I noticed that two constants were initially assigned to the same ALU. Because this placement offered the best cost, their positions remained unchanged throughout the Mapping process. As a result, both associated ALU and Load operations also attempted to map to the same ALU, causing same ALU node-cells overlap in every iteration.

DEMO:

Get the recent changes from function_to_pin_mapping
Run ./run_gramm.sh Kernels_Modified/Conv_Balance/conv_nounroll_Balance.dot 6 6 RIKEN
Note that default architecture name RIKEN itself will have pins incorporated into them so RIKEN_with_pins has been removed in the latest commit.
Mapping above benchmark on 6x6 CGRA:

ombhilare999 commented 3 weeks ago

Closing this based on PR #12

UniversalGRAMM / UGRAMM

Benchmarks not currently mapping in master #11