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

lower: fix a bug causing undefined variables when applying fuse #362

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

rohany
Copy link
Contributor

@rohany rohany commented Jan 14, 2021

Fixes #355.

This commit fixes a bug where the fuse transformation would not generate
necessary locator variables when applied to iteration over two dense
variables.

Additionally, this commit adds a test for when a dense iteration results
in a transposition of a tensor.

@rohany
Copy link
Contributor Author

rohany commented Jan 14, 2021

cc @stephenchouca, this is another attempt at #360. Instead of generating parent locators at a particular point, I instead generate locators on the way down through the iteration graph for variables that must be recovered.

Fixes tensor-compiler#355.

This commit fixes a bug where the fuse transformation would not generate
necessary locator variables when applied to iteration over two dense
variables.

Additionally, this commit adds a test for when a dense iteration results
in a transposition of a tensor.
@stephenchouca
Copy link
Contributor

Looks good as far as I can tell! Thanks!

@stephenchouca stephenchouca merged commit 468ad7f into tensor-compiler:master Jan 20, 2021
@Infinoid
Copy link
Contributor

Hi,

This patch causes a test failure for me.

Configure with -DCUDA=ON. Then run the scheduling_eval.spmvGPU test:

bin/taco-test --gtest_filter=scheduling_eval.spmvGPU

With revision 468ad7f (merge this PR):

% bin/taco-test --gtest_filter=scheduling_eval.spmvGPU
Note: Google Test filter = scheduling_eval.spmvGPU
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from scheduling_eval
[ RUN      ] scheduling_eval.spmvGPU
zsh: segmentation fault (core dumped)  bin/taco-test --gtest_filter=scheduling_eval.spmvGPU

With revision 468ad7f^ (master branch prior to merge):

% bin/taco-test --gtest_filter=scheduling_eval.spmvGPU
Note: Google Test filter = scheduling_eval.spmvGPU
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from scheduling_eval
[ RUN      ] scheduling_eval.spmvGPU
[       OK ] scheduling_eval.spmvGPU (8408 ms)
[----------] 1 test from scheduling_eval (8408 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (8408 ms total)
[  PASSED  ] 1 test.

When I trigger it in gdb:

Thread 1 "taco-test" received signal SIGSEGV, Segmentation fault.
taco::LowererImpl::declLocatePosVars (this=0x55555665ddf0, locators=...)
    at /home/infinoid/workspace/taco/git/src/lower/lowerer_impl.cpp:2288
2288	        taco_iassert(isValue(locate.getResults()[1], true));
(gdb) p locateIterator.hasLocate()
$1 = false
(gdb) p locate
$2 = {content = std::shared_ptr<taco::ModeFunction::Content> (empty) = {get() = 0x0}}

Apparently the ModeFormat claims that it can't locate, and locateIterator.locate() returns a NULL pointer. The call to locate.getResults() then segfaults.

I also noticed that the coords argument to locateIterator.locate() is a vector of 2 values in this case, it contains fposA twice. Normally coords only contains a single Var.

(gdb) p coords
$1 = std::vector of length 2, capacity 2 = {{<taco::ir::IRHandle> = {<taco::util::IntrusivePtr<taco::ir::IRNode const>> = {
        _vptr.IntrusivePtr = 0x555555a0e778 <vtable for taco::ir::Expr+16>, 
        ptr = 0x55555665c8e0}, <No data fields>}, <No data fields>}, 
  {<taco::ir::IRHandle> = {<taco::util::IntrusivePtr<taco::ir::IRNode const>> = {
        _vptr.IntrusivePtr = 0x555555a0e778 <vtable for taco::ir::Expr+16>, 
        ptr = 0x55555665c8e0}, <No data fields>}, <No data fields>}}

Github CI didn't catch this failure, because it doesn't run the GPU tests.

@rohany
Copy link
Contributor Author

rohany commented Jan 28, 2021

Can you open an issue about it? I or someone else can take a look at it later

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.

scheduling: bug in fuse transformation
3 participants