[Arc] Add Initial Cost Model

[elhewaty]

No description provided.

[elhewaty]

@maerhart @fabianschuiki, What is wrong with the build?

[maerhart]

@maerhart @fabianschuiki, What is wrong with the build?

You are using operations from the Comb and Func dialects but not linking against them in the CMakeLists.txt file. You'd need to add them as link targets there.

[elhewaty]

@maerhart

in lib/Dialect/Arc/CMakeLists.txt

add_circt_dialect_library(CIRCTArc
  ${CIRCT_Arc_Sources}

  ...

  DEPENDS
  ...
  LINK_COMPONENTS
  Support

  LINK_LIBS PUBLIC
  + CIRCTComb
  ...
  ...
  + MLIRFunc
)

[mingzheTerapines]

I was also interested with cost model for the whole project, this looks very nice. However, it seems to be the cost for time dimension, could you add cost for memory-space dimiension, which may be determined by data type or data amount?

[elhewaty]

I was also interested with cost model for the whole project, this looks very nice. However, it seems to be the cost for time dimension, could you add cost for memory-space dimiension, which may be determined by data type or data amount?

@mingzheTerapines Can you please explain a little further? Honestly, I don't get this

[mingzheTerapines]

@elhewaty I mean this is calculating static cost for the model, but if we know the size of input(such as vector) we can know the dynamcia memory/register space cost. Code writen by @angelzzzzz looks like this. You can consider output of function/module is liveout.

void Node::calculateLiveOutCost(const SmallPtrSet<Value, 16> &liveOutValues,
                                float compressionRate) {
  if (liveOutValues.empty()) {
    return;
  }
  int64_t width = 0;
  for (auto value : liveOutValues) {
    width += value.getType().getIntOrFloatBitWidth();
  }
  liveOutCost = width / 64 * compressionRate;
}

[elhewaty]

@mingzheTerapines, It seems an interesting idea, I will try to investigate further. @fabianschuiki what do you think, will this affect the vectors' cost significantly??

I am working on minimizing the shuffling cost in vectors. and I will return to this PR.

[elhewaty]

@fabianschuiki @maerhart PING

[fabianschuiki]

This does not show up as cost in the public interface of the cost model (which is great)! Therefore I would move this into the *.cpp file.

[fabianschuiki]

Suggested change

	DenseMap<Operation *, OperationCosts> opCostCash;
	DenseMap<Operation *, OperationCosts> opCostCache;

[fabianschuiki]

You can often use iterators to combine checking if a value exists in a map, and then doing something with the value if it does exist:

Suggested change

	if (opCostCash.count(op))
	return opCostCash[op];
	if (auto it = opCostCache.find(op); it != opCostCache.end())
	return it->second;

[fabianschuiki]

Suggested change

	else if (isa<arc::VectorizeOp>(op)) {
	// VectorizeOpCost = packingCost + shufflingCost + bodyCost
	OperationCosts inputVecCosts =
	getInputVectorsCost(dyn_cast<VectorizeOp>(op));
	else if (auto vecOp = dyn_cast<arc::VectorizeOp>(op)) {
	// VectorizeOpCost = packingCost + shufflingCost + bodyCost
	OperationCosts inputVecCosts = getInputVectorsCost(vecOp);

[elhewaty]

Thanks for the review, I will update and push it now

[elhewaty]

@fabianschuiki What is wrong with CMakeList file?

[fabianschuiki]

Leftover file 😁?

[elhewaty]

Oh, I will update now. I will finish this PR even it costs my life, it's 12:36 AM here 😭

[fabianschuiki]

LGTM! And CI also seems to be happy 😃 Thanks a lot for iterating on this!

[elhewaty]

Thanks for reviewing, Let's work on the splitting 🚀 🔥

[mingzheTerapines]

@elhewaty Could you add an option for printing the cost explictly in Ops in the future? Maybe using dbg dialect or something.