-
Notifications
You must be signed in to change notification settings - Fork 189
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
*: add support for windowing of tensors #372
*: add support for windowing of tensors #372
Conversation
744f88f
to
4fd7744
Compare
cf3369d
to
f364047
Compare
@@ -220,14 +223,24 @@ class Access : public IndexExpr { | |||
Access() = default; | |||
Access(const Access&) = default; | |||
Access(const AccessNode*); | |||
Access(const TensorVar& tensorVar, const std::vector<IndexVar>& indices={}); | |||
Access(const TensorVar& tensorVar, const std::vector<IndexVar>& indices={}, const std::map<int, AccessWindow>& windows={}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably break this line (and other longer lines) up into multiple lines. (Lines should be restricted to 80 characters at most where possible.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. clang-format
isn't set up on the repository, so I'm missing these small formatting things everywhere.
include/taco/tensor.h
Outdated
auto resultReversed = this->_access(args...); | ||
std::vector<std::shared_ptr<IndexVarInterface>> result; | ||
result.reserve(resultReversed.size()); | ||
for (auto it = resultReversed.rbegin(); it != resultReversed.rend(); it++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also use util::reverse
(defined in taco/util/collections.h
) for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/codegen/codegen_c.cpp
Outdated
@@ -516,7 +516,7 @@ void CodeGen_C::visit(const Allocate* op) { | |||
stream << ", "; | |||
} | |||
else { | |||
stream << "malloc("; | |||
stream << "calloc(1, "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you need calloc
instead of malloc
, you should make that optional since it can impose performance overhead and often it's not necessary to actually zero-initialize the array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, made it configurable based on the context.
test/tests-windowing.cpp
Outdated
// These dimensions are chosen so that one is above the constant in `mode_format_dense.cpp:54` | ||
// where the known stride is generated vs using the dimension. | ||
// TODO (rohany): Change that constant to be in a header file and import it here. | ||
for (auto& dim : {6, 20}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably implement this as parameterized tests using TEST_P
, so that each combination of dimensions/formats can be tested individually. (Same with the other test cases below.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I couldn't figure out how to get good names for the parameters to show up in the gtest output though.
This commit adds support for windowing of tensors in the existing index notation DSL. For example: ``` A(i, j) = B(i(1, 4), j) * C(i, j(5, 10)) ``` causes `B` to be windowed along its first mode, and `C` to be windowed along its second mode. In this commit any mix of windowed and non-windowed modes are supported, along with windowing the same tensor in different ways in the same expression. The windowing expressions correspond to the `:` operator to slice dimensions in `numpy`. Currently, only windowing by integers is supported. Windowing is achieved by tying windowing information to particular `Iterator` objects, as these are created for each `Tensor`-`IndexVar` pair. When iterating over an `Iterator` that may be windowed, extra steps are taken to either generate an index into the windowed space, or to recover an index from a point in the windowed space.
f364047
to
2d4a7d3
Compare
Looks good! I'll merge it into master. |
This commit adds support for windowing of tensors in the existing index
notation DSL. For example:
causes
B
to be windowed along its first mode, andC
to be windowedalong its second mode. In this commit any mix of windowed and
non-windowed modes are supported, along with windowing the same tensor
in different ways in the same expression. The windowing expressions
correspond to the
:
operator to slice dimensions innumpy
.Currently, only windowing by integers is supported.
Windowing is achieved by tying windowing information to particular
Iterator
objects, as these are created for eachTensor
-IndexVar
pair. When iterating over an
Iterator
that may be windowed, extrasteps are taken to either generate an index into the windowed space, or
to recover an index from a point in the windowed space.