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
57 stars 8 forks source link

Update DFG::combineForIter() #21

Closed MeowMJ closed 5 months ago

MeowMJ commented 7 months ago

I find two bugs in combineForIter() when I am debugging combineForUnroll() (because they share key codes).

  1. isSuccessor() is used incorrectly. succNode->isSuccessor(headNode) means to tell whether succNode is a successor of headNode. But we actually want to find whether headNode is a successor of succNode to guarantee it is a cycle path.
  2. currentFunc doesn't recover its original position when we finished finding the target pattern under specific dfgNode.
MeowMJ commented 7 months ago

I finished the DFG::combineForUnroll(). One problem, I can't new a DFG node by my code. So I'm not sure if the result fits your idea.

conv (unroll twice) before combineForUnroll: (0)phi->(11)add->(20)add->(0)phi conv-Unroll2-before

conv (unroll twice) after combineForUnroll: (0)phi<->(11)add (11)add->(20)add conv-Unroll2-after

tancheng commented 7 months ago

I didn't get the fusion about 0, 11, 20 DFG nodes. Can we have a sync on this?

MeowMJ commented 5 months ago

I have finished this pull request. Below is an example.

conv.c unroll 2 times before combineForUnroll() convU2

conv.c unroll 2 times after combineForUnroll() convU2-NEW

tancheng commented 5 months ago

Can this currently be used as a parameter/argument set inside the param.json?

tancheng commented 5 months ago

And what about unrolling factor = 3?

tancheng commented 5 months ago

And can you also provide a fused DFG that both the unrolling and the i++ ctrl flow are fused?

tancheng commented 5 months ago

I didn't see my previous comments are addressed?

MeowMJ commented 5 months ago

Can this currently be used as a parameter/argument set inside the param.json?

No. Below is an example using it (in DFG.cpp).

    list<string>* toConbinePattern = new list<string>[5];
    toConbinePattern->push_back("phi");
    toConbinePattern->push_back("add");
    toConbinePattern->push_back("add");
    toConbinePattern->push_back("add");
    toConbinePattern->push_back("add");
    combineForUnroll(toConbinePattern);
MeowMJ commented 5 months ago

And what about unrolling factor = 3?

And what about unrolling factor = 3?

conv.c unroll = 3 before conbineForUnroll() convU3

conv.c unroll = 3 after conbineForUnroll() convU3-NEW

MeowMJ commented 5 months ago

And can you also provide a fused DFG that both the unrolling and the i++ ctrl flow are fused?

conv.c unroll=2 after fuse and combineForIter() and combineForUnroll() convU2-ALL

tancheng commented 5 months ago

For the conv.c unroll = 3 before combineForUnroll(), are we able to fuse 3 nodes out of the 4? Say fusing 28/0/11 without touching 22? There will be still one cycle with length of 2 after your merging, but would relief HW complexity.

And do you know why the edge from 25 to 0 changed its color from blue (ctrl) to red (data)? Can this be fixed?

MeowMJ commented 5 months ago

This is conv.c unroll = 3 fuse 3 nodes convU3-0610

I am trying to figure out why the control edge becomes a data edge.

MeowMJ commented 5 months ago

For the conv.c unroll = 3 before combineForUnroll(), are we able to fuse 3 nodes out of the 4? Say fusing 28/0/11 without touching 22? There will be still one cycle with length of 2 after your merging, but would relief HW complexity.

And do you know why the edge from 25 to 0 changed its color from blue (ctrl) to red (data)? Can this be fixed?

If we want to keep the blue edge, we should guarantee that the nodes of the blue edge are PatternRoot. Otherwise, the blue edge will be replaced by a red edge. See https://github.com/tancheng/CGRA-Mapper/blob/master/src/DFG.cpp#L76

To keep the blue edge, we may need to modify the replaceDFGEdge().

conv.c Unroll = 3 choose phi-add-add-add to fuse convU3-NEW

conv.c Unroll = 3 choose add-phi-add-add to fuse convU3-0611

tancheng commented 5 months ago

ha, so for now, one ctrl edge out of the two edges will become data edge. Then do you think you can fix this in this PR?

MeowMJ commented 5 months ago

I hope I can fix it this week in this PR.

------------------ Original ------------------ From: Cheng Tan @.> Date: Tue,Jun 11,2024 6:10 PM To: tancheng/CGRA-Mapper @.> Cc: MeowMJ @.>, Author @.> Subject: Re: [tancheng/CGRA-Mapper] Update DFG::combineForIter() (PR #21)

ha, so for now, one ctrl edge out of the two edges will become data edge. Then do you think you can fix this in this PR?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

tancheng commented 5 months ago

Thanks!

MeowMJ commented 5 months ago

I changed replaceDFGEdge() to make sure that every edge being replaced can keep their ctrl property.

conv.c unroll=3 after combineForUnroll() convU3-0612