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

Generalizes TACO Merge lattice construction #390

Merged
merged 34 commits into from
Feb 4, 2022

Conversation

rawnhenry
Copy link
Collaborator

@rawnhenry rawnhenry commented Feb 1, 2021

This PR should now be largely functional on for the TACO CPU build.

This PR attempts to generalize the lattice construction of TACO. It largely implements the concepts discussed in this Master thesis. In short, this PR adds:

  1. A generalized theory for Merge lattice construction - TACO can construct lattices for any iteration space instead of just complements and unions.
  2. Introduces the concept of a fill value in TACO.
  3. Introduces operator properties and allows TACO to infer some basic optimizations from these properties.

Why this is still in draft (TODO in priority order. ETA mid Feb)

  1. Fix bug from merge :(
  2. Fix segfault due to fill value with GPU backend :(
  3. Fix compiler crash when using indexVars that have been scheduled as the operands to funcs
  4. Fix segfault when running GPU spmv schedule - Looks like this is the same issue as scheduling_eval.spmvGPU test failure #389, will not fix
  5. The Merge Lattice implementation does not match the theory discussed in the thesis. The new theory is much simpler, requiring none of the complicated bookkeeping currently done in the code.
  6. The code responsible for extracting iteration algebra from Index Expressions currently does not handle nested Funcs.
  7. The code responsible for short circuiting will fail if attempting to break out of multiple loops. Need to add mechanism to detect this and insert goto statements when necessary. This is just a performance bug.

Current Limitations:

  1. Currently, TACO default to fill value of zero whenever one is not specified (this is done in the constructor of the tensorVar). It would be nice to do this for only RHS operands and allow TACO to set the fill value of LHS expressions during computation.

…w propagation of indexVar type to generated code
…omputing with IndexVars when using split command. Started implementing boilerplate code for iteration space algebra
…so users can create vectors of properties which are actually a vector of pointers. This is a work around to avoid object slicing when storing properties in a vector
…ith lowering generic tensorOp. Changed spec of special definition to take an IR function.
…ttice optimizations to the end of lattice construction. Conditions to apply these optimizations still not quite right so all tests pass for now. Need to also check that no producer regions have a special definition. Lowerer also needs to be altered to handle compute regions more generally instead of just sparse. Lowerer needs to be altered to handle explicit zeros.
…ice in lowerer. Added explicit zero checks in bottom loop of compute
…ing explicit zero checks before bottommost loop. Got Masked BFS optimization working for pull bfs!
…tice construction introduce in the merge. Seems to always include the dimension iterator in the merge points even when the iterators are sparse
@rawnhenry rawnhenry changed the title Array algebra Generalizes TACO Merge lattice theory Feb 1, 2021
@rawnhenry rawnhenry changed the title Generalizes TACO Merge lattice theory Generalizes TACO Merge lattice construction Feb 1, 2021
…orVar was merged incorrectly so all formats were dense inside the lattice
…the GPU backend to be completely non-functional
@rawnhenry
Copy link
Collaborator Author

The slice multiple ways test fails after the most recent merge from master. If you have a chance, would you mind taking a look @rohany?

It also seems that WindowedIndexVars should also inherit from IndexExpr so that they can be used in computations.

This commit fixes a bug caused by merging together the windowing and
array algebra work. In particular, the newly introduced deep equality
defined on `Access` types did not include additions for the newly added
components used in windowing.
@rohany
Copy link
Contributor

rohany commented Feb 18, 2021

The slice multiple ways test fails after the most recent merge from master. If you have a chance, would you mind taking a look @rohany?

I looked into it and opened a PR here rawnhenry#1. I believe that merging on your repository will cause this PR to get updated. I double checked that make test passes on that commit. This wasn't a fun one to track down though.

It also seems that WindowedIndexVars should also inherit from IndexExpr so that they can be used in computations.

The way I have this set up is that anything windowing related folds into an Access and is handled in lowering when looking at Access objects, rather than dealing with WindowedIndexVar's everywhere.

@rohany
Copy link
Contributor

rohany commented Feb 18, 2021

Once that PR is merged, we can set up the new branch. However, I don't have write permission here, so I wouldn't be able to make it. @stephenchouca either you can make the branch or add me and I'll make it.

@stephenchouca
Copy link
Contributor

stephenchouca commented Feb 18, 2021

I've created a new array_algebra branch based off of the current master. @RawnH, you should be able to modify this PR to merge into that branch.

lower: fix a bug introduced by merging windowing and array algebra
@weiya711 weiya711 merged commit 7d84d5d into tensor-compiler:master Feb 4, 2022
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.

4 participants