Skip to content

stt: create SteinerTreeBuilder class#862

Merged
eder-matheus merged 56 commits into
The-OpenROAD-Project:masterfrom
eder-matheus:stt_builder
Jul 19, 2021
Merged

stt: create SteinerTreeBuilder class#862
eder-matheus merged 56 commits into
The-OpenROAD-Project:masterfrom
eder-matheus:stt_builder

Conversation

@eder-matheus
Copy link
Copy Markdown
Member

@eder-matheus eder-matheus commented Jul 15, 2021

  • Add a central Steiner tree API that controls the use of pdr and flute

@eder-matheus
stt: update flute namespace
bbad13e 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: move flute files to stt/flt
efa4e4b 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: move pdr to stt/
19727f0 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: mv pdr and flt to src
69be6eb 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: update cmake files
cd79374 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: add initial src files
a3a25e3 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: add Make files
83abffd 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: add stt builder into openroad
2ef8d92 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: remove deprecated pdr files
e137546 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: move pdr swig functions to SteinerTreeBuilder
f657e4e 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: move read/delete LUT to stt builder
8992d7c 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: fix namespaces
cb9b75c
@eder-matheus
stt: fixes during compilation
9244615 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
grt: remove deprecated functions
d308d0e 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
Merge branch 'master' into stt_builder
9e3e88b
@eder-matheus
grt: get alphas from stt builder
71e2fec 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
grt: get tree for stt builder
72ef60e 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
drt: add stt builder object
4432391 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
drt: get/free tree with stt builder
1296d39 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: move flute and pdr to stt lib
bbb5789 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: fix namespaces
5efbcd6 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
grt: fix includes
0e8acd5 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
cts: fix cmake and include
c5ae5d7 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
rsz: add stt builder
03637c0 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
rsz: remove routingAlpha
d68ec86 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
rsz: use stt builder
a7476cb 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: update tests
488cb68 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: add .gitignore
fc3f8b7 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus eder-matheus requested a review from maliberty July 15, 2021 19:02
@eder-matheus
Merge branch 'master' into stt_builder
6f149f2 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/include/stt/SteinerTreeBuilder.h Outdated
jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/SteinerTreeBuilder.cpp Outdated
jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/SteinerTreeBuilder.cpp Outdated
jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/SteinerTreeBuilder.i Outdated
jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/SteinerTreeBuilder.tcl Outdated
jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/flt/flute.cpp Outdated
tp->branch[index[ii]].n = index[tt.branch[ii].n];
}
free(tt.branch);
tt.branch.clear();
Copy link
Copy Markdown
Contributor

@jjcherry56 jjcherry56 Jul 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

despite what the extremely confused pdrev authors think, clear has nothing to do with free. Since the branch vector is owned by the stack allocated variable tt there is no point in clearing it.

jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jjcherry56 jjcherry56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I guess Matt got his way. I would not have bothered with all of this restructuring for 2 callers of a small api. I think there is really no point in switching between flute and pdref, since pdrev with alpha=0 is equivalent (albeit poorer/slower code). The resizer will probably bypass this api at some point and use the pdrev graph instead of the horrible flute data structures.

jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/TritonRoute/src/gr/FlexGR_topo.cpp Outdated
jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/flt/flute.cpp Outdated
minl = ll;
free(bestt1.branch);
free(bestt2.branch);
bestt1.branch.clear();
Copy link
Copy Markdown
Contributor

@jjcherry56 jjcherry56 Jul 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pointless clears

jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/flt/flute.cpp Outdated
} else {
free(t1.branch);
free(t2.branch);
t1.branch.clear();
Copy link
Copy Markdown
Contributor

@jjcherry56 jjcherry56 Jul 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/flt/flute.cpp Outdated

free(bestt1.branch);
free(bestt2.branch);
bestt1.branch.clear();
Copy link
Copy Markdown
Contributor

@jjcherry56 jjcherry56 Jul 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again

jjcherry56
jjcherry56 reviewed Jul 18, 2021
View reviewed changes
Comment thread src/stt/src/SteinerTreeBuilder.cpp Outdated
@eder-matheus
stt: small fixes
e7acb11 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: more fixes
cab3037 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: refactor
69e09ba 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: update pdr api
c07e58b 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus eder-matheus requested a review from jjcherry56 July 18, 2021 18:20
jjcherry56
jjcherry56 reviewed Jul 19, 2021
View reviewed changes
Comment thread src/rsz/src/MakeResizer.cc Outdated
openroad->getDb(),
openroad->getSta(),
openroad->getGlobalRouter());
openroad->getGlobalRouter(),
Copy link
Copy Markdown
Contributor

@jjcherry56 jjcherry56 Jul 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the global router is no longer needed since you moved the only functions it accessed

jjcherry56
jjcherry56 reviewed Jul 19, 2021
View reviewed changes
Comment thread src/stt/CMakeLists.txt Outdated
jjcherry56
jjcherry56 reviewed Jul 19, 2021
View reviewed changes
Comment thread src/stt/src/flt/flute.cpp
@@ -1487,7 +1494,6 @@ Tree flutes_MD(int d, DTYPE xs[], DTYPE ys[], int s[], int acc)
}

minl = (DTYPE)INT_MAX;
bestt1.branch = bestt2.branch = NULL;
Copy link
Copy Markdown
Contributor

@jjcherry56 jjcherry56 Jul 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is where you need the clear

jjcherry56
jjcherry56 reviewed Jul 19, 2021
View reviewed changes
Comment thread src/stt/src/flt/flute.cpp Outdated
@maliberty
Copy link
Copy Markdown
Member

maliberty commented Jul 19, 2021

Well I guess Matt got his way. I would not have bothered with all of this restructuring for 2 callers of a small api. I think there is really no point in switching between flute and pdref, since pdrev with alpha=0 is equivalent (albeit poorer/slower code). The resizer will probably bypass this api at some point and use the pdrev graph instead of the horrible flute data structures.

There are three callers : grt, rsz, and TR-gr
Why would you bypass the API rather than make it better?
It's about centralizing all the logic, of which I am sure there will be more. Per net alpha would need three implementations. If we add any new feature it will need to be added in all places. If we add a new method (say congestion or blockage aware Steiner trees) all clients will have to be updated. Why would we want to do this in multiple places? To quote you "rule #1: don't copy code. refactor". That's exactly what is happening here.

@eder-matheus
rsz: remove grt
a79e972 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus
stt: flute fixes
bb4160f 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus eder-matheus requested a review from jjcherry56 July 19, 2021 14:11
@jjcherry56
Copy link
Copy Markdown
Contributor

jjcherry56 commented Jul 19, 2021

I would have waited until flute was removed as an alternative.
The only code this change factors is 2 if statements in the resizer and global router.
Otherwise it just moves code around.
I think there are way more important issues, like fixing the 2 outstanding bugs in PD.
It is about priorities and this does not rank as a priority.

@eder-matheus
Merge branch 'master' into stt_builder
d4b5fd2 
Signed-off-by: Eder Monteiro <eder.matheus.monteiro@gmail.com>
@eder-matheus eder-matheus merged commit 6fdb00f into The-OpenROAD-Project:master Jul 19, 2021
@eder-matheus eder-matheus deleted the stt_builder branch July 19, 2021 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maliberty maliberty maliberty approved these changes

@jjcherry56 jjcherry56 Awaiting requested review from jjcherry56

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eder-matheus @maliberty @jjcherry56