search

wsmoses / Tapir-Meta

24 stars 7 forks source link

nqueens without optimizations #3

Closed stelleg closed 6 years ago

stelleg commented 7 years ago

I'm seeing an issue with the nqueens example from the test tarball from the paper with optimizations turned off. With -On for n>0, I get the correct result, but with -O0 it's returning between zero and a few hundred solutions for n=13 for large CILK_NWORKERS.

Not sure how to debug it, as I don't see an obvious way to dump the tapir IR. Is there an analogous -emit-tapir to llvm's -emit-llvm?

To reproduce, follow build instructions from the readme, then build the nqueens.c from the testing.tar linked in the paper testing/cilkbench/nqueens.c. Then build with:clang -ftapir -DCILKP -fno-exceptions -march=native -D__INTEL_COMPILER=1 -O0 -DPIR nqueens.c -o nqueens. Then run with CILK_NWORKERS=64 ./nqueens.

neboat commented 7 years ago

I'm looking into this. In the meantime, you might try updating the checkouts of the tapir and tapir/tools/clang submodules to use the latest contents of the master branch for their respective repositories. We might have pushed a bugfix for -O0 builds to master, but not yet propagated the fix to stable. If you give that a try, please let us know either way whether that resolves the issue you're seeing.

In general, you can take a look at the Tapir IR for a given program by dropping the "-ftapir" flag and adding "-S -emit-llvm". When you do this, clang might issue a warning about Cilk not being enabled. That warning should be harmless, and you should see IR with detach, reattach, and sync instructions.

stelleg commented 7 years ago

Unfortunately updating to master for the llvm and clang submodules didn't help.

Thanks for the IR dump info, that's working well for me. I'll poke around a bit and see if anything obvious jumps out at me.

neboat commented 7 years ago

Can you confirm that your updated tapir submodule is at commit 9b55aac80, and your updated tapir/tools/clang submodule is at commit 9ab51fd49? If you're on those commits, then you should be able to drop the -DPIR compile flag. I'm not sure that will fix the issue you're seeing, but it should resolve at least one issue I noticed with the code.

stelleg commented 7 years ago

Ah, that fixed it. Using the PIR format seems to have caused the issue. Using the standard cilk format instead works as expected now. I think with the earlier commits that were default with this meta package only the PIR format would compile. Maybe it would be worth updating up the submodule commits that are checked out by build.sh?

Thanks for the help!

neboat commented 7 years ago

No problem! I'm glad we were able to resolve this.

It's definitely worth updating the submodules. We'll probably do that once we've tested the latest changes to those repositories a bit more.

wsmoses commented 7 years ago

I'm slightly inclined to move from submodules to the script doing a separate checkout simply because submodules seem to have rather nasty maintenance. Thoughts?