tancheng / CGRA-Mapper

An LLVM pass that can generate CDFG and map the target loops onto a parameterizable CGRA.
BSD 3-Clause "New" or "Revised" License
53 stars 8 forks source link

Bugs about enabling load/store on specific tiles in CGRA.cpp #22

Closed HobbitQia closed 3 months ago

HobbitQia commented 3 months ago

In CGRA.cpp:104-141, the code enables the load/store on specific CGRA nodes based on param.json and in the code below we count nodes that we enable to perform load or store operations.

            // CGRA.cpp:120
            if ((iter->first).compare("store")) {
              storeCount += 1;
            }
            if ((iter->first).compare("load")) {
              loadCount += 1;
            }

However, since compare method will return 0 if they compare equal, when the condition is true (i.e. The return value does not equal 0) in the if clause, the string in the item of t_additionalFunc should be different from the string "store" or "load". In that case we should not increment the counter storeCount or loadCount. The correct code should be:

            // CGRA.cpp:120
            if ((iter->first).compare("store")==0) {
              storeCount += 1;
            }
            if ((iter->first).compare("load")==0) {
              loadCount += 1;
            }
HobbitQia commented 3 months ago

The similar problems occur at CGRANode.cpp:409-424

bool CGRANode::enableFunctionality(string t_func) {
  if (t_func.compare("store")) {
    enableStore();
  } else if (t_func.compare("load")) {
    enableLoad();
  } else if (t_func.compare("return")) {
    enableReturn();
  } else if (t_func.compare("call")) {
    enableCall();
  } else if (t_func.compare("complex")) {
    enableComplex();
  } else {
    return false;
  }
  return true;
}

It should be:

bool CGRANode::enableFunctionality(string t_func) {
  if (t_func.compare("store")==0) {
    enableStore();
  } else if (t_func.compare("load")==0) {
    enableLoad();
  } else if (t_func.compare("return")==0) {
    enableReturn();
  } else if (t_func.compare("call")==0) {
    enableCall();
  } else if (t_func.compare("complex")==0) {
    enableComplex();
  } else {
    return false;
  }
  return true;
}
tancheng commented 3 months ago

Thanks @HobbitQia, can you please make a PR for this simple change? This set up/PR can also help us later sync on your local changes frequently.

HobbitQia commented 3 months ago

Thanks @HobbitQia, can you please make a PR for this simple change? This set up/PR can also help us later sync on your local changes frequently.

Ok, I have created a PR here.