Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cursor API Step 1 #220

Merged
merged 24 commits into from
Jul 29, 2022
Merged

Cursor API Step 1 #220

merged 24 commits into from
Jul 29, 2022

Conversation

gilbo
Copy link
Contributor

@gilbo gilbo commented Jul 23, 2022

This change switches over the entire scheduling API to use the

p = sched(p, ...)

convention instead of the

p = p.sched(...)

convention.

Additionally, the pattern arguments now accept either pattern strings or cursors. An API_Cursors.py wrapper for the internal cursors has been created, which are the standard for interface use now.

  • This PR does not resolve the problem of having cursor forwarding yet. That will be a "Cursor API Step 2" change in the future.
  • This PR has not yet made sure that any untested code is properly updated to use the new API conventions.
  • This PR makes some substantial cleanups to parts of the GEMMINI code, but is not a full cleanup of that code yet.
  • This PR will leave large chunks of code in API.py commented out but not deleted. This is intentional. It will help me more smoothly merge in other branches that have been on hold; it may help others merge any outstanding branches as well. In particular, I need this to help me merge in all of the yet-to-be-committed fixes to the program analyses.

@yamaguchi1024
Copy link
Member

We don't intend to expose @sched_op([GapCursorA]) decorators to users, right? I wonder if it should start with an underscore to ensure it's for internal use only. We have @proc and @instr() and users can reasonably mistaken that @sched_op is one of them.

from .configs import Config
from .memory import Memory, DRAM
from . import query_asts as QAST

from . import stdlib
Copy link
Member

Choose a reason for hiding this comment

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

I am really not sure if it's a good idea to name our standard library stdlib. Maybe just primitives or ops ? That's more consistent with e.g., apps/ and platforms/, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is that stdlib lives under exo (i.e. exo.stdlib) in the Python module namespace. Ultimately, I expect stdlib to hold things like "standard memories" excluding apps and platforms which are not part of the standard exo-lang distribution on PyPI. We can create a second PyPI distribution for things like platforms libraries.

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm I think stdlib is not saying anything, shouldn't users be able to import DRAM etc. and all the scheduling primitives directly with from exo import * ? I just think exo.stdlib should be just as simple as exo. And we can have peripheral code such as builtins and platforms separately and be accessed with exo.builtins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, there will be a lot of scheduling primitives, not all of which will be atomic (e.g. repeat) and I expect that list to grow somewhat. Generally I would expect from exo import * to get you things like proc that you'll need to use regularly, and then you may want to specifically bind the namespace of scheduling operations to a shorthand like S or XS (exo scheduling?) via

from exo.stdlib import scheduling as S

Copy link
Contributor Author

Choose a reason for hiding this comment

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

builtins may be worth having under exo directly instead of stdlib. I'm not sure. As far as platforms, that shouldn't even be under exo in the namespace at all, and ultimately shouldn't be packaged with exo-lang in PyPI. We can create a separate exo-lang-platforms package if we want to have standard "platform support" with lower guarantees of quality than the core exo-lang distribution.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be exo.scheduling then? I'm not sure why having stdlib in the middle is useful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My reply to this got scrambled. My ultimate plan for this is to move most of the current contents of the exo directory into a subdirectory called something like compiler. At that point it may make sense to move scheduling to exo.scheduling and likewise for other submodules that we want users to import. However, right now, there would be no distinction between exo.scheduling and exo.LoopIR which definitely should not be imported by user code.

# Block-specific operations
# ------------------------------------------------------------------------ #

def expand(self, lo=None, hi=None):
Copy link
Member

Choose a reason for hiding this comment

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

I think it'll be useful if we can do a syntax sugar on this s.t.

c = cblock.next()
cblock = cblock + c

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there may be better choices. (I would use the union operator a | b for what you're suggesting, not +) This "expand" method is just there so I could solve one particular issue and move forward. Don't take it as a long term commitment for what the API should be.

Copy link
Member

Choose a reason for hiding this comment

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

Make sense. Shouldn't it be + because it's like expanding a list? union feels like unordered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a join in an appropriate lattice of sequences really. For example, suppose I have cursors to two statements with another statement in between.

s1 <- c1
s2
s3 <- c2

The smallest block containing the two statements with cursors (s1 and s3) is a block of all 3 statements. That's neither a concatenation nor union. It's a join of "intervals."

@@ -2,143 +2,172 @@

from exo import proc, instr, DRAM, config, QAST
from exo.libs.memories import GEMM_SCRATCH, GEMM_ACCUM
from exo.stdlib.scheduling import *
Copy link
Member

Choose a reason for hiding this comment

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

Gemmini changes are great, thank you for looking into this!
I'll ask Jennifer to test this on AWS before merging alongside with gemmini-rocc-tests stuff

@gilbo
Copy link
Contributor Author

gilbo commented Jul 25, 2022

We don't intend to expose @sched_op([GapCursorA]) decorators to users, right? I wonder if it should start with an underscore to ensure it's for internal use only. We have @proc and @instr() and users can reasonably mistaken that @sched_op is one of them.

The scheduling operators are exposed in src/exo/stdlib/scheduling.py. This file should never be imported by user-code.

@gilbo
Copy link
Contributor Author

gilbo commented Jul 28, 2022 via email

for k in seq(0, m):
x[i, j, k] = A[i + j + k]

foo = divide_dim(foo, 'x', 1, 4)
Copy link
Member

Choose a reason for hiding this comment

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

What will happen if there are multiple 'x' buffers in this proc? It seems the API does not loop over multiple Syms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the scheduling operations are now designed to apply exactly once. Handling multiple applications of a scheduling operation should now be handled using higher-order combinators. This will be even easier once step 2 of the API update is done, since then you can do something like p.find_all(pattern) to get a list of cursors, and then repeat an operation for each cursor in the list.

@gilbo gilbo merged commit d3adc77 into master Jul 29, 2022
@gilbo gilbo deleted the cursor-api branch July 29, 2022 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants