Bug 217 - create a "ring" system which allows pad locations to be specified conveniently
Summary: create a "ring" system which allows pad locations to be specified conveniently
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Hardware Layout (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- enhancement
Assignee: Jock Tanner
URL:
Depends on:
Blocks: 178
  Show dependency treegraph
 
Reported: 2020-03-12 11:57 GMT by Luke Kenneth Casson Leighton
Modified: 2022-07-31 18:22 BST (History)
3 users (show)

See Also:
NLnet milestone: ---
total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for this task, excluding subtasks' budget: 0
parent task for budget allocation:
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:


Attachments
I think this picture is correct (80.92 KB, image/png)
2020-03-17 18:37 GMT, Jock Tanner
Details
That's how I see it might be done (31.43 KB, image/png)
2020-04-06 15:25 BST, Jock Tanner
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Kenneth Casson Leighton 2020-03-12 11:57:17 GMT
see http://bugs.libre-riscv.org/show_bug.cgi?id=178#c206

we need a way to be able to conveniently and easily specify where, for any given block, its inputs and output points (known as "Pads") are to be located.

examples:

* auto-specifying that all "inputs" (to be identified by a matching pattern in the pin name "_i") are to go on NORTH, and that all "outputs" (matching the pattern "_o") are to go on SOUTH

* signals a[0..15] are to go onto the LEFT side of NORTH whilst signals b[0..15] are to go on to the RIGHT side of NORTH

* signals a[0..15] and b[0..15] are to be interleaved (zip(a,b)) with a spacing gap of 5 between a[0],b[0] and a spacing gap of *20* between a[0],b[0] and a[1],b[1] ...

* just as in ioring.py, a list of signals is to be evenly-spaced along a given side.

that first one might be best done by providing a "helper" routine which returns a subset of pin signals based on regex pattern-matching (not that i like regex's but hey)

i don't know whether it's best to suggest this is done by way of one class, or by way of several classes, or by way of "specifiers" that could end up in a dictionary (like ioring) or something else.

whatever it is, the way to specify what to do needs to be quite compact, and easily readable.
Comment 1 Luke Kenneth Casson Leighton 2020-03-12 16:39:09 GMT
moving discussion from http://bugs.libre-riscv.org/show_bug.cgi?id=178#c208

(In reply to Jean-Paul.Chaput from comment #208)
> (In reply to Jean-Paul.Chaput from comment #207)
> > Created attachment 35 [details]
> > Coriolis2 ioring.py with explicit pad positioning
> 
> > > Second, I don't think that introducing an additional file with a special
> > > format would do us any good. I suppose that 'coriolis2.ioring' is ok as it
> > > is for passing pad configuration between stages. If I am just not aware of
> > > the issues with it, please point me in a right direction.
> > 
> > the issue with ioring.py "as-is", is that the procedure for laying out the
> > pads is entirely automatic, spaced-out evenly, because it's designed
> > *exclusively* for doing Quad Flat Packages (QFPs).
> > 
> > what it can't do is something like this:
> > 
> > https://cdn.sparkfun.com/r/600-600/assets/7/a/6/9/c/51c0d009ce395feb33000000.
> > jpg
> > 
> > that's an *uneven* distribution of the pads.
> 
>   Wrong! ;-)

ah *ha*!

>  You can specify the exact position of each pad with the
> ioring.py
>   file. I did put an example in attachement #35. It is not part of the
> toolkit
>   as the related example is the adder with AMS 350nm (under NDA).

okaay.  so the positions are specified in coordinates along that edge.  excellent.

>   I did not completely follow the thead about CSV file, but in my experience,
>   I don't see the need of introducing a new file format (even if its simple
>   CSV), directly write in Python instead.

the format of the ioring.py (which we didn't know about) covers it.

and, that leads me to investigate more thoroughly (grep -r "ioring.py" src/coriolis-2.x) and i find this:

cumulus/src/plugins/chip/Configuration.py

ta-daaaa, ChipConf does pretty much (nearly) everything we need, jock.

which would leave just some "convenience" routines that help us to find
and construct the pad names, on which side.


>   In my experience again, there is too many different cases so that an
> automated
>   way works for for enough of them. And I'm very reluctant to put semantic in
>   the pad names, it may clash with the HDL in too many ways.

yes this is why i suggested a pattern-match "helper" routine

 
>    Lastly, if I'm not mistaken, you want to put information about the pads at
>    nMigen level. I think it should be avoided. nMigen is for the design
>    (behavioral or high leval description), and information pertaining to
>    the layout should be kept at Coriolis level.

i was tempted to suggest putting the *chip* information (which pads, which side) into the structure-of-classes-which-happens-to-be-in-nmigen

why?  because the development is quite closely tied together

however a good case can be made for keeping the pads information completely separate (in the soclayout repository)
Comment 2 Luke Kenneth Casson Leighton 2020-03-12 16:57:18 GMT
so, jock, just a bit of background:

we have a *massive* class hierarchy (well over 250 class instances, all of which need to be laid out), which will produce something like 500,000 gates, which is far too much to do all in one go.

so we want to split down the "tree" of class instances into progressively smaller blocks, not going completely mad about it, maybe two levels deep.  this will mean that the number of class instances ("blocks") that need to be placed *by hand* is  reduced down to maybe... 50 to 80.

the "top level" is called a "floor plan", and we need to best organise the routing between all those "blocks", so that their inputs and outputs are all on the "most efficient side".

the last thing we want, for example, is that to get the I/O to a particular block, it has to go *all the way round* the outside of a block.

some of the data buses will be a whopping FOUR TO FIVE HUNDRED wires (or more), which will be completely dominating the centre of the chip, branching out.

sometimes, for smaller blocks, we definitely want the wires to be evenly spread out across the whole of the side of the block (just like in benchs/nmigen/ALU16)

sometimes, we want a bus (address / data / control lines) to come in on a *specific* location - the corner, or the middle - of a block, on one particular side.

so this is what we need to be able to specify.


also, we need to ensure that the auto-router does *not* ignore the pads (which will go on the very outer edge of each Block), and that means putting BLOCKAGEs in, as "rings", around the outside of the Block.

jock: see /alliance-check-toolkit/benchs/nmigen/ALU16/doAlu16.py for an example.

looking at class ChipConf: Jean-Paul, it is not clear whether ChipConf will automatically create the full range of BLOCKAGEs nets.  i am grepping the source code "self.corona" and there is no evidence of self.corona actually being set!

ChipConf.findPowerAndClockNets() *uses* self.corona, however there's no *setting* of self.corona!

what gives?
Comment 3 Luke Kenneth Casson Leighton 2020-03-12 17:07:57 GMT
(sotto voice: so for what we are doing, we don't use plugins.chip.Configuration.loadConfiguration, we use plugins.chip.Configuration.ChipConf *directly*.
Comment 4 Jean-Paul Chaput 2020-03-12 17:24:02 GMT
I did miss some posts.

self.corona is an accessor property (defined in Configuration.py,
line 663). It's the first element of the self.coronas list,
which initialized in the checkPads() function at line 1008.
Rule is simple: any instance which is not a pad is a corona.
And there should be only one.

I did lost a post about HDL langiage. No time to rewrite it right
now but this is a question we asked ourselves too. Until now,
we are not happy with anything that exists.
Comment 5 Luke Kenneth Casson Leighton 2020-03-12 17:37:18 GMT
(In reply to Jean-Paul.Chaput from comment #4)
> I did miss some posts.

oops.

> self.corona is an accessor property (defined in Configuration.py,
> line 663). It's the first element of the self.coronas list,
> which initialized in the checkPads() function at line 1008.
> Rule is simple: any instance which is not a pad is a corona.
> And there should be only one.

okaaay.  right.  yes.  sorry, i was searching "self.corona".  of course,
properties would not show up.
 
> I did lost a post about HDL langiage. No time to rewrite it right
> now but this is a question we asked ourselves too. Until now,
> we are not happy with anything that exists.

interesting.  yes, on the one hand, the design and layout can be closely
tied.  on the other, they are separate.

if you separate them too much, you lose track on the layout side.

here, really, we have the same analogy - and exactly the same problem - between PCB Schematic and PCB Layout.

often these two get out-of-sync and it is a bitch.

and you just... deal with it.
Comment 6 Luke Kenneth Casson Leighton 2020-03-13 11:36:57 GMT
ok, jock, i thought about it: i have a specific task for you :)

* take benchs/nmigen/ALU16 and create (cut/paste) experiments7
  in soclayout
* in doAlu16.py, in each of add(), sub() and alu16(), *replace*
  the hand-coded addition of Pins (Pin.create) with explicit
  use of ChipConf from cumulus/src/plugins/chip/Configuration.py

the only thing to watch out for is that BLOCKAGE2 through BLOCKAGE5
are added in add() and sub().  it *may* be necessary to create a
separate function for that, we just have to see.

if necessary, for now, take a copy of Configuration.py and add it
to experiments7 in the soclayout repo, so that you can edit it.
or, better (preferred), subclass it.

just get stuck in, and see how that goes?
Comment 7 Jock Tanner 2020-03-16 22:01:50 GMT
(In reply to Luke Kenneth Casson Leighton from comment #6)
> ok, jock, i thought about it: i have a specific task for you :)
> 
> * take benchs/nmigen/ALU16 and create (cut/paste) experiments7
>   in soclayout
> * in doAlu16.py, in each of add(), sub() and alu16(), *replace*
>   the hand-coded addition of Pins (Pin.create) with explicit
>   use of ChipConf from cumulus/src/plugins/chip/Configuration.py
> 
> the only thing to watch out for is that BLOCKAGE2 through BLOCKAGE5
> are added in add() and sub().  it *may* be necessary to create a
> separate function for that, we just have to see.
> 
> if necessary, for now, take a copy of Configuration.py and add it
> to experiments7 in the soclayout repo, so that you can edit it.
> or, better (preferred), subclass it.
> 
> just get stuck in, and see how that goes?

Luke and everybody,

I played with the example for several days, but I afraid I have no answers, but only more questions.

First of all, I'm not sure what the result of the 'doAlu16.py' script should be, and how running of the script relates to running 'make' with different targets. I thought I can find some documents on this, but I couldn't.

I think the script is not working anyway. The most I can get out of it is

> [ERROR] NegociateWindow::createTrackSegment(): No track near axis of <id:7676 Horizontal b(15) METAL2 [320l 400l] [320l 400l] 2l rpD:0 ----UC---T-----ri-tt-> (after adjust).

Is something wrong with my setup?

Oh, and it was only after I fixed an error with forgotten import:

(~/alliance-check-tookit/benchs/nmigen/ALU16/doAlu16.py:523)
>     showPythonTrace( __file__, e, False )

but 'showPythonTrace()' is not defined.

I thought I could get something out of documentation, but it seems very scarce to me. I could not find an API reference manual (neither Python nor C++), but then I was starting to think it wouldn't help me either.

It also makes me wonder why all the examples are totally devoid of meaningful comments or docstrings, as if they aren't meant for didactic purposes, but just for spinning off new devices. Like boilerplates. Except they are too big for boilerplates, and not exactly working.

I hope I was able to somehow address the issue of creating the layer Luke mentions:

https://git.libre-riscv.org/?p=soclayout.git;a=blob;f=experiments7/doAlu16.py;hb=HEAD#l25

But otherwise I am lost. And the overall quality of the Python code I'm dealing with in Coriolis makes me want to poke my eyes out. I'm terribly sorry if I offended someone, but at the same time I'm sure I'm not the only one saying this. I wonder why it is so bad and where to start improving it.
Comment 8 Jean-Paul Chaput 2020-03-16 23:06:38 GMT
(In reply to Jock Tanner from comment #7)
> (In reply to Luke Kenneth Casson Leighton from comment #6)
> > ok, jock, i thought about it: i have a specific task for you :)
> > 
> > * take benchs/nmigen/ALU16 and create (cut/paste) experiments7
> >   in soclayout
> > * in doAlu16.py, in each of add(), sub() and alu16(), *replace*
> >   the hand-coded addition of Pins (Pin.create) with explicit
> >   use of ChipConf from cumulus/src/plugins/chip/Configuration.py
> > 
> > the only thing to watch out for is that BLOCKAGE2 through BLOCKAGE5
> > are added in add() and sub().  it *may* be necessary to create a
> > separate function for that, we just have to see.
> > 
> > if necessary, for now, take a copy of Configuration.py and add it
> > to experiments7 in the soclayout repo, so that you can edit it.
> > or, better (preferred), subclass it.
> > 
> > just get stuck in, and see how that goes?
> 
> Luke and everybody,
> 
> I played with the example for several days, but I afraid I have no answers,
> but only more questions.
> 
> First of all, I'm not sure what the result of the 'doAlu16.py' script should
> be, and how running of the script relates to running 'make' with different
> targets. I thought I can find some documents on this, but I couldn't.

  The end result of this script is a "layout", that is literally the
  drawing you will send to the factory (foundry) to be fabricated. It is stored
  in the various ".ap" files (symbolic layout). They must be coherent with the
  associated ".vst" (nelist) files.


> I think the script is not working anyway. The most I can get out of it is
> 
> > [ERROR] NegociateWindow::createTrackSegment(): No track near axis of <id:7676 Horizontal b(15) METAL2 [320l 400l] [320l 400l] 2l rpD:0 ----UC---T-----ri-tt-> (after adjust).
> 
> Is something wrong with my setup?

  Obviously. Because it works on my end. I will redo the whole install
  process under Debian 10 in the next few days to see if I can reproduce
  your problem.


> Oh, and it was only after I fixed an error with forgotten import:
> 
> (~/alliance-check-tookit/benchs/nmigen/ALU16/doAlu16.py:523)
> >     showPythonTrace( __file__, e, False )
> 
> but 'showPythonTrace()' is not defined.

  That is right, I will correct it.


> I thought I could get something out of documentation, but it seems very
> scarce to me. I could not find an API reference manual (neither Python nor
> C++), but then I was starting to think it wouldn't help me either.

  You can find the documentation here: http://coriolis.lip6.fr/
  The C++ API is exported almost exactly "as is" in Python.
  There is also the documentation of alliance-check-toolkit.


> It also makes me wonder why all the examples are totally devoid of
> meaningful comments or docstrings, as if they aren't meant for didactic
> purposes, but just for spinning off new devices. Like boilerplates. Except
> they are too big for boilerplates, and not exactly working.

  Both true and false. alliance-check-toolkit, as it's name hints was started
  as a kind of regression test and tookit for people already acquainted with
  the ASIC design flow. In that sense it is not pedagogical.
    In the other hand, for people familiar with it, the target names are
  almost self explanatory.

 
> I hope I was able to somehow address the issue of creating the layer Luke
> mentions:
> 
> https://git.libre-riscv.org/?p=soclayout.git;a=blob;f=experiments7/doAlu16.
> py;hb=HEAD#l25
> 
> But otherwise I am lost. And the overall quality of the Python code I'm
> dealing with in Coriolis makes me want to poke my eyes out. I'm terribly
> sorry if I offended someone, but at the same time I'm sure I'm not the only
> one saying this. I wonder why it is so bad and where to start improving it.

  Coriolis has always been tricky to install from sources (but try to build
  the whole Gnome3 stack instead of using packaged versions to make a fair
  comparison). As I happen to manage Cadence/Mentor/Synopsys in my lab,
  I assure you, that they are not simpler, by a long shot (with a license
  fee around 100K$ a year, for commercial purpose).
    That context being setup, I know myself to have a tendency to make things
  too complicated, but it is difficult to improve without feedback.
    I'm always eager to improve the quality of my software and coding style,
  but ranting about "this is badly written" do not help. I flatter myself
  to have the "scientific spirit", so if you pinpoint me badly written code
  and *why* it is so (with reference), I will improve it in my futur code
  and progressively rewrite the old one.
Comment 9 Luke Kenneth Casson Leighton 2020-03-16 23:25:26 GMT
(In reply to Jock Tanner from comment #7)

> I played with the example for several days, 

ok - don't leave it several days.  we don't have time for "i can't get it to work therefore i'm going to keep quiet and spend time because i'm embarrassed to admit i can't get it to work".

the *moment* you can't get something to work, please say so *immediately*, on here.  this is *really* important.

> but I afraid I have no answers,
> but only more questions.

great.

> First of all, I'm not sure what the result of the 'doAlu16.py' script should
> be, and how running of the script relates to running 'make' with different
> targets. 

you need to run "make lvx" then "make view".  nothing else: absolutely nothing else.

you *might* have to run "make clean" if something has not worked because occasionally the intermediary files are modified such that on subsequent runs they affect ongoing output.

> I thought I can find some documents on this, but I couldn't.

that's what the tutorial is for.

> I think the script is not working anyway. The most I can get out of it is
> 
> > [ERROR] NegociateWindow::createTrackSegment(): No track near axis of <id:7676 Horizontal b(15) METAL2 [320l 400l] [320l 400l] 2l rpD:0 ----UC---T-----ri-tt-> (after adjust).

ah.  that's a bug, that needs to be reported to jean-paul.

he will need the *EXACT* git version/revision of all repositories that you have checked out (alliance, alliance-check-toolkit, coriolis2)

so, for example: i am using:

toolkit

commit abec1bea9643d69ac231791ad8a30c964a236794
Merge: b2380d6 a90033f
Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
Date:   Wed Mar 4 01:40:39 2020 +0100

    Merge branch 'master' of gitlab.lip6.fr:vlsi-eda/alliance-check-toolkit


alliance

commit 93cd988d07037b71c3b8497c4c376b1d7d151f71
Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
Date:   Wed Nov 13 17:47:22 2019 +0100

    Stable AP read/write generation.
    


coriolis-2

commit 59ee8358cac8254dcd999a945e07ef717573e32e
Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
Date:   Wed Mar 4 00:50:18 2020 +0100

    First working recursive place & route (Libre-SOC ALU16 benchmark).
    


and i just "git pulled" soclayout (and pushed an add of a symlink to mksyms.sh)

aaaand.... it works perfectly.  "make lvx" followed by "make view" does what is expected.

now let me do a git pull and rebuild on all three repos (one at a time, first).

.... arse.  alliance-check-toolkit critically relies on updates in coriolis2.  arse.  ok have to do all three.

ok coriolis-2 is a big update it seems: alliance not so much.

ok that's built: i re-ran experiments7 (after running mksyms.sh and "make clean" and, again, it worked perfectly.


> Is something wrong with my setup?

we don't know.  do "make clean", "make lvx" and (only if that works) "make view".

coriolis2 is *supposed* to be absolutely replicable (deterministic) and so the output is *not* supposed to "go wrong" on one system and "go right" on another.


> Oh, and it was only after I fixed an error with forgotten import:
> 
> (~/alliance-check-tookit/benchs/nmigen/ALU16/doAlu16.py:523)
> >     showPythonTrace( __file__, e, False )
> 
> but 'showPythonTrace()' is not defined.

generally this kind of approach - catching an exception then showing it - is very bad practice.  it's better to let the exception happen, where it happens.  the exit code *should* be non-zero on exception which is exactly what is wanted anyway:

lkcl@fizzy:$ python -c "x = 5 / 0"; echo $?
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ZeroDivisionError: integer division or modulo by zero
1
lkcl@fizzy:$ python -c "x = 5 / 1"; echo $?
0

yyyep, non-zero exit code on exception.  therefore, doing "catch exception then do sys.exit(code)" is completely unnecessary.


> I thought I could get something out of documentation, but it seems very
> scarce to me. I could not find an API reference manual (neither Python nor
> C++), but then I was starting to think it wouldn't help me either.

it's in HTML in the repository.  locate a docs dir.  however... yes, it doesn't help exactly.

> It also makes me wonder why all the examples are totally devoid of
> meaningful comments or docstrings, as if they aren't meant for didactic
> purposes, but just for spinning off new devices. Like boilerplates. Except
> they are too big for boilerplates, and not exactly working.

time, basically.  jean-paul's a little overwhelmed with other tasks.


> I hope I was able to somehow address the issue of creating the layer Luke
> mentions:
> 
> https://git.libre-riscv.org/?p=soclayout.git;a=blob;f=experiments7/doAlu16.
> py;hb=HEAD#l25
> 
> But otherwise I am lost. And the overall quality of the Python code I'm
> dealing with in Coriolis makes me want to poke my eyes out. 

i know.  it absolutely seriously needs autopep8 run on it, big-time - the whole lot.  (except autopep8 has gone downhill as its developer is failing to respond to bugreports).

i haven't mentioned it yet, because autopep8, if run incorrectly, can do quite a bit of readability damage (due to its developer not properly listening and not fixing reported bugs that have been there for a minimum of eight years, even when i reported a brain-dead trivial repro case)


> I'm terribly
> sorry if I offended someone, but at the same time I'm sure I'm not the only
> one saying this. I wonder why it is so bad and where to start improving it.

at the right time.

or if it's seriously intolerable, we do have a budget available for fixing coriolis2 itself.  it needs coordinating with jean-paul and marie-minerve at lip6.
Comment 10 Luke Kenneth Casson Leighton 2020-03-16 23:38:13 GMT
(In reply to Jean-Paul.Chaput from comment #8)

>   Obviously. Because it works on my end. I will redo the whole install
>   process under Debian 10 in the next few days to see if I can reproduce
>   your problem.

i forgot to say, jean-paul: i have debian/10 now as well. the "export LD_LIBRARY_PATH=....lib export LD_LIBRARY_PATH= ... lib64" hack shown here makes the install "work" with no mods needed to alliance/coriolis2
https://libre-riscv.org/HDL_workflow/coriolis2/

also, we are doing soclayout/experiment7 which is a duplicate of benchs/nmigen/alu_hier (for now)


>  
> > I hope I was able to somehow address the issue of creating the layer Luke
> > mentions:
> > 
> > https://git.libre-riscv.org/?p=soclayout.git;a=blob;f=experiments7/doAlu16.
> > py;hb=HEAD#l25
> > 
> > But otherwise I am lost. And the overall quality of the Python code I'm
> > dealing with in Coriolis makes me want to poke my eyes out. I'm terribly
> > sorry if I offended someone, but at the same time I'm sure I'm not the only
> > one saying this. I wonder why it is so bad and where to start improving it.
> 
>   Coriolis has always been tricky to install from sources (but try to build
>   the whole Gnome3 stack instead of using packaged versions to make a fair
>   comparison). 

eek! :)

>   but ranting about "this is badly written" do not help. I flatter myself
>   to have the "scientific spirit", so if you pinpoint me badly written code
>   and *why* it is so (with reference), I will improve it in my futur code
>   and progressively rewrite the old one.

none of the python code is pep8 compliant, jean-paul.  the 2-spaces indentation is extremely hard on the eyes for anyone used to the pep8 standard (because the smaller indentation makes "code blocks" that much harder to visually identify).

and the use of commas at the *beginning* of the line on a parameter list, rather than the usual place at the *end* of the line, is very difficult to get used to.  i find that, automatically, i add the comma at the end (because that is the pep8 standard) and of course it creates a syntax error because there is a *second* (unexpected) comma at the start of the next line.

a single run of autopep8 will "fix" that however any lines that have to be wrapped (due to length over-run) can be "mashed" somewhat in an inappropriate fashion, by putting function parameters onto a single line (forcibly).

this is done in a lot of places anyway (single line per function parameter) so it may not be so bad.

however... i haven't mentioned any of this because i felt it was more important to get working "stuff" within the context of what time you have and also knowing that you have... "memorised" the python code in this (unusual, non-standard) format... and moving to strict pep8 would be quite disruptive and time-consuming (for you to get used to).

it would need careful scheduling because, really, you need to hit the entire codebase with a single carefully-reviewed autopep8 update and commit, and not do *anything* else in between.

if there is a good time to schedule that, we can have several people help out with it.
Comment 11 Jock Tanner 2020-03-17 00:05:08 GMT
(In reply to Luke Kenneth Casson Leighton from comment #9)
> ok - don't leave it several days.  we don't have time for "i can't get it to
> work therefore i'm going to keep quiet and spend time because i'm
> embarrassed to admit i can't get it to work".

Roger that. But I can't say I'm much embarrassed (though maybe I should be), because I've seen several such big projects, which seemed cryptic and incomprehensible at first, but opened up with time and observation. Except that this time I seemed to underestimate the domain.
 
> > First of all, I'm not sure what the result of the 'doAlu16.py' script should
> > be, and how running of the script relates to running 'make' with different
> > targets. 
> 
> you need to run "make lvx" then "make view".  nothing else: absolutely
> nothing else.
> 
> you *might* have to run "make clean" if something has not worked because
> occasionally the intermediary files are modified such that on subsequent
> runs they affect ongoing output.

Wait a minute, Luke. I'm doing 'make lvx' and 'make view', and it gives me pretty picture and no errors. But I also run the script itself ('doAlu16.py'), because I remember you said somewhere that we must be able to get the results without GUI. That's where I get the 'No track near axis' error. Should I run it or not?
Comment 12 Jean-Paul Chaput 2020-03-17 00:19:34 GMT
(In reply to Luke Kenneth Casson Leighton from comment #10)

Answering here about showPythonTrace():

I created it because:
* I wanted a more compact displaying of the trace (no line wrap because
  to read more quickly as I debug).
* The script can be executed in three contexts:
  1. GUI
  2. Text with cgt (--script=...)
  3. Direct command line.
    I wanted especially the GUI to be able to recover after
  an exception is raised, to do some forensic.
    It is far from perfect but did help me much while debugging.

> (In reply to Jean-Paul.Chaput from comment #8)
> 
> >   Obviously. Because it works on my end. I will redo the whole install
> >   process under Debian 10 in the next few days to see if I can reproduce
> >   your problem.
> 
> i forgot to say, jean-paul: i have debian/10 now as well. the "export
> LD_LIBRARY_PATH=....lib export LD_LIBRARY_PATH= ... lib64" hack shown here
> makes the install "work" with no mods needed to alliance/coriolis2
> https://libre-riscv.org/HDL_workflow/coriolis2/

  My latest commit should correct that, but I will confirm when I check
  it under Debian 10.

 
> >   but ranting about "this is badly written" do not help. I flatter myself
> >   to have the "scientific spirit", so if you pinpoint me badly written code
> >   and *why* it is so (with reference), I will improve it in my futur code
> >   and progressively rewrite the old one.
> 
> none of the python code is pep8 compliant, jean-paul.  the 2-spaces
> indentation is extremely hard on the eyes for anyone used to the pep8
> standard (because the smaller indentation makes "code blocks" that much
> harder to visually identify).

  I understand, I'm accustomed to it and have no problem with it.
  This is because sometime the nesting could be very deep, and I
  did not want things to go too much to the right.
  I will switch to 4 spaces.

> and the use of commas at the *beginning* of the line on a parameter list,
> rather than the usual place at the *end* of the line, is very difficult to
> get used to.  i find that, automatically, i add the comma at the end
> (because that is the pep8 standard) and of course it creates a syntax error
> because there is a *second* (unexpected) comma at the start of the next line.

  Funny thing, I did put the comma at the beginning because I kept
  forgetting to put it at the end. I find it also much more clear as it
  makes one straight column below the opening parenthesis and is
  more convenient for rectangular selection. And finally it helps
  keep thing vertically aligned so you immediately notice if a
  comma is missing (tabulating at the end is possible but tedious
  because of the variable length or the arguments).
    I understand it goes against a lot of habits and automatism,
  but I think I will keep it.

> a single run of autopep8 will "fix" that however any lines that have to be
> wrapped (due to length over-run) can be "mashed" somewhat in an
> inappropriate fashion, by putting function parameters onto a single line
> (forcibly).
> 
> this is done in a lot of places anyway (single line per function parameter)
> so it may not be so bad.
> 
> however... i haven't mentioned any of this because i felt it was more
> important to get working "stuff" within the context of what time you have
> and also knowing that you have... "memorised" the python code in this
> (unusual, non-standard) format... and moving to strict pep8 would be quite
> disruptive and time-consuming (for you to get used to).

  Yes, and time is always at a premium.

> it would need careful scheduling because, really, you need to hit the entire
> codebase with a single carefully-reviewed autopep8 update and commit, and
> not do *anything* else in between.

  Yes and perform complete regression tests.

> if there is a good time to schedule that, we can have several people help
> out with it.
Comment 13 Jean-Paul Chaput 2020-03-17 00:24:12 GMT
(In reply to Jean-Paul.Chaput from comment #12)
> (In reply to Luke Kenneth Casson Leighton from comment #10)
> 
> Answering here about showPythonTrace():
> 
> I created it because:
> * I wanted a more compact displaying of the trace (no line wrap because
>   to read more quickly as I debug).
> * The script can be executed in three contexts:
>   1. GUI
>   2. Text with cgt (--script=...)
>   3. Direct command line.
>     I wanted especially the GUI to be able to recover after
>   an exception is raised, to do some forensic.
>     It is far from perfect but did help me much while debugging.
> 
> > (In reply to Jean-Paul.Chaput from comment #8)
> > 
> > >   Obviously. Because it works on my end. I will redo the whole install
> > >   process under Debian 10 in the next few days to see if I can reproduce
> > >   your problem.
> > 
> > i forgot to say, jean-paul: i have debian/10 now as well. the "export
> > LD_LIBRARY_PATH=....lib export LD_LIBRARY_PATH= ... lib64" hack shown here
> > makes the install "work" with no mods needed to alliance/coriolis2
> > https://libre-riscv.org/HDL_workflow/coriolis2/
> 
>   My latest commit should correct that, but I will confirm when I check
>   it under Debian 10.
> 
>  
> > >   but ranting about "this is badly written" do not help. I flatter myself
> > >   to have the "scientific spirit", so if you pinpoint me badly written code
> > >   and *why* it is so (with reference), I will improve it in my futur code
> > >   and progressively rewrite the old one.
> > 
> > none of the python code is pep8 compliant, jean-paul.  the 2-spaces
> > indentation is extremely hard on the eyes for anyone used to the pep8
> > standard (because the smaller indentation makes "code blocks" that much
> > harder to visually identify).
> 
>   I understand, I'm accustomed to it and have no problem with it.
>   This is because sometime the nesting could be very deep, and I
>   did not want things to go too much to the right.
>   I will switch to 4 spaces.
> 
> > and the use of commas at the *beginning* of the line on a parameter list,
> > rather than the usual place at the *end* of the line, is very difficult to
> > get used to.  i find that, automatically, i add the comma at the end
> > (because that is the pep8 standard) and of course it creates a syntax error
> > because there is a *second* (unexpected) comma at the start of the next line.
> 
>   Funny thing, I did put the comma at the beginning because I kept
>   forgetting to put it at the end. I find it also much more clear as it
>   makes one straight column below the opening parenthesis and is
>   more convenient for rectangular selection. And finally it helps
>   keep thing vertically aligned so you immediately notice if a
>   comma is missing (tabulating at the end is possible but tedious
>   because of the variable length or the arguments).
>     I understand it goes against a lot of habits and automatism,
>   but I think I will keep it.
> 
> > a single run of autopep8 will "fix" that however any lines that have to be
> > wrapped (due to length over-run) can be "mashed" somewhat in an
> > inappropriate fashion, by putting function parameters onto a single line
> > (forcibly).
> > 
> > this is done in a lot of places anyway (single line per function parameter)
> > so it may not be so bad.
> > 
> > however... i haven't mentioned any of this because i felt it was more
> > important to get working "stuff" within the context of what time you have
> > and also knowing that you have... "memorised" the python code in this
> > (unusual, non-standard) format... and moving to strict pep8 would be quite
> > disruptive and time-consuming (for you to get used to).
> 
>   Yes, and time is always at a premium.
> 
> > it would need careful scheduling because, really, you need to hit the entire
> > codebase with a single carefully-reviewed autopep8 update and commit, and
> > not do *anything* else in between.
> 
>   Yes and perform complete regression tests.
> 
> > if there is a good time to schedule that, we can have several people help
> > out with it.

(In reply to Jock Tanner from comment #11)
> (In reply to Luke Kenneth Casson Leighton from comment #9)
> > ok - don't leave it several days.  we don't have time for "i can't get it to
> > work therefore i'm going to keep quiet and spend time because i'm
> > embarrassed to admit i can't get it to work".
> 
> Roger that. But I can't say I'm much embarrassed (though maybe I should be),
> because I've seen several such big projects, which seemed cryptic and
> incomprehensible at first, but opened up with time and observation. Except
> that this time I seemed to underestimate the domain.
>  
> > > First of all, I'm not sure what the result of the 'doAlu16.py' script should
> > > be, and how running of the script relates to running 'make' with different
> > > targets. 
> > 
> > you need to run "make lvx" then "make view".  nothing else: absolutely
> > nothing else.
> > 
> > you *might* have to run "make clean" if something has not worked because
> > occasionally the intermediary files are modified such that on subsequent
> > runs they affect ongoing output.
> 
> Wait a minute, Luke. I'm doing 'make lvx' and 'make view', and it gives me
> pretty picture and no errors. But I also run the script itself
> ('doAlu16.py'), because I remember you said somewhere that we must be able
> to get the results without GUI. That's where I get the 'No track near axis'
> error. Should I run it or not?

  You cannot (yet) run the script directly. However to run in text mode:
  you can use:
    cgt --script=doAlu16
  (assuming the environment as been correctly set)

  I will make the necessary changes so the script can be run by itself
  (need to request configuation loading).
Comment 14 Jean-Paul Chaput 2020-03-17 00:30:54 GMT
(In reply to Jean-Paul.Chaput from comment #13)
> (In reply to Jean-Paul.Chaput from comment #12)

>   You cannot (yet) run the script directly. However to run in text mode:
>   you can use:
>     cgt --script=doAlu16
>   (assuming the environment as been correctly set)
> 
>   I will make the necessary changes so the script can be run by itself
>   (need to request configuation loading).

Errata:

   cgt --text --script=doAlu16
Comment 15 Jock Tanner 2020-03-17 01:38:16 GMT
(In reply to Jean-Paul.Chaput from comment #8)
>   Coriolis has always been tricky to install from sources (but try to build
>   the whole Gnome3 stack instead of using packaged versions to make a fair
>   comparison). As I happen to manage Cadence/Mentor/Synopsys in my lab,
>   I assure you, that they are not simpler, by a long shot (with a license
>   fee around 100K$ a year, for commercial purpose).
>     That context being setup, I know myself to have a tendency to make things
>   too complicated, but it is difficult to improve without feedback.
>     I'm always eager to improve the quality of my software and coding style,
>   but ranting about "this is badly written" do not help. I flatter myself
>   to have the "scientific spirit", so if you pinpoint me badly written code
>   and *why* it is so (with reference), I will improve it in my futur code
>   and progressively rewrite the old one.

Jean-Paul,

I'm feeling like I'm slowly burying myself in the sand, but instead of bad code I would like to pinpoint you something much more important − bad tools you use.

There are many errors in and around Coriolis, similar to the one I found:

- clrcore/helpers/Alliance.py:199 − gaugeDatasNo is undefined
- cumulus/plugins/chip/BlockCorona.py − a lot of undefined vars
- cumulus/plugins/chip/BlockPower.py − a lot of undefined vars
- etc/coriolis2/common/colors.py − ditto

Most of these errors are outside “happy path”. But I don't have to catch all these errors to see them. In fact I don't have to run the code at all. All I have to do is open the file in Pycharm, and it highlights all kind of problems. The good code is mostly black&white, while the bad code is lit up like a Christmas tree!

'colors.py' shows another typical problem of the project. The first indent is 4 spaces, but all the following indents are 2 spaces. It most probably means that this file once contained a class, that later was refactored into free functions. I do that a lot. I love free functions. And my editor indents/dedents any amount of code with one key press. I don't have to spend my time making my indentation consistent.

Another thing is dead code. It comes in two flavors: it may be either just unreachable or commented out.

- etc/coriolis2/common/patterns.py:109 − unreachable (and meaningless btw)
- soclayout/experiments5/ringoscillator.py:879-914 − also unreachable
- stratus/dpgen_ROM.py:861-916 − commented-out

First flavor reveals itself instantly with code highlight. Second one, well, it kinda have a reason to be there. But if you use version control, that reason is invalid. Especially if you can access any commit in two clicks: “Version control” → “Log”. If you find it hard to just delete the unneeded code, well, you must be using the bad tools.

I won't even get started on Pycharm making hard or even impossible to break PEP8, let alone reformatting any code in an instant. The style is not a most important problem of the project. Bad tools are.

Important disclaimer! I mostly speaking of my lovely Pycharm, because I'm lazy, and it comes configured right out of the box. I know that the same result can be achieved with properly configured vim, Emacs, SublimeText or Notepad++. I just can't recommend all the necessary plugins.

Jean-Paul, if you don't want to kill me already, I can pinpoint the second big problem. Hint: it's not the style.
Comment 16 Jock Tanner 2020-03-17 01:54:06 GMT
(In reply to Jean-Paul.Chaput from comment #14)
> (In reply to Jean-Paul.Chaput from comment #13)
> > (In reply to Jean-Paul.Chaput from comment #12)
> 
> >   You cannot (yet) run the script directly. However to run in text mode:
> >   you can use:
> >     cgt --script=doAlu16
> >   (assuming the environment as been correctly set)
> > 
> >   I will make the necessary changes so the script can be run by itself
> >   (need to request configuation loading).
> 
> Errata:
> 
>    cgt --text --script=doAlu16

Thanks for the hint. It gives me the same error as if I run the script directly:

>[ERROR] An exception occured while loading the Python script module:
>        "doAlu16"
>        You should check for simple python errors in this module.
>        Python stack trace:
>        #0 in             placeAndRoute() at doAlu16.py:48
>        #1 in                       sub() at doAlu16.py:305
>        #2 in                ScriptMain() at doAlu16.py:420
>        #3 in                 runScript() at .../coriolis-2.x/Linux.x86_64/Release.Shared/install/bin/cgt:83
>        Error was:
>          [ERROR] NegociateWindow::createTrackSegment(): No track near axis of <id:7678 Horizontal b(15) METAL2 [320l 400l] [320l 400l] 2l rpD:0 ----UC---T-----ri-tt-> (after adjust).
>        Trying to continue anyway...
Comment 17 Luke Kenneth Casson Leighton 2020-03-17 06:53:22 GMT
(In reply to Jock Tanner from comment #11)
> (In reply to Luke Kenneth Casson Leighton from comment #9)
> > ok - don't leave it several days.  we don't have time for "i can't get it to
> > work therefore i'm going to keep quiet and spend time because i'm
> > embarrassed to admit i can't get it to work".
> 
> Roger that. But I can't say I'm much embarrassed (though maybe I should be),
> because I've seen several such big projects, which seemed cryptic and
> incomprehensible at first, but opened up with time and observation. Except
> that this time I seemed to underestimate the domain.

do keep in touch much more often.  we really should start an irc channel
except i need to make sure it's publicly archived.

> Wait a minute, Luke. I'm doing 'make lvx' and 'make view', and it gives me
> pretty picture and no errors. But I also run the script itself
> ('doAlu16.py'), because I remember you said somewhere that we must be able
> to get the results without GUI. That's where I get the 'No track near axis'
> error. Should I run it or not?

*click*.  sorry, jock - i hard-context-switched for a week and forgot how to run the damn thing:

make clean         # get rid of EVERYTHING
make lvx           # only create the VHDL that the next step needs
python doAlu16.py  # use the experimental code we're in the middle of doing

OR (as jean-paul points out)

make clean                   # get rid of EVERYTHING
make lvx                     # only create the VHDL that the next step needs
cgt --text --script=doAlu16
make cgt followed by manually opening "alu16" 

OR (as i just discovered)

make clean                   # get rid of EVERYTHING
make lvx                     # only create the VHDL that the next step needs
cgt --script=doAlu16

this last one will open up alu16.ap in cgt after running the python script
so that you don't have to do it manually.
Comment 18 Luke Kenneth Casson Leighton 2020-03-17 07:17:51 GMT
(In reply to Jean-Paul.Chaput from comment #12)

>   I understand, I'm accustomed to it and have no problem with it.
>   This is because sometime the nesting could be very deep, and I
>   did not want things to go too much to the right.

i trained as a software engineer. very first time i dropped my code
in front of a lab assistant, he rattled out some changes on vi (which
got me hooked on it, instantly) and said, "if you have to go beyond
about 3 levels of indentation, write a new function".

>   I will switch to 4 spaces.

i only recommend this if you also redesign the code to use functions
in order to avoid exactly what you used 2 spaces for.

this unfortunately will be a lot of work so should be done incrementally.


> > and the use of commas at the *beginning* of the line on a parameter list,
> > rather than the usual place at the *end* of the line, is very difficult to
> > get used to.  i find that, automatically, i add the comma at the end
> > (because that is the pep8 standard) and of course it creates a syntax error
> > because there is a *second* (unexpected) comma at the start of the next line.
> 
>   Funny thing, I did put the comma at the beginning because I kept
>   forgetting to put it at the end. I find it also much more clear as it
>   makes one straight column below the opening parenthesis and is
>   more convenient for rectangular selection. And finally it helps
>   keep thing vertically aligned so you immediately notice if a
>   comma is missing (tabulating at the end is possible but tedious
>   because of the variable length or the arguments).
>     I understand it goes against a lot of habits and automatism,
>   but I think I will keep it.

just bear in mind, everyone who has done standard python software
programming (for 20 years) will find this very awkward to work with,
as well as read.

(we just got used to it, in other words, and it became second-nature).

for example: at some point, just to be able to read the code, i may have
to run autopep8 *before* searching for clues in the code, then on each
"git pull", destroy the working tree (git reset --hard), then "git pull",
then re-run autopep8.

! :)

> > it would need careful scheduling because, really, you need to hit the entire
> > codebase with a single carefully-reviewed autopep8 update and commit, and
> > not do *anything* else in between.
> 
>   Yes and perform complete regression tests.

autopep8 is specifically designed not to cause "damage".  there are a couple
of options which can change the meaning of the code (fix some things that you
should not be doing), however they are off by default and need to be explicitly
enabled.

as more of coriolis2 is converted to python, jean-paul, there should not just
be regression tests added, there should be unit tests added as well.

having worked with massive python code-bases before, i mention this here,
first and top priority thing of "Coding" section:
https://libre-riscv.org/HDL_workflow/

you *have* to have unit tests in a medium to large sized python project. 
without fail.

i know it is annoying, and feels like it is "stopping useful and productive work".  however it is really quite simple: if you don't have unit tests, you absolutely cannot safely change even one line of code that is already in use.

anyway.  OT for this bugreport.  we have EUR available, allocated specifically to coriolis2, to help ok?
Comment 19 Luke Kenneth Casson Leighton 2020-03-17 07:26:25 GMT
ok i tried this:

make clean
make lvx
cgt --text --script=doAlu16

and i got this:

  o  Routing did not complete, unrouted segments:
      1| &<id:15840 Horizontal abc_828_new_n73 METAL4 [585l 210l] [625l 210l] 2l rpD:3 -----CG------A----bt- [582.5l:627.5l] 45l 1----s->

along with a few "repair" errors

* add completed fine
* sub completed fine
* addsub16 did not

give me one second, jock, i will munge the code slightly: i found last week that letting the auto-placer put the top-level hierarchy cells anywhere in the large box wasn't ok.
Comment 20 Luke Kenneth Casson Leighton 2020-03-17 07:43:13 GMT
commit dd8ddc307197a406fa0f08f6ac80b16565004fdd
Author: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date:   Tue Mar 17 07:37:27 2020 +0000

    reposition add and sub, and do place in the *middle* of alu16

ok try that, jock - but before doing so, have a look at alu16.ap
(if you can)

this is actually a really good illustration of what we need to do.
i'm assuming you didn't get a hard routing completion error, here.

the non-completing of the routing is not jean-paul's problem, it's
ours.

the placement of the cells all over the large area (1100 x 700)
meant that when it comes to routing, it is a total mess.

what i've done is: limited the "placement" box for the remaining
(little) components - Cells - of alu16 to be in the MIDDLE.

*between* add and sub.

then, the auto-router can not only successfully connect those up,
it has room to connect to add and sub as well.

now, it turns out that if you move add and sub down by even 25 units,
*there is not enough room to add the tracks* connecting them to the
outside world.

but, we need to get you onto the same page, first.
Comment 21 Jock Tanner 2020-03-17 13:04:56 GMT
(In reply to Luke Kenneth Casson Leighton from comment #20)
> ok try that, jock - but before doing so, have a look at alu16.ap
> (if you can)

I dug up this:

http://manpages.ubuntu.com/manpages/bionic/man5/ap.5.html

and I looked at the file knowing approximately what kind of problem it solves. Still the file is huge and I don't think I can debug the code just by looking at it. I just wonder how many 'I' records it has. Shouldn't it be like 3 instances? Or is this because each bit slice counts?

> the placement of the cells all over the large area (1100 x 700)
> meant that when it comes to routing, it is a total mess.
> 
> what i've done is: limited the "placement" box for the remaining
> (little) components - Cells - of alu16 to be in the MIDDLE.
> 
> *between* add and sub.
> 
> then, the auto-router can not only successfully connect those up,
> it has room to connect to add and sub as well.
> 
> now, it turns out that if you move add and sub down by even 25 units,
> *there is not enough room to add the tracks* connecting them to the
> outside world.

I pulled your changes and did only

  make clean
  make lvx
  cgt --script=doAlu16

Still I got exactly the same error as before. And the picture is just a quarter of the 'make view' picture size.
Comment 22 Luke Kenneth Casson Leighton 2020-03-17 13:48:05 GMT
(In reply to Jock Tanner from comment #21)
> (In reply to Luke Kenneth Casson Leighton from comment #20)
> > ok try that, jock - but before doing so, have a look at alu16.ap
> > (if you can)
> 
> I dug up this:
> 
> http://manpages.ubuntu.com/manpages/bionic/man5/ap.5.html
> 
> and I looked at the file knowing approximately what kind of problem it
> solves. 

sorry, sorry, i meant "look at it with make cgt" followed by "File-Open"
and type "alu16".

or, yes, use "cgt --script=doAlu16"

the .ap files are basically purely positions of components etc. and the
tracks.  looking at them (directly) will drive you nuts :)


> I pulled your changes and did only
> 
>   make clean
>   make lvx
>   cgt --script=doAlu16
> 
> Still I got exactly the same error as before. 

arse.  oh wait - don't run "make lvx".  i just tried that, and yes i got
an error.


do this:

make clean
make vst               <---- makes the VHDL
cgt --script=doAlu16


the reason is: "make lvx" will *already create* the .ap files.  however - and this is quite smart - doAlu16.py is instructed to *pick up* (read) them if they exist, because we might want to do a bit of work (some place), then save them, and do some routing later on.

loadCell Catalog.State.Logical will load only the VHDL files (alu16.vhd
then add.vhd and sub.vhd because those are sub-components)

loadCell Catalog.State.Views will load *everything* that exists:
alu16.vhd, alu16.ap, *and* the sub-components of those as well
(add.vhd, add.ap, sub.vhd, sub.ap - you can see the hierarchy
in the log output)


> And the picture is just a
> quarter of the 'make view' picture size.

that's ok - just resize it then press "F"
Comment 23 Jock Tanner 2020-03-17 14:19:54 GMT
(In reply to Luke Kenneth Casson Leighton from comment #22)
> arse.  oh wait - don't run "make lvx".  i just tried that, and yes i got
> an error.
> 
> 
> do this:
> 
> make clean
> make vst               <---- makes the VHDL
> cgt --script=doAlu16

This way I get another error:

> [ERROR] An exception occured while loading the Python script module:
>         "doAlu16"
>         You should check for simple python errors in this module.
>         Python stack trace:
>         #0 in             placeAndRoute() at doAlu16.py:46
>         #1 in                       add() at doAlu16.py:193
>         #2 in                ScriptMain() at doAlu16.py:437
>         #3 in                 runScript() at .../coriolis-2.x/Linux.x86_64/Release.Shared/install/bin/cgt:83
>         Error was:
>           [BUG] Unmanaged Configuration [16843009] = [1+1+0+1,1+0] <id:1493 Net "a(12)" e-- LOGICAL i--- (IN)> in <id:2921 Anabatic::GCell <Box 250l 350l 300l 400l> ----------M---,--A- 9>
>       The global routing seems to be defective.
>         Trying to continue anyway...

And the picture in cgt looks really funny, more like child's drawing. And it's called 'add', not 'alu16.

Maybe that's all because our toolchain's versions are out of sync?
Comment 24 Jock Tanner 2020-03-17 14:22:05 GMT
BTW I've scaled the bit width of both modules down to 4. I thought it'd be easier to debug this way, but I got no errors this way. May I share the pictures?

And I also wonder where the 'op' pin went?
Comment 25 Luke Kenneth Casson Leighton 2020-03-17 14:52:34 GMT
(In reply to Jock Tanner from comment #23)
> (In reply to Luke Kenneth Casson Leighton from comment #22)
> > arse.  oh wait - don't run "make lvx".  i just tried that, and yes i got
> > an error.
> > 
> > 
> > do this:
> > 
> > make clean
> > make vst               <---- makes the VHDL
> > cgt --script=doAlu16
> 
> This way I get another error:
> 
> > [ERROR] An exception occured while loading the Python script module:
> >         "doAlu16"
> >         You should check for simple python errors in this module.
> >         Python stack trace:
> >         #0 in             placeAndRoute() at doAlu16.py:46
> >         #1 in                       add() at doAlu16.py:193
> >         #2 in                ScriptMain() at doAlu16.py:437
> >         #3 in                 runScript() at .../coriolis-2.x/Linux.x86_64/Release.Shared/install/bin/cgt:83
> >         Error was:
> >           [BUG] Unmanaged Configuration [16843009] = [1+1+0+1,1+0] <id:1493 Net "a(12)" e-- LOGICAL i--- (IN)> in <id:2921 Anabatic::GCell <Box 250l 350l 300l 400l> ----------M---,--A- 9>

ah.  that tells me that you may be on the wrong branch (or git repo)

> >       The global routing seems to be defective.
> >         Trying to continue anyway...
> 
> And the picture in cgt looks really funny, more like child's drawing. And
> it's called 'add', not 'alu16.

there's three functions that do routing, and they are all separate functions:

* add()   - which uses add.vst to create add.ap
* sub()   - which uses sub.vst to create sub.ap
* alu16() - which uses alu16.vst *and* add.ap/vst *and* sub.ap/vst to create
            alu16.ap


> Maybe that's all because our toolchain's versions are out of sync?

that seems likely.  did you check the git versions i sent a couple of messages back?

for alliance you should have this in .git/config:
[remote "origin"]
        url = https://gitlab.lip6.fr/vlsi-eda/alliance.git

where git branch should be "master"


and for coriolis2:

[remote "origin"]
        url = https://gitlab.lip6.fr/vlsi-eda/coriolis.git
...
...
[branch "devel"]
        remote = origin
        merge = refs/heads/devel

where git branch should be "devel".


and alliance-check-toolkit:

[remote "origin"]
        url = https://gitlab.lip6.fr/vlsi-eda/alliance-check-toolkit.git

where branch should be "master".


you should be on:

commit 622019240ff7d9c00927ee52556401091436ec2e
Merge: e5fa9a6 e385909
Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
Date:   Sun Mar 15 23:59:43 2020 +0100

    Merge branch 'master' of gitlab.lip6.fr:vlsi-eda/alliance-check-toolkit



commit 93cd988d07037b71c3b8497c4c376b1d7d151f71
Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
Date:   Wed Nov 13 17:47:22 2019 +0100

    Stable AP read/write generation.



commit b07a4722509bfcb2aacdb72d3122793e4bf377ea
Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
Date:   Sun Mar 15 23:58:31 2020 +0100

    Dynamic detection of Coriolis2 library directory.
Comment 26 Luke Kenneth Casson Leighton 2020-03-17 14:58:52 GMT
(In reply to Jean-Paul.Chaput from comment #12)

>   3. Direct command line.
>     I wanted especially the GUI to be able to recover after
>   an exception is raised, to do some forensic.
>     It is far from perfect but did help me much while debugging.

yes that makes sense.

i *think* there may be a better way to do that,
hmmm https://wiki.python.org/moin/PythonDebuggingTools

ah yes:
https://docs.python.org/3/library/pdb.html

python3 -m pdb {scriptname.py}
Comment 27 Jock Tanner 2020-03-17 15:12:45 GMT
(In reply to Luke Kenneth Casson Leighton from comment #25)
> for alliance you should have this in .git/config:
> [remote "origin"]
>         url = https://gitlab.lip6.fr/vlsi-eda/alliance.git
> 
> where git branch should be "master"

tanner@ibmpc:~/alliance/alliance$ git status
On branch master
Your branch is up to date with 'origin/master'.


> and for coriolis2:
> 
> [remote "origin"]
>         url = https://gitlab.lip6.fr/vlsi-eda/coriolis.git
> ...
> ...
> [branch "devel"]
>         remote = origin
>         merge = refs/heads/devel
> 
> where git branch should be "devel".

tanner@ibmpc:~/coriolis-2.x/src/coriolis$ git status 
Refresh index: 100% (8230/8230), done.
On branch devel
Your branch is up to date with 'origin/devel'.


> and alliance-check-toolkit:
> 
> [remote "origin"]
>         url = https://gitlab.lip6.fr/vlsi-eda/alliance-check-toolkit.git
> 
> where branch should be "master".

tanner@ibmpc:~/alliance-check-toolkit$ git status
On branch master
Your branch is up to date with 'origin/master'.


> you should be on:
> 
> commit 622019240ff7d9c00927ee52556401091436ec2e
> Merge: e5fa9a6 e385909
> Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
> Date:   Sun Mar 15 23:59:43 2020 +0100

I was 10 days behind, on e3859099a6f55b14e7b76388a8f885c4b78ea8fd, but now I did 'git pull' and don't see changes in relevant files.
Comment 28 Jock Tanner 2020-03-17 15:24:22 GMT
(In reply to Luke Kenneth Casson Leighton from comment #26)
> (In reply to Jean-Paul.Chaput from comment #12)
> 
> >   3. Direct command line.
> >     I wanted especially the GUI to be able to recover after
> >   an exception is raised, to do some forensic.
> >     It is far from perfect but did help me much while debugging.
> 
> yes that makes sense.
> 
> i *think* there may be a better way to do that,
> hmmm https://wiki.python.org/moin/PythonDebuggingTools
> 
> ah yes:
> https://docs.python.org/3/library/pdb.html
> 
> python3 -m pdb {scriptname.py}

Oh, being unable to use Pycharm's built-in debugger, I had to resort to ipdb. It's like pdb but nicer. It's possible to just throw

'import ipdb; ipdb.set_trace()'

anywhere in the source file, and it opens its console there.

But exception wrappers are perfectly valid. Sometimes even indispensable. In Celery tasks, for example. It's just may be more convenient to use 'traceback.print_exception()' inside them instead of 'print()'.

https://docs.python.org/2.7/library/traceback.html
Comment 29 Jock Tanner 2020-03-17 16:06:13 GMT
(In reply to Jock Tanner from comment #27)
> (In reply to Luke Kenneth Casson Leighton from comment #25)
> > and alliance-check-toolkit:
> > 
> > [remote "origin"]
> >         url = https://gitlab.lip6.fr/vlsi-eda/alliance-check-toolkit.git
> > 
> > where branch should be "master".
> 
> tanner@ibmpc:~/alliance-check-toolkit$ git status
> On branch master
> Your branch is up to date with 'origin/master'.
> 
> 
> > you should be on:
> > 
> > commit 622019240ff7d9c00927ee52556401091436ec2e
> > Merge: e5fa9a6 e385909
> > Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
> > Date:   Sun Mar 15 23:59:43 2020 +0100
> 
> I was 10 days behind, on e3859099a6f55b14e7b76388a8f885c4b78ea8fd, but now I
> did 'git pull' and don't see changes in relevant files.

Well, Luke, I am on the same commit as you now, and I get this:

> make vst
> 
> ...
> 
>         + sub [.model]
>            + /home/tanner/alliance/install/cells/sxlib/on12_x1.vbe [behavioral]
>            + /home/tanner/alliance/install/cells/sxlib/on12_x1.ap
>            + /home/tanner/alliance/install/cells/sxlib/no4_x1.vbe [behavioral]
>            + /home/tanner/alliance/install/cells/sxlib/no4_x1.ap
>            + /home/tanner/alliance/install/cells/sxlib/ao2o22_x2.vbe [behavioral]
>            + /home/tanner/alliance/install/cells/sxlib/ao2o22_x2.ap
> Traceback (most recent call last):
>   File "/home/tanner/alliance-check-toolkit/bin/blif2vst.py", line 71, in <module>
>     renameNMigenUniquify( cell )
>   File "/home/tanner/alliance-check-toolkit/bin/blif2vst.py", line 32, in renameNMigenUniquify
>     for occurrence in topCell.getTerminalNetlistInstanceOccurrences():
> AttributeError: 'Hurricane.Cell' object has no attribute 'getTerminalNetlistInstanceOccurrences'
>  make: [mk/synthesis-yosys.mk:73: alu16.vst] Error 1 (ignored)

I checked 'dir(Cell)' and it really has no 'getTerminalNetlistInstanceOccurrences' attribute. My Coriolis is up to date. Is Alliance toolkit now ahead of Coriolis?
Comment 30 Luke Kenneth Casson Leighton 2020-03-17 16:41:00 GMT
(In reply to Jock Tanner from comment #29)

> I checked 'dir(Cell)' and it really has no
> 'getTerminalNetlistInstanceOccurrences' attribute. My Coriolis is up to
> date. Is Alliance toolkit now ahead of Coriolis?

remember, coriolis2 is a c++ application (not a pure python application), with "wrapper" python bindings around the c++ code.  after the git pull you need to rebuild the c++ code.
Comment 31 Jock Tanner 2020-03-17 17:06:33 GMT
(In reply to Luke Kenneth Casson Leighton from comment #30)
> (In reply to Jock Tanner from comment #29)
> 
> > I checked 'dir(Cell)' and it really has no
> > 'getTerminalNetlistInstanceOccurrences' attribute. My Coriolis is up to
> > date. Is Alliance toolkit now ahead of Coriolis?
> 
> remember, coriolis2 is a c++ application (not a pure python application),
> with "wrapper" python bindings around the c++ code.  after the git pull you
> need to rebuild the c++ code.

My Coriolis is already up to date, on 'devel' branch. Its code didn't change since the last time I built it. Just reran 'bootstrap/ccb.py' to make sure. And its current C++ code does not contain the identifiers 'getTerminalNetlistInstanceOccurrences' or 'getNonTerminalNetlistInstanceOccurrences'. The repository is not recursive, the names just can't come out of thin air.

The only thing I can come up with is that alliance toolkit is ahead of Coriolis. Or what else can it be?
Comment 32 Luke Kenneth Casson Leighton 2020-03-17 17:24:28 GMT
(In reply to Jock Tanner from comment #31)
> (In reply to Luke Kenneth Casson Leighton from comment #30)
> > (In reply to Jock Tanner from comment #29)
> > 
> > > I checked 'dir(Cell)' and it really has no
> > > 'getTerminalNetlistInstanceOccurrences' attribute. My Coriolis is up to
> > > date. Is Alliance toolkit now ahead of Coriolis?
> > 
> > remember, coriolis2 is a c++ application (not a pure python application),
> > with "wrapper" python bindings around the c++ code.  after the git pull you
> > need to rebuild the c++ code.
> 
> My Coriolis is already up to date, on 'devel' branch. Its code didn't change
> since the last time I built it. Just reran 'bootstrap/ccb.py' to make sure.
> And its current C++ code does not contain the identifiers
> 'getTerminalNetlistInstanceOccurrences' or
> 'getNonTerminalNetlistInstanceOccurrences'. 

do "git log" | head just to make absolutely sure.


> The repository is not recursive,
> the names just can't come out of thin air.

correct.  they're in this git diff:
git diff dfe4d80b e2d69

> The only thing I can come up with is that alliance toolkit is ahead of
> Coriolis. 

correct.

which means... you've not git pulled correctly and/or are on the master branch not the devel branch

https://gitlab.lip6.fr/vlsi-eda/coriolis

here is the commit:
https://gitlab.lip6.fr/vlsi-eda/coriolis/commit/dfe4d80b600601dca1255de6d7066ecabe7985ec



ah.

i know what it is.

(coriolis2)lkcl@fizzy:~/coriolis-2.x/Linux.x86_64/Release.Shared/install$ ls -altr
total 0
drwxr-xr-x 1 lkcl lkcl   18 Mar  4 04:56 etc
drwxr-xr-x 1 lkcl lkcl   24 Mar  4 04:56 ..
drwxr-xr-x 1 lkcl lkcl   42 Mar  4 04:56 .
drwxr-xr-x 1 lkcl lkcl   26 Mar  4 04:56 share
drwxr-xr-x 1 lkcl lkcl   56 Mar  4 04:56 include
drwxr-xr-x 1 lkcl lkcl 3276 Mar 16 23:10 lib
drwxr-xr-x 1 lkcl lkcl  240 Mar 16 23:10 bin
(coriolis2)lkcl@fizzy:~/coriolis-2.x/Linux.x86_64/Release.Shared/install$ 

what do you have in this directory?

you wouldn't *happen* to have two subdirectories - one named "lib" and one named "lib64", would you?

suggestion: blow the entire ~/coriolis-2.x/Linux.x86_64 directory away and re-run ccb bootstrap.
Comment 33 Jean-Paul Chaput 2020-03-17 17:28:11 GMT
(In reply to Jock Tanner from comment #31)
> (In reply to Luke Kenneth Casson Leighton from comment #30)
> > (In reply to Jock Tanner from comment #29)
> > 
> > > I checked 'dir(Cell)' and it really has no
> > > 'getTerminalNetlistInstanceOccurrences' attribute. My Coriolis is up to
> > > date. Is Alliance toolkit now ahead of Coriolis?
> > 
> > remember, coriolis2 is a c++ application (not a pure python application),
> > with "wrapper" python bindings around the c++ code.  after the git pull you
> > need to rebuild the c++ code.
> 
> My Coriolis is already up to date, on 'devel' branch. Its code didn't change
> since the last time I built it. Just reran 'bootstrap/ccb.py' to make sure.
> And its current C++ code does not contain the identifiers
> 'getTerminalNetlistInstanceOccurrences' or
> 'getNonTerminalNetlistInstanceOccurrences'. The repository is not recursive,
> the names just can't come out of thin air.
> 
> The only thing I can come up with is that alliance toolkit is ahead of
> Coriolis. Or what else can it be?

  Are you on Coriolis commit b07a4722 ? (the latest one).
  The getTerminalNetlistInstanceOccurrences methods were commited with
  dfe4d80b four commits ago (and one week). Are you sure you did
  rebuild & rinstall Coriolis ? You may check the contents of the
  library with:
    > nm libhurricane.so | grep TerminalNetlist
  You should have lots of references if it is the right one.
Comment 34 Jock Tanner 2020-03-17 17:42:41 GMT
(In reply to Jean-Paul.Chaput from comment #33)
>   Are you on Coriolis commit b07a4722 ? (the latest one).

I'm on 'devel' branch and up to date, but my latest commits are from 9/02/2020, so maybe I'm just on different repo?

My git config says:

> [remote "origin"]
>     url = https://www-soc.lip6.fr/git/coriolis.git
Comment 35 Luke Kenneth Casson Leighton 2020-03-17 17:53:31 GMT
(In reply to Jock Tanner from comment #34)
> (In reply to Jean-Paul.Chaput from comment #33)
> >   Are you on Coriolis commit b07a4722 ? (the latest one).
> 
> I'm on 'devel' branch and up to date, but my latest commits are from
> 9/02/2020, so maybe I'm just on different repo?
> 
> My git config says:
> 
> > [remote "origin"]
> >     url = https://www-soc.lip6.fr/git/coriolis.git

read comment #25 again, jock - specifically noting the change of URLs.

l.
Comment 36 Jock Tanner 2020-03-17 18:07:31 GMT
(In reply to Luke Kenneth Casson Leighton from comment #35)
> read comment #25 again, jock - specifically noting the change of URLs.

Yeah you're right, guys, I'm sorry.
Comment 37 Jock Tanner 2020-03-17 18:37:07 GMT
Created attachment 44 [details]
I think this picture is correct

I'll get back to 'ChipConf' tomorrow.
Comment 38 Luke Kenneth Casson Leighton 2020-03-17 18:55:38 GMT
(In reply to Jock Tanner from comment #37)
> Created attachment 44 [details]
> I think this picture is correct
> 
> I'll get back to 'ChipConf' tomorrow.

oleeeee!  it's so preeetttyyy :)  we got to the bottom of it.  hurrah.

if you press Ctrl-I then you can enable Filter-tab "Process Tools",
and see the actual cells.

personally i find it absolutely bleedin fascinating.
zoom in close and then use Ctrl-Right-Mouse and it will show you
the name of any instances at that mouse-point (VIAs, METAL traces,
and the Cells themselves.

you can then, if you are feeling particularly fascinated / horrified
by the level of detail, search (grep) for that name back in the
*vst* files:

   grep subctk_33_sff1 *.vst

for example.

ok so let's start again, tomorrow, from here:
http://bugs.libre-riscv.org/show_bug.cgi?id=217#c20

btw i have to travel down to Essex tomorrow, i'm catching a coach at
6pm, overnight.  i'll have internet access (through a WIFI-4G router)
just no laptop set up. most likely i'll be "properly" back online by
thursday evening.  so if you get messages from me which are both
obtuse and badly spelled, you know i'm pecking one-finger at a hand-held
device.
Comment 39 Jock Tanner 2020-03-18 14:31:43 GMT
I'm glad I managed to fix my Pycharm in terms of introspecting the Coriolis Python extensions. It was a stale skeleton cache. I deleted it manually (the usual “Invalidate and restart” did not do the trick), and everything suddenly started to work, including code completion.

I made a wrapper function around 'Pin.create()', that tries to use some defaults for simpler use. I may got some of the defaults wrong though. Maybe more appropriate values can be deducted from the Hurricane's guts. Please have a look.

https://git.libre-riscv.org/?p=soclayout.git;a=blob;f=experiments7/doAlu16.py;hb=HEAD#l40
Comment 40 Luke Kenneth Casson Leighton 2020-03-18 15:15:08 GMT
(In reply to Jock Tanner from comment #39)
> I'm glad I managed to fix my Pycharm in terms of introspecting the Coriolis
> Python extensions. It was a stale skeleton cache. I deleted it manually (the
> usual “Invalidate and restart” did not do the trick), and everything
> suddenly started to work, including code completion.

exccellent.

> I made a wrapper function around 'Pin.create()', that tries to use some
> defaults for simpler use. I may got some of the defaults wrong though. Maybe
> more appropriate values can be deducted from the Hurricane's guts. Please
> have a look.

looks great to me.  the docstring quality is excellent.

i did my usual "make myself go over the code by making
whitespace cleanups" i find it's a good way to "review" by making my fingers
"do" if you know what i mean.  you'll need to do a git pull, there.

FIXED is a good default.  2.0 for the track-width, we might want to have
a class that sets that, at some point, rather than a function.

so the next two things, i think:

1. can you make a function which creates the BLOCKAGE "things"?
   then call it from add() and sub().  defaults to 2,3,4 however allow
   it to be parameterisable.

2. add() and sub() are near-duplicates and to some extent so is alu16.
   could you make a function which both add() and sub() call, and break
   out pieces for alu16()?  for example, the "find and place" add and sub

we want the actual "parameterisation" to be as basic and as bleedingly
obvious as possible by keeping it compact, with everything "coriolis-related"
behind functions which are well-commented as to what they do.

so for add() and sub() they should be calling functions that, effectively,
take nothing more than:

- the cell name
- the size
- the pins.

err... that's it!


the task after that is where the "real" stuff starts if you know what i mean.

you see how there's a massive amount of whitespace at the top of alu16.ap
and at the bottom? with tracks that, because of the VIAs, you can't fit
any multi-layer routing, so (apart from the very top 50 which is really
puzzling, must raise a bug with Jean-Paul about it) there's very little
opportunity to "optimise it".

however... this is entirely a problem of our own making, becaise both add()
and sub() have inputs at the *top* and outputs at the *bottom*.

what is needed instead is:

* inputs on add() and sub() to be *ALL* at the top (NORTH)
  - A+B to still be interleaved (just spaced closer together)
    and on the LEFT of NORTH
  - O to be in the RIGHT side of NORTH

* when "placed" inside alu16, the "add" cell needs to be *ROTATED*
  (not Orientation.ID), clockwise and
  "sub" needs to be rotated *anti*-clockwise

* alu16 A and B input and O output - just for fits and giggles - to be
  brought closer together (not as spaced-out across the whole of the cell)
  whilst still left on their respective sides.

* place-and-route attempted in the same way.

you _will_ find that the route of add() / sub() will go "quirky" i.e. if
certain combinations of the pins are given, the routing will not work
("error, pin is too far away from cell" that sort of thing)

you just have to experiment until you find a working combination.

this kind of thing is basically the "core" of what needs doing: looking at
how the router reacts, then going "hmmm" when it fails.


as we progress through to larger blocks, we will need to plan things
a bit carefully.

for example: there will be about *THIRTY* of these ALUs - some of them
extremely large - coming off of a *SINGLE* bus (!!).  i.e. the data
*in* comes *back* to the same location as the data *out*, back onto
the same bus.

therefore, when we get to that point, the "thinner" each ALU is, the
better.  in other words, we need a "chain" of pipeline stages, where
half of them go one direction, then the data "turns" round, and the
stages for the last half come *back* the other way.  that way we have
the inputs arriving back at the same place as the outputs, and ta-daa,
we can route them onto the same bus.

basically this here - alu16 - is a trial run of that concept, which will
be massively expanded by about 100 times larger than add() and sub() are,
right now.

(!!) :)


oh.

yes.

nearly forgot.

third task: we need to compute (auto-calculate) the size of alu16 based
on the relative size of add(), sub(), and the remaining components.
that one we might have to ask Jean-Paul about.

erm, why are we discussing this under this bugreport?  must create
a new one, really (doh).
Comment 41 Jock Tanner 2020-03-18 22:09:21 GMT
(In reply to Luke Kenneth Casson Leighton from comment #40)
> (In reply to Jock Tanner from comment #39)
> looks great to me.  the docstring quality is excellent.

When I enter 3 quotes under a 'def', Pycharm creates a docstring template for me. I only have to fill in the human-readable descriptions.

> 
> i did my usual "make myself go over the code by making
> whitespace cleanups" i find it's a good way to "review" by making my fingers
> "do" if you know what i mean.  you'll need to do a git pull, there.

I see. Your style is valid, but it eats up more space than mine (newline after and before parentheses aka 'hanging indent'). I think hanging indent

- is better than smaller indent when it comes to cramping things up,

- has a consistent look in regard of other multiline expressions (split strings, lists, list/dict comprehensions, et c.)

> so the next two things, i think:
> 
> 1. can you make a function which creates the BLOCKAGE "things"?
>    then call it from add() and sub().  defaults to 2,3,4 however allow
>    it to be parameterisable.

What about the existing 'get_layer()'?

> 2. add() and sub() are near-duplicates and to some extent so is alu16.
>    could you make a function which both add() and sub() call, and break
>    out pieces for alu16()?  for example, the "find and place" add and sub

I think it's doable. The code indeed is very repetitive.

I have two questions about our current setup.

1. Is it OK that we have separate adder and subtractor? I think most ALUs just negate and add instead of subtract. I think this would use a bit less logic, and negation in itself is also a useful operation.

2. Did we lost 'op' pin?
Comment 42 Luke Kenneth Casson Leighton 2020-03-18 23:39:30 GMT
(In reply to Jock Tanner from comment #41)
> (In reply to Luke Kenneth Casson Leighton from comment #40)
> > (In reply to Jock Tanner from comment #39)
> > looks great to me.  the docstring quality is excellent.
> 
> When I enter 3 quotes under a 'def', Pycharm creates a docstring template
> for me. I only have to fill in the human-readable descriptions.

niiice.

> > 
> > i did my usual "make myself go over the code by making
> > whitespace cleanups" i find it's a good way to "review" by making my fingers
> > "do" if you know what i mean.  you'll need to do a git pull, there.
> 
> I see. Your style is valid, but it eats up more space than mine (newline
> after and before parentheses aka 'hanging indent').

i tend to make a new function or use an intermediary variable if that happens.

am not a fan of massive.selfexplanatory.variablenames.and.functionnames.

much better to have a short name and a good comment.

> I think hanging indent
> 
> - is better than smaller indent when it comes to cramping things up,
> 
> - has a consistent look in regard of other multiline expressions (split
> strings, lists, list/dict comprehensions, et c.)

if you have good reason to use them i.am all in. bear in mind i keep sometimes EIGHT 80x63 xterms open at once onscreen side by side (now 12 with a 4k screen) so small functions and the 80 char limit is really important.
 
> > so the next two things, i think:
> > 
> > 1. can you make a function which creates the BLOCKAGE "things"?
> >    then call it from add() and sub().  defaults to 2,3,4 however allow
> >    it to be parameterisable.
> 
> What about the existing 'get_layer()'?

nuts. cant see it.  on phone. can you send yrl to gitweb thing?
 
> > 2. add() and sub() are near-duplicates and to some extent so is alu16.
> >    could you make a function which both add() and sub() call, and break
> >    out pieces for alu16()?  for example, the "find and place" add and sub
> 
> I think it's doable. The code indeed is very repetitive.

yes. i cutpaste it in a hurry
 
> I have two questions about our current setup.
> 
> 1. Is it OK that we have separate adder and subtractor?

this is just an experiment, from nmigen alu_hier.py example.

> I think most ALUs
> just negate and add instead of subtract. I think this would use a bit less
> logic, and negation in itself is also a useful operation.

yes. in the "real" alu we have... mmm... not sure, actually.  probably -

do me a favour and raise a bugreport as a reminder to look up what rocketchip does?

 
> 2. Did we lost 'op' pin?

yes, well spotted :)  that should come in at top.

clock on the side just for fits and giggles.

check ringoscillator benchs or grep -r benchs createpin to find examples.
Comment 44 Jock Tanner 2020-03-26 21:39:34 GMT
Sorry for being missing for some time, it was a busy week for me. My other projects and local bureaucracy required my attention.

So I reworked the ALU16 example with the hope that this approach could be (re)used in a future work.

https://git.libre-riscv.org/?p=soclayout.git;a=blob;f=experiments7/utils.py

My goals were:

- eliminate dead, duplicated and redundant code,

- facilitate multiple instantiation of electronic components through modular design with a support for module “nesting”,

- enable parameterization of the electronic modules in a simple, declarative manner,

- proper incapsulate all Coriolis interfacing to overcome the “global context” antipattern that prevailed in the original code, and further improve the readability and maintainability.

To achieve these goals, I

- created a class named 'Module', which encapsulated the execution context ('Hurricane.Cell', 'Hurricane.DataBase', 'CRL.AllianceFramework', 'editor', et c.) in its attributes and properties,

- converted the typical actions into the methods of this class,

- separated the initialization of the Module object ('Module.__init__()') with the actual operations ('Module.build()'),

- provided a method for recursive building of “nested” objects.

Right now, 'Module.build()' is designed to be implemented in 'Module' subclasses. As soon as my understanding of the requirements will grow beyond the simple examples, 'Module.build()' can be converted into the “template method”.

The amount of required parameters can also be reduced. Maybe we could teach the module to deduct either its size or pin spacing. Now the initialization looks quite fuzzy.

I deleted all the 'print' statements from the example, since I could not grasp the logic and requirements behind them. Perhaps it makes sense to return some logging, preferably with a proper use of 'logging' facility.
Comment 45 Luke Kenneth Casson Leighton 2020-03-26 22:58:59 GMT
(In reply to Jock Tanner from comment #44)
> Sorry for being missing for some time, it was a busy week for me. My other
> projects and local bureaucracy required my attention.
> 
> So I reworked the ALU16 example with the hope that this approach could be
> (re)used in a future work.
> 
> https://git.libre-riscv.org/?p=soclayout.git;a=blob;f=experiments7/utils.py

niiiice.  that's really clean.  excellent - and precisely what i was hoping
to see.


oh.  one really important thing: we need an "auto" option for the width
and height... and for the *hierarchy to be able to work with that*.

so for example:

Add and Sub use the standard "auto-box" height/width (i forget the function,
sorry)

ALU16 computes its height based on:
max(Add.H, Sub.H, middle-stuff-H) + 100 (to give room for the routing)

and its width based on:
Add.W + Sub.W + middle-stuff-W + 20-or-so

this is actually really important because if we set fixed W+H and the HDL
changes, we're screwed when it comes to re-running the layout.

also, there *may* be small changes when we go to different geometries,
so it's really important to stay flexible and not have hard-coded sizes.

> The amount of required parameters can also be reduced. Maybe we could teach
> the module to deduct either its size or pin spacing. Now the initialization
> looks quite fuzzy.

yes, there's a function which does auto-size assessment, however it does
width *and* height, it doesn't do "width given height" or "height given
width".

also, we need "estimated size of the *remaining Cells which have not been
laid out*.

or "estimated size of just the following group of Cells".

we might have to do that by splitting the HDL into further modules
(this is a fallback plan).

so for example, everything in ALU16 that connects Add() and Sub() -
*all* that HDL goes into a *separate* module, such that ALU16()
*only* has 3 sub-modules:

* Add
* Sub
* ConnectAddAndSub

and thus *only* needs "routing".

however this would be a bit of a pain and i'd like to avoid redesigning
the HDL to this kind of extreme level, if possible.


> I deleted all the 'print' statements from the example, since I could not
> grasp the logic and requirements behind them. Perhaps it makes sense to
> return some logging, preferably with a proper use of 'logging' facility.

good idea.  if you can put a template / example together i'll tend to use it :)

so could you track down that auto-box-size computing function and then
look at removing all hard-coded parameters?

you'll likely need to run add.build() manually, then sub.build() manually,
*then* in ALU16()'s build constructor it will have access to the
(computed) width+height of each, *then* it can compute its width and
height.
Comment 46 Jock Tanner 2020-03-27 00:03:17 GMT
(In reply to Luke Kenneth Casson Leighton from comment #45)
> so could you track down that auto-box-size computing function and then
> look at removing all hard-coded parameters?

I think it is 'void EtesianEngine::setDefaultAb()'. But I can be wrong, since it is used only once in Python code shipped with Coriolis, and not documented anywhere.
Comment 47 Luke Kenneth Casson Leighton 2020-03-27 00:38:29 GMT
(In reply to Jock Tanner from comment #46)
> (In reply to Luke Kenneth Casson Leighton from comment #45)
> > so could you track down that auto-box-size computing function and then
> > look at removing all hard-coded parameters?
> 
> I think it is 'void EtesianEngine::setDefaultAb()'. But I can be wrong,
> since it is used only once in Python code shipped with Coriolis, and not
> documented anywhere.

have a look in other experiments, i used the function i am thinking of at least once.

computeAbutmentBox or something
Comment 48 Jock Tanner 2020-03-27 01:27:02 GMT
(In reply to Luke Kenneth Casson Leighton from comment #47)
> (In reply to Jock Tanner from comment #46)
> > I think it is 'void EtesianEngine::setDefaultAb()'. But I can be wrong,
> > since it is used only once in Python code shipped with Coriolis, and not
> > documented anywhere.
> 
> have a look in other experiments, i used the function i am thinking of at
> least once.
> 
> computeAbutmentBox or something

Then we're talking about the same thing.

This is the contents of 'cumulus.plugins.clocktree.ClockTree.computeAbutmentBox' (minus the commented out code):

  def computeAbutmentBox ( cell, spaceMargin, aspectRatio, cellGauge ):
      UpdateSession.open()
      etesian = Etesian.EtesianEngine.create( cell )
      etesian.setDefaultAb()
      etesian.destroy()
      UpdateSession.close()
      return cell.getAbutmentBox()

It's just a call to 'etesian.setDefaultAb()'. All parameters but 'cell' are ignored.

BTW 'etesian.place()' calls 'etesian.setDefaultAb()' under the hood if cell's AB is empty, so even this code may be superfluous. But it needs further experimenting.
Comment 49 Luke Kenneth Casson Leighton 2020-03-27 02:17:11 GMT
(In reply to Jock Tanner from comment #48)

>   def computeAbutmentBox ( cell, spaceMargin, aspectRatio, cellGauge ):

yep use that, it is the one JP advised.
Comment 50 Jock Tanner 2020-04-06 15:18:36 BST
Sorry for another big delay on my part.

(In reply to Luke Kenneth Casson Leighton from comment #45)
> so could you track down that auto-box-size computing function and then
> look at removing all hard-coded parameters?

'Module.compute_ab()' added.

> you'll likely need to run add.build() manually, then sub.build() manually,

I kinda anticipated such changes in workflow, so I divided the submodules processing into 'build()' and 'place()' phases beforehand. Now I added an utilities such as 'Module.submodules' iterator and 'Module.find_submodule(name)'.

> *then* in ALU16()'s build constructor it will have access to the
> (computed) width+height of each,

Also not a problem: after 'build_submodules()' it can be accessed via submodule's 'ab' property.

> *then* it can compute its width and
> height.

Here I stumbled on how to get the end result.

> ALU16 computes its height based on:
> max(Add.H, Sub.H, middle-stuff-H) + 100 (to give room for the routing)
> 
> and its width based on:
> Add.W + Sub.W + middle-stuff-W + 20-or-so

I tried this approach, but middle stuff size calculates as 650×650 and everything bloats. And there are many “unplaced design” errors. =(
Comment 51 Jock Tanner 2020-04-06 15:25:41 BST
Created attachment 49 [details]
That's how I see it might be done
Comment 52 Luke Kenneth Casson Leighton 2020-04-06 17:53:42 BST
(In reply to Jock Tanner from comment #50)
> Sorry for another big delay on my part.

no problem.  we still need to get the MoU signed.  money's allocated,
just not accessible yet.

> 
> (In reply to Luke Kenneth Casson Leighton from comment #45)
> > so could you track down that auto-box-size computing function and then
> > look at removing all hard-coded parameters?
> 
> 'Module.compute_ab()' added.

great.

> > you'll likely need to run add.build() manually, then sub.build() manually,
> 
> I kinda anticipated such changes in workflow, so I divided the submodules
> processing into 'build()' and 'place()' phases beforehand. Now I added an
> utilities such as 'Module.submodules' iterator and
> 'Module.find_submodule(name)'.

excellent
 
> > *then* in ALU16()'s build constructor it will have access to the
> > (computed) width+height of each,
> 
> Also not a problem: after 'build_submodules()' it can be accessed via
> submodule's 'ab' property.
> 
> > *then* it can compute its width and
> > height.
> 
> Here I stumbled on how to get the end result.
> 
> > ALU16 computes its height based on:
> > max(Add.H, Sub.H, middle-stuff-H) + 100 (to give room for the routing)
> > 
> > and its width based on:
> > Add.W + Sub.W + middle-stuff-W + 20-or-so
> 
> I tried this approach, but middle stuff size calculates as 650×650 and
> everything bloats. And there are many “unplaced design” errors. =(

yes.

ok, you'll see i've done a lot of experimentation, i went back to using
an iterative try-it-and-see approach

hum

also i did this:

* changed the I/O pins for ALU16 so that they are closer to the middle

* put all the pins for add and sub at the top (this needed experimentation
  just to get an I/O config for each that would "pass" routing)

* made add's pins a *mirror* of sub's pins (you'll see why, next)

* *ROTATED* add and sub so that their inputs/outputs "faced" each other

* put in a hard-coded positioning for the "middle" placement (ALU16
  little stuff)

* called place *only* (for all the little stuff)

* after the place, *only* call route *not* place-and-route

what happens if you call place-and-route after place, the entire box is
filled with "dummy cells" that go *underneath* (overlap) the add() and
sub() boxes.

whoops

this only showed up because of the rotation of add() and sub().

the reason for doing the rotate (and mirror) is to get the add() and sub()
I/O to try to "connect" with dead-straight lines, right across the middle-stuff.

however this (if you think about it carefully) creates an artificial barrier 
where no VIAs can be properly placed.

whoops.

ah.

just found what might be a serious bug: two routes trying to occupy the
same space.

jean-paul, i will raise a bugreport on gitlap.lip6.fr
Comment 53 Luke Kenneth Casson Leighton 2020-04-06 17:58:30 BST
https://gitlab.lip6.fr/vlsi-eda/coriolis/issues/8
Comment 54 Luke Kenneth Casson Leighton 2020-04-06 18:15:18 BST
(In reply to Luke Kenneth Casson Leighton from comment #53)
> https://gitlab.lip6.fr/vlsi-eda/coriolis/issues/8

jean-paul i think we need your help on this one.  are we pushing
the auto-router too hard, not giving it enough space?  i do see
some tracks going in weird directions (doubling back)

https://gitlab.lip6.fr/vlsi-eda/coriolis/issues/7
Comment 55 Jean-Paul Chaput 2020-04-07 09:12:32 BST
(In reply to Luke Kenneth Casson Leighton from comment #54)
> (In reply to Luke Kenneth Casson Leighton from comment #53)
> > https://gitlab.lip6.fr/vlsi-eda/coriolis/issues/8
> 
> jean-paul i think we need your help on this one.  are we pushing
> the auto-router too hard, not giving it enough space?  i do see
> some tracks going in weird directions (doubling back)
> 
> https://gitlab.lip6.fr/vlsi-eda/coriolis/issues/7

  Hello Luke & Jock,

  I'm going to look into it this afternoon or tomorrow. I've been
  busy with integrating the analog resistor devices then reviewing
  a little the Python code. I'm also working on improving the
  Python/C++ interface, going from C preprocessor macros to
  C++ templates (have to [re]learn things here), and preparing
  the move toward Python 3.

  For the MoU, only the Sorbonne University can sign it on my
  behalf, so I did forward it to my contact lawyer. Hoping it
  would not take too long to process in thoses troubled times.

  Best regards,
Comment 56 Luke Kenneth Casson Leighton 2020-04-07 10:46:15 BST
(In reply to Jean-Paul.Chaput from comment #55)
> (In reply to Luke Kenneth Casson Leighton from comment #54)
> > (In reply to Luke Kenneth Casson Leighton from comment #53)
> > > https://gitlab.lip6.fr/vlsi-eda/coriolis/issues/8
> > 
> > jean-paul i think we need your help on this one.  are we pushing
> > the auto-router too hard, not giving it enough space?  i do see
> > some tracks going in weird directions (doubling back)
> > 
> > https://gitlab.lip6.fr/vlsi-eda/coriolis/issues/7
> 
>   Hello Luke & Jock,
> 
>   I'm going to look into it this afternoon or tomorrow.

appreciated.

> I've been
>   busy with integrating the analog resistor devices then reviewing
>   a little the Python code. I'm also working on improving the
>   Python/C++ interface, going from C preprocessor macros to
>   C++ templates (have to [re]learn things here), and preparing
>   the move toward Python 3.

ok.  well, given that the code is actually working, the less disruption
that we have (say, for example... by *not* moving to python 3 so that
we don't have to spend another 3-7 days recompiling and working out
issues and redesigning the scripts and installation instructions) the better.

 
>   For the MoU, only the Sorbonne University can sign it on my
>   behalf, so I did forward it to my contact lawyer. 

if there is a way to emphasise to him that it is NOT a contract,
and that both LIP6 and other individuals with families are reliant
on him getting an answer - fast - for receiving money - that would
be... good.
Comment 57 Jean-Paul Chaput 2020-04-20 15:16:13 BST
Hello Jock & Luke,

I just commited in Coriolis & soclayout my latest corrections and
enhancements. You will find under experiment7/Status.rst
detailed comments about what I have done and the remaining
questions.

Good reading and best regards,
Comment 58 Luke Kenneth Casson Leighton 2020-04-20 16:55:55 BST
(In reply to Jean-Paul.Chaput from comment #57)
> Hello Jock & Luke,
> 
> I just commited in Coriolis & soclayout my latest corrections and
> enhancements. You will find under experiment7/Status.rst
> detailed comments about what I have done and the remaining
> questions.

appreciated.

hmm is everything committed? i just git pulled up to here:

commit 8978074fe72acb90b94a6c3b20ba820a1b753b69
Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
Date:   Mon Apr 20 12:53:07 2020 +0200

and for alliance-check-toolkit:
commit b9e5d37953b423b2a17c0599289d0682447c5f35
Author: Jean-Paul Chaput <Jean-Paul.Chaput@lip6.fr>
Date:   Fri Apr 10 12:26:15 2020 +0200

the reason i ask is because after rebuilding coriolis2 i get this:

Traceback (most recent call last):
  File "doAlu16Flat.py", line 137, in <module>
    success = scriptMain(**kwargs)
  File "doAlu16Flat.py", line 132, in scriptMain
    return alu16.build()
  File "doAlu16Flat.py", line 89, in build
    result = self.route()
  File "/home/lkcl/soclayout/experiments7/utils.py", line 191, in route
    katana.loadGlobalRouting(Anabatic.EngineLoadGrByNet)
hurricane.HurricaneError: [BUG] Unmanaged Configuration [16843266] = [2+2+0+1,1+0] <id:1492 Net "a(5)" e-- LOGICAL i--- (IN)> in <id:5325 Anabatic::GCell <Box 200l 0l 250l 50l> ----------M---,i-A- 15>
      The global routing seems to be defective.
Comment 59 Jean-Paul Chaput 2020-04-20 18:15:33 BST
(In reply to Luke Kenneth Casson Leighton from comment #58)
> (In reply to Jean-Paul.Chaput from comment #57)
>   File "doAlu16Flat.py", line 89, in build
>     result = self.route()
>   File "/home/lkcl/soclayout/experiments7/utils.py", line 191, in route
>     katana.loadGlobalRouting(Anabatic.EngineLoadGrByNet)
> hurricane.HurricaneError: [BUG] Unmanaged Configuration [16843266] =
> [2+2+0+1,1+0] <id:1492 Net "a(5)" e-- LOGICAL i--- (IN)> in <id:5325
> Anabatic::GCell <Box 200l 0l 250l 50l> ----------M---,i-A- 15>
>       The global routing seems to be defective.

Argh! Yes everything was commited. But, as we are not 100% clone of
each other (what Intel would call "copy exact" for its foundries),
you may have got a slighty different Yosys netlist and so a
slightky different placement. I was lazy, I didn't manage that case.
I will do it this evening...
Comment 60 Luke Kenneth Casson Leighton 2020-04-20 19:10:27 BST
(In reply to Jean-Paul.Chaput from comment #59)
> (In reply to Luke Kenneth Casson Leighton from comment #58)
> > (In reply to Jean-Paul.Chaput from comment #57)
> >   File "doAlu16Flat.py", line 89, in build
> >     result = self.route()
> >   File "/home/lkcl/soclayout/experiments7/utils.py", line 191, in route
> >     katana.loadGlobalRouting(Anabatic.EngineLoadGrByNet)
> > hurricane.HurricaneError: [BUG] Unmanaged Configuration [16843266] =
> > [2+2+0+1,1+0] <id:1492 Net "a(5)" e-- LOGICAL i--- (IN)> in <id:5325
> > Anabatic::GCell <Box 200l 0l 250l 50l> ----------M---,i-A- 15>
> >       The global routing seems to be defective.
> 
> Argh! Yes everything was commited. But, as we are not 100% clone of
> each other (what Intel would call "copy exact" for its foundries),
> you may have got a slighty different Yosys netlist and so a
> slightly different placement. 

iinteresting.  i remember your warning about that on bug #178.

ahh so in case you need to repro that:

$ cd ~/yosys
$ git diff
diff --git a/Makefile b/Makefile
index e9dfd9df..8a935579 100644
--- a/Makefile
+++ b/Makefile
@@ -128,7 +128,7 @@ bumpversion:
 # is just a symlink to your actual ABC working directory, as 'make mrproper'
 # will remove the 'abc' directory and you do not want to accidentally
 # delete your work on ABC..
-ABCREV = 71f2b40
+ABCREV = default
 ABCPULL = 1
 ABCURL ?= https://github.com/berkeley-abc/abc

> I was lazy, I didn't manage that case.
> I will do it this evening...

i'll git pull on yosys and recompile.

commit ae115fa3aac9acf7534b3f7c08afaa1b86bfc4ad
Merge: 2b1fb8c0 35990b95
Author: Claire Wolf <clifford@clifford.at>
Date:   Mon Apr 20 14:51:40 2020 +0200
Comment 61 Luke Kenneth Casson Leighton 2020-04-20 19:25:23 BST
wark.  nope :)

hurricane.HurricaneError: [BUG] Unmanaged Configuration [16843266] = [2+2+0+1,1+0] <id:1492 Net "a(5)" e-- LOGICAL i--- (IN)> in <id:5325 Anabatic::GCell <Box 200l 0l 250l 50l> ----------M---,i-A- 14>
      The global routing seems to be defective.
Comment 62 Luke Kenneth Casson Leighton 2020-04-20 19:33:55 BST
this is a new one, too:

$ make clean
$ make vst
$ python doAlu16.py

        <event:00000659 0:917.91:52l> &<id:4837 Vertical o(11) METAL3 [125l 535l] [125l 585l] 2l rpD:2 -----CG-----w-----bb- [534l:586l] 52l 0------>
        <event:00000660 0:662.935:47l> &<id:4806 Vertical o(2) METAL3 [125l 90l] [125l 135l] 2l rpD:2 -----CG-----w-----bb- [89l:136l] 47l 0------>
        <event:00000661 0:662.935:12l> &<id:4846 Vertical o(4) METAL3 [125l 190l] [125l 200l] 2l rpD:2 -----CG-----w-----bb- [189l:201l] 12l 0------>
        <event:00000662 0:662.935:7l> &<id:4869 Vertical o(10) METAL3 [125l 495l] [125l 500l] 2l rpD:2 -----CG-----w-----bb- [494l:501l] 7l 0------>
     o  Repair Stage.
        <repair.queue:00000000>
  o  Computing statistics.
     - Processeds Events Total ............................................. 663
     - Unique Events Total ................................................. 572
     - # of GCells .......................................................... 64
     - Track Segment Completion Ratio ............................. 100% [572+0]
     - Wire Length Completion Ratio ............................. 100% [11039+0]
     - Wire Length Expand Ratio ............................. -1.02% [min:11153]
     - Unrouted horizontals .......................................... -nan% [0]
     - Unrouted verticals ............................................ -nan% [0]
     - Done in ................................................. 0.03s, 413696Kb
     - Raw measurements .............................. 0.030762s, +404Kb/287.8Mb
  o  Checking Katana Database coherency.
[STOP] After routing <id:119 Cell add>
       Type <y> to continue, <n> to abort: (y) 

(y here)

  File "/home/lkcl/soclayout/experiments7/utils.py", line 193, in route
    katana.runNegociate(Katana.Flags.NoFlags)
hurricane.HurricaneError: [ERROR] NegociateWindow::createTrackSegment(): No track near axis of <id:9171 Vertical b(5) METAL3 [-1l 275l] [-1l 275l] 2l rpD:15 -----C-------AS-il--->.
Comment 63 Jean-Paul Chaput 2020-04-20 20:00:33 BST
(In reply to Jean-Paul.Chaput from comment #59)
> (In reply to Luke Kenneth Casson Leighton from comment #58)
> > (In reply to Jean-Paul.Chaput from comment #57)
> >   File "doAlu16Flat.py", line 89, in build
> >     result = self.route()
> >   File "/home/lkcl/soclayout/experiments7/utils.py", line 191, in route
> >     katana.loadGlobalRouting(Anabatic.EngineLoadGrByNet)
> > hurricane.HurricaneError: [BUG] Unmanaged Configuration [16843266] =
> > [2+2+0+1,1+0] <id:1492 Net "a(5)" e-- LOGICAL i--- (IN)> in <id:5325
> > Anabatic::GCell <Box 200l 0l 250l 50l> ----------M---,i-A- 15>
> >       The global routing seems to be defective.
> 
> Argh! Yes everything was commited. But, as we are not 100% clone of
> each other (what Intel would call "copy exact" for its foundries),
> you may have got a slighty different Yosys netlist and so a
> slightky different placement. I was lazy, I didn't manage that case.
> I will do it this evening...

  This should be corrected by the last commit.
Comment 64 Jean-Paul Chaput 2020-04-20 20:08:59 BST
(In reply to Luke Kenneth Casson Leighton from comment #62)
> this is a new one, too:
> 
> $ make clean
> $ make vst
> $ python doAlu16.py
> 
>         <event:00000659 0:917.91:52l> &<id:4837 Vertical o(11) METAL3 [125l
> 535l] [125l 585l] 2l rpD:2 -----CG-----w-----bb- [534l:586l] 52l 0------>
>         <event:00000660 0:662.935:47l> &<id:4806 Vertical o(2) METAL3 [125l
> 90l] [125l 135l] 2l rpD:2 -----CG-----w-----bb- [89l:136l] 47l 0------>
>         <event:00000661 0:662.935:12l> &<id:4846 Vertical o(4) METAL3 [125l
> 190l] [125l 200l] 2l rpD:2 -----CG-----w-----bb- [189l:201l] 12l 0------>
>         <event:00000662 0:662.935:7l> &<id:4869 Vertical o(10) METAL3 [125l
> 495l] [125l 500l] 2l rpD:2 -----CG-----w-----bb- [494l:501l] 7l 0------>
>      o  Repair Stage.
>         <repair.queue:00000000>
>   o  Computing statistics.
>      - Processeds Events Total .............................................
> 663
>      - Unique Events Total .................................................
> 572
>      - # of GCells
> .......................................................... 64
>      - Track Segment Completion Ratio ............................. 100%
> [572+0]
>      - Wire Length Completion Ratio ............................. 100%
> [11039+0]
>      - Wire Length Expand Ratio ............................. -1.02%
> [min:11153]
>      - Unrouted horizontals .......................................... -nan%
> [0]
>      - Unrouted verticals ............................................ -nan%
> [0]
>      - Done in ................................................. 0.03s,
> 413696Kb
>      - Raw measurements .............................. 0.030762s,
> +404Kb/287.8Mb
>   o  Checking Katana Database coherency.
> [STOP] After routing <id:119 Cell add>
>        Type <y> to continue, <n> to abort: (y) 
> 
> (y here)
> 
>   File "/home/lkcl/soclayout/experiments7/utils.py", line 193, in route
>     katana.runNegociate(Katana.Flags.NoFlags)
> hurricane.HurricaneError: [ERROR] NegociateWindow::createTrackSegment(): No
> track near axis of <id:9171 Vertical b(5) METAL3 [-1l 275l] [-1l 275l] 2l
> rpD:15 -----C-------AS-il--->.

  I was hoping it will not show... The current workaround is to slightly enlarge
  the block abutment box, 10 lambdas by 10 lambdas.
Comment 65 Jean-Paul Chaput 2020-04-20 20:16:48 BST
(In reply to Luke Kenneth Casson Leighton from comment #60)
> (In reply to Jean-Paul.Chaput from comment #59)
> > (In reply to Luke Kenneth Casson Leighton from comment #58)
> > > (In reply to Jean-Paul.Chaput from comment #57)
> > >   File "doAlu16Flat.py", line 89, in build
> > >     result = self.route()
> > >   File "/home/lkcl/soclayout/experiments7/utils.py", line 191, in route
> > >     katana.loadGlobalRouting(Anabatic.EngineLoadGrByNet)
> > > hurricane.HurricaneError: [BUG] Unmanaged Configuration [16843266] =
> > > [2+2+0+1,1+0] <id:1492 Net "a(5)" e-- LOGICAL i--- (IN)> in <id:5325
> > > Anabatic::GCell <Box 200l 0l 250l 50l> ----------M---,i-A- 15>
> > >       The global routing seems to be defective.
> > 
> > Argh! Yes everything was commited. But, as we are not 100% clone of
> > each other (what Intel would call "copy exact" for its foundries),
> > you may have got a slighty different Yosys netlist and so a
> > slightly different placement. 
> 
> iinteresting.  i remember your warning about that on bug #178.
> 
> ahh so in case you need to repro that:
> 
> $ cd ~/yosys
> $ git diff
> diff --git a/Makefile b/Makefile
> index e9dfd9df..8a935579 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -128,7 +128,7 @@ bumpversion:
>  # is just a symlink to your actual ABC working directory, as 'make mrproper'
>  # will remove the 'abc' directory and you do not want to accidentally
>  # delete your work on ABC..
> -ABCREV = 71f2b40
> +ABCREV = default
>  ABCPULL = 1
>  ABCURL ?= https://github.com/berkeley-abc/abc
> 
> > I was lazy, I didn't manage that case.
> > I will do it this evening...
> 
> i'll git pull on yosys and recompile.
> 
> commit ae115fa3aac9acf7534b3f7c08afaa1b86bfc4ad
> Merge: 2b1fb8c0 35990b95
> Author: Claire Wolf <clifford@clifford.at>
> Date:   Mon Apr 20 14:51:40 2020 +0200

I think this is a more wider issue. Throughout all the project, all the versions
of the tools should be fixed, and when bumped everybody should do it at the same
time. And when the chip is finished, a full snapshot of all the versions should
be packed alongside the chip source code.

This is the way we did it at LIP6.
Comment 66 Jacob Lifshay 2020-04-20 20:20:50 BST
(In reply to Jean-Paul.Chaput from comment #65)
> I think this is a more wider issue. Throughout all the project, all the
> versions
> of the tools should be fixed, and when bumped everybody should do it at the
> same
> time. And when the chip is finished, a full snapshot of all the versions
> should
> be packed alongside the chip source code.

Sounds like we should use git submodules!
Comment 67 Luke Kenneth Casson Leighton 2020-04-20 20:52:31 BST
(In reply to Jacob Lifshay from comment #66)

> Sounds like we should use git submodules!

with each of the build dependencies with their specific versions
added, yeah.
Comment 68 Luke Kenneth Casson Leighton 2020-04-20 21:45:44 BST
how come i manage to break things all the time??
https://gitlab.lip6.fr/vlsi-eda/coriolis/-/issues/9
:)
Comment 69 Luke Kenneth Casson Leighton 2020-04-20 22:00:21 BST
mooooo
https://gitlab.lip6.fr/vlsi-eda/alliance/-/issues/1

placement (just visual i hope) has the positioning ignored under
certain circumstances.  it is very weird.
Comment 70 Jean-Paul Chaput 2020-04-20 23:17:18 BST
(In reply to Luke Kenneth Casson Leighton from comment #68)
> how come i manage to break things all the time??
> https://gitlab.lip6.fr/vlsi-eda/coriolis/-/issues/9
> :)

That's half a bug ;-). Put a breakpoint just before the final routing
stage, to see the placement. You will see that it is broken.

In particular, no cell must be outside the abutment box for the
routing to work. It shouldn't end up in core dump, but it issue
a lot of warning/erros before. Especially the "No GCell under"
which means that something to connect to is outside the AB, that
must never happens.
Comment 71 Luke Kenneth Casson Leighton 2020-04-21 13:12:40 BST
jean-paul, a quick hack-patch:
https://gitlab.lip6.fr/vlsi-eda/coriolis/-/issues/10

not loading the full views results in losing the offsets during
recursive placement.
Comment 72 Luke Kenneth Casson Leighton 2020-04-21 14:17:16 BST
ok i managed to:

* crush doAluFlat.py down to 465x800

* crush doAlu.py down to 520x840

visually you can see that there is plenty of room for improvement,
it is quite fascinating to examine.
Comment 73 Jean-Paul Chaput 2020-04-21 15:32:17 BST
(In reply to Luke Kenneth Casson Leighton from comment #72)
> ok i managed to:
> 
> * crush doAluFlat.py down to 465x800
> 
> * crush doAlu.py down to 520x840
> 
> visually you can see that there is plenty of room for improvement,
> it is quite fascinating to examine.

Yes. What would be very informative is to try with increasing size
of blocks (16, 32, 64, 128 bits) to see if the "flat" advantage
persists.

I'm pretty sure we can further compress with an optimized manual
routing of the inter-block (especially by using M2+M4 correct
balance for vertical routing)
Comment 74 Luke Kenneth Casson Leighton 2020-04-21 16:12:32 BST
(In reply to Jean-Paul.Chaput from comment #73)
> (In reply to Luke Kenneth Casson Leighton from comment #72)
> > ok i managed to:
> > 
> > * crush doAluFlat.py down to 465x800
> > 
> > * crush doAlu.py down to 520x840
> > 
> > visually you can see that there is plenty of room for improvement,
> > it is quite fascinating to examine.
> 
> Yes. What would be very informative is to try with increasing size
> of blocks (16, 32, 64, 128 bits) to see if the "flat" advantage
> persists.

fascinating. doAlu16.py - worked immediately (after i worked out that
you have to expand the space down the middle to accommodate the extra
tracks, duh)

took about.... 40 seconds to make the code changes: 600.0 width worked
perfectly (good guess, because, later, trying 590 failed).

doAlu16Flat.py - couldn't find a width that would work, not after trying
about 8 times to expand the width.  600 failed.  700 failed with
a missing config (https://gitlab.lip6.fr/vlsi-eda/coriolis/-/issues/11)
800 failed.  900 failed.

> I'm pretty sure we can further compress with an optimized manual
> routing of the inter-block (especially by using M2+M4 correct
> balance for vertical routing)

honestly i think this experiment has worked really well.  the only ways
i would think it worthwhile to expand is in the identification of the
blocks involved in each bit (the muxes).

we do actually need that for another part of the project, because, similarly,
i have some SR Latches which are specified just like the MUXes.

this is a standard recursive graph-walk algorithm, from edges to nodes
to find more (unique, not-previously-seen) edges:

1. here are the starting inputs/outputs
2. what cells are connected to them
3. add their inputs and outputs to a NEW list to recursively search
4. make sure that in the NEW list, the OLD list is excluded from the search
5. repeat from step 2. with the new list

a loop on that algorithm with first [a0, b0, o0] then [a1, b1, o1] etc.
will identify each "group" of cells.

then, it would be good to have a way to specify that only each *group*
should be Etesian-placed, within a manually-created abutment box.

whilst it is by no means perfect (each place a0b0o0, a1b1o1... a15b15o15
could theoretically be in different locations), to identify the cells
algorithmically from the inputs/outputs is much better than to hard-code
what yosys/ABC produces.
Comment 75 Luke Kenneth Casson Leighton 2020-04-21 16:22:10 BST
(In reply to Luke Kenneth Casson Leighton from comment #74)

> this is a standard recursive graph-walk algorithm, from edges to nodes
> to find more (unique, not-previously-seen) edges:
> 
> 1. here are the starting inputs/outputs
> 2. what cells are connected to them
> 3. add their inputs and outputs to a NEW list to recursively search
> 4. make sure that in the NEW list, the OLD list is excluded from the search
> 5. repeat from step 2. with the new list

i will see if i can do this.
Comment 76 Luke Kenneth Casson Leighton 2020-04-21 16:49:51 BST
+        if False:
+            print (dir(self.cell))
+            for net in self.cell.getNets():
+                print (net.getName())
+            print (self.cell.getNet("a(0)"))
+            print (dir(self.cell.getNet("a(0)")))
+            for inst in self.cell.getInstances():
+                for net in self.cell.getNets():
+                    if net.getName() == "a(1)":
+                        icell = inst.getMasterCell()
+                        #if len(list(icell.getInstances())) > 0:
+                            #continue # already placed, do not include it
+                        for net in icell.getNets():
+                            print ("inst", icell, "has nets", net.getName())
+            sys.exit(0)
+

i managed to walk the instances pins (of the cell's inputs and outputs,
not what the cell is *connected* to).

hmm...
Comment 77 Jean-Paul Chaput 2020-04-21 17:16:43 BST
(In reply to Luke Kenneth Casson Leighton from comment #76)
> +        if False:
> +            print (dir(self.cell))
> +            for net in self.cell.getNets():
> +                print (net.getName())
> +            print (self.cell.getNet("a(0)"))
> +            print (dir(self.cell.getNet("a(0)")))
> +            for inst in self.cell.getInstances():
> +                for net in self.cell.getNets():
> +                    if net.getName() == "a(1)":
> +                        icell = inst.getMasterCell()
> +                        #if len(list(icell.getInstances())) > 0:
> +                            #continue # already placed, do not include it
> +                        for net in icell.getNets():
> +                            print ("inst", icell, "has nets", net.getName())
> +            sys.exit(0)
> +
> 
> i managed to walk the instances pins (of the cell's inputs and outputs,
> not what the cell is *connected* to).
> 
> hmm...

The concept you are missing here, is the "Plug". A plug is a logical connector
of an instance.

* Plug belongs to Instance. One Plug per external Net.
* Plug may be connected to a Net in the Cell the instance is in
  (method getNet())
* Plug are connected to the Net of the Cell the instance represent,
  called the "master net" (method: getMasterNet()).
* Plug are derived from Components. When a Plug is connected to a Net,
  is put in it's components. So you may find all that is logically
  connected to the Net with net.getPlugs().
* Of courses Instance also have access to all it's Plugs: inst.getPlugs().

So, from the Plug you can know the Net, the Master Net and the instance.
Form the Net you know the Plugs.
From the Instance you can know the PLugs.
Comment 78 Luke Kenneth Casson Leighton 2020-04-21 17:55:11 BST
        print (dir(self.cell))
        for net in self.cell.getNets():
            print (net.getName())
        print (self.cell.getNet("a(0)"))
        print (dir(self.cell.getNet("a(0)")))
        net = self.cell.getNet("a(2)")
        print (list(net.getPlugs()))
        for plug in net.getPlugs():
            print (plug, dir(plug))
            components = list(plug.getConnexComponents())
            pc = plug.getCell()
            print ("plug cell", pc)
            mnet = plug.getMasterNet()
            print ("mstnet", mnet)

hmmm this is only printing out add and sub instances
<id:3628 Plug a(2) subckt_48_add.a(2)>
<id:3495 Plug a(2) subckt_49_sub.a(2)>

the reason for that, i am guessing, is because they've been instantiated
(placed).

i need the Plugs that have *not* been instantiated.
Comment 79 Luke Kenneth Casson Leighton 2020-04-21 18:03:08 BST
getConnectedPlugs().  maybe.
Comment 80 Jean-Paul Chaput 2020-04-21 18:40:47 BST
(In reply to Luke Kenneth Casson Leighton from comment #78)
>         print (dir(self.cell))
>         for net in self.cell.getNets():
>             print (net.getName())
>         print (self.cell.getNet("a(0)"))
>         print (dir(self.cell.getNet("a(0)")))
>         net = self.cell.getNet("a(2)")
>         print (list(net.getPlugs()))
>         for plug in net.getPlugs():
>             print (plug, dir(plug))
>             components = list(plug.getConnexComponents())
>             pc = plug.getCell()
>             print ("plug cell", pc)
>             mnet = plug.getMasterNet()
>             print ("mstnet", mnet)
> 
> hmmm this is only printing out add and sub instances
> <id:3628 Plug a(2) subckt_48_add.a(2)>
> <id:3495 Plug a(2) subckt_49_sub.a(2)>
> 
> the reason for that, i am guessing, is because they've been instantiated
> (placed).
> 
> i need the Plugs that have *not* been instantiated.

  Duh? It seems ok to me, according to the netlist of alu16, top level
  "a(2)" is connected only to "add" and "sub". Plug represent logical
  connexions, so they cannot be "placed". The instance they belong to,
  however, may be placed. What Plug connected to "a(2)" do you miss?
Comment 81 Luke Kenneth Casson Leighton 2020-04-21 19:05:38 BST
(In reply to Jean-Paul.Chaput from comment #80)

>   Duh? It seems ok to me, according to the netlist of alu16, top level
>   "a(2)" is connected only to "add" and "sub". Plug represent logical
>   connexions, so they cannot be "placed". The instance they belong to,
>   however, may be placed. What Plug connected to "a(2)" do you miss?

tch, tch, you are right.  i needed add_o(2) and sub_o(2).

        find = self.get_net_connections(['add_o(0)', 'sub_o(0)', 'o(0)'],
                                        ['clk', 'rst'])

more ['op', 'abc_829_new_n51', 'op', 'abc_829_new_n51', 'o_next(0)']
set([[0x7fe5b3dd9110<->0x55c7b4b77b10 <id:3433 Instance subckt_0_nmx2_x1 nmx2_x1>], [0x7fe5b3dd9130<->0x55c7b4b92b30 <id:3801 Instance subckt_32_sff1_x4 sff1_x4>]])

at the first level of recursion, it seems to be working.  for a given value of work.

this will, recursively, grab all unconnected nets that are connected to nets that are... etc.

however that is too much.

this algorithm can be used to resolve all paths between two points: 
https://www.python.org/doc/essays/graphs/

so by knowing that we want all components involved in add_0(0), sub_o(0)
and o(0) it is possible to find...

hmmm... net "op" is a problem.  if you traverse "op" it goes everywhere.

by excluding "op" as a possibility, i managed to get it to "work"
recursively:

        find = self.get_net_connections(['add_o(1)', 'sub_o(1)', 'o(1)'],
                                        ['clk', 'rst', 'op'])

set([[0x7f13ce86c070<->0x559cb535bc20 <id:3769 Instance subckt_33_sff1_x4 sff1_x4>], [0x7f13ce86c130<->0x559cb535a680 <id:3755 Instance subckt_2_nmx2_x1 nmx2_x1>], [0x7f13ce86c190<->0x559cb5352100 <id:3668 Instance subckt_3_no2_x1 no2_x1>]])

first list is what is wanted (start position), second list is what you do
*not* want.  anything already found will be added recursively to the second
list.
Comment 82 Jean-Paul Chaput 2020-04-21 20:24:56 BST
(In reply to Luke Kenneth Casson Leighton from comment #81)
> (In reply to Jean-Paul.Chaput from comment #80)

I'm still not sure about the exact walktrough you want, but if you want
to have all the connexions (Plugs) to a given net through all the
hierarchy levels, you may use the HyperNet object. You can then
select any Plug with the occurrence Path.
Comment 83 Luke Kenneth Casson Leighton 2020-04-21 21:23:01 BST
(In reply to Jean-Paul.Chaput from comment #82)
> (In reply to Luke Kenneth Casson Leighton from comment #81)
> > (In reply to Jean-Paul.Chaput from comment #80)
> 
> I'm still not sure about the exact walktrough you want, 

i want to replace ALU16.place_data_path() with an automated system.



1) given a net ("o(2)") find all unrouted Instances that are left which need placing.

and *only* those for "o(2)".

not those connecting to "o(3)", or o(0).

the function Module.get_net_connections now achieves that job.

get_net_connections("o(2)") returns these:

* Instance subckt_31_no2_x1 no2_x1
* Instance subckt_47_sff1_x4 sff1_x4
* Instance subckt_30_nmx2_x1 nmx2_x1


2) given those unplaced components, perform an *automatic* (Etesian)
   place on those components, within a small abutment box.




> but if you want
> to have all the connexions (Plugs) to a given net through all the
> hierarchy levels, you may use the HyperNet object. You can then
> select any Plug with the occurrence Path.

will look at that one later, i think i have the function for part (1)
already.
Comment 84 Luke Kenneth Casson Leighton 2020-04-21 21:43:43 BST
(In reply to Luke Kenneth Casson Leighton from comment #83)

> 2) given those unplaced components, perform an *automatic* (Etesian)
>    place on those components, within a small abutment box.

in theory, once the group of [nmx2, no2 and sff1] for each connection
between add, sub and the output, could follow this process:

for i in range(BIT_WIDTH)

0) identify the [nmx2+no2+sff1] cells connected to output(i) add(i) and sub(i)
1) create a "fake" View (both physical and logical) fake(i)
2) ***REMOVE*** the nmx2+no2+sff1 instances from alu16
3) ***ADD*** them to the "fake" View
4) perform the "normal" Etesian place on fake(i)

this would achieve the desired task, with zero changes to coriolis2.
Comment 85 Luke Kenneth Casson Leighton 2020-04-21 22:19:55 BST
ok i have replaced ALU16.place() with a new version.

commit 83f918107bb29f1ab3ef08eae5c1624d0376179b
Author: Luke Kenneth Casson Leighton <lkcl@lkcl.net>
Date:   Tue Apr 21 21:05:18 2020 +0000

    automatically located the joining cells between add and sub


this new version no longer explicitly regexp pattern-searches for
the cells named nmx2, no2 and sff1

instead it uses the new function, calling get_net_connections("o(%d)" % i),
for all i in range(16).

it produces - finds - the exact same three cells as the regexp pattern
search ALU16.match_instance()

however unlike match_instance(), if the yosys/ABC changes, produces
completely difference cells, it will *still find them all*.

now we need step (2)

step (2) is:

* given the (arbitrary) list of instances, auto-place them within a set
  box.

you can see i have started to try that, and it produces a segfault.

attempting to call Etesian.create(inst.getCell()) does not work.

attempting to call Etesian.create(inst.getMasterCell()) does not work.

attempting to call Etesian.create(inst) segfaults.

the only other trick i can think of is to LITERALLY remove them,
programmatically, from alu16, and move them to a new (fake) Cell.

times 16.

this will be extremely tedious because it requires moving the netlists
as well.



what is *really* needed is, to be able to pass a **LIST** of Cells
to Etesian.create().  (actually, a list of Instances)

and to be able to have EtesianEngine constructor take that **LIST**
of Instances

and to perform the place on that **LIST** of Instances.
Comment 86 Jean-Paul Chaput 2020-04-21 23:00:16 BST
(In reply to Luke Kenneth Casson Leighton from comment #85)
> attempting to call Etesian.create(inst.getMasterCell()) does not work.
> 
> attempting to call Etesian.create(inst) segfaults.

  Oups. Should throw an exception, not segfault... Will correct.

> the only other trick i can think of is to LITERALLY remove them,
> programmatically, from alu16, and move them to a new (fake) Cell.
> 
> times 16.
> 
> this will be extremely tedious because it requires moving the netlists
> as well.

  Something similar is done for the clock tree. It is nightmarish,
  don't go that way.
 
> what is *really* needed is, to be able to pass a **LIST** of Cells
> to Etesian.create().  (actually, a list of Instances)
> 
> and to be able to have EtesianEngine constructor take that **LIST**
> of Instances
> 
> and to perform the place on that **LIST** of Instances.

  I see no problem implementing that feature, but will do it next
  week. I'm long overdue to help an intern...
Comment 87 Luke Kenneth Casson Leighton 2020-04-21 23:21:55 BST
(In reply to Jean-Paul.Chaput from comment #86)

> > this will be extremely tedious because it requires moving the netlists
> > as well.
> 
>   Something similar is done for the clock tree. It is nightmarish,
>   don't go that way.

:)

the alternative is that we do it at the HDL level.  create the series of "modules" ehich result in separate actual VST files, link_add0_sub0_out0.vst etc.

also tedious :)
 
 
> > what is *really* needed is, to be able to pass a **LIST** of Cells
> > to Etesian.create().  (actually, a list of Instances)

>   I see no problem implementing that feature,

that would be extremely cool.

it will have many uses, in combination with the netlist subset finder we can have much better control over placement, without complicating the HDL.

> but will do it next
>   week. I'm long overdue to help an intern...

no problem.  thank you JP.