Page MenuHomePhabricator

[WIP] [Polly] [PPCG] Bump up PPCG to v0.07
AbandonedPublic

Authored by bollu on Jul 19 2017, 5:19 PM.

Event Timeline

bollu created this revision.Jul 19 2017, 5:19 PM
grosser edited edge metadata.Jul 19 2017, 9:39 PM

Nice. Is there any reason this is still marked as WIP? Sould I already have a look.

(Marking this as requesting changes to make sure this does not ye show up in my review queue)

grosser requested changes to this revision.Jul 19 2017, 9:39 PM
This revision now requires changes to proceed.Jul 19 2017, 9:39 PM
singam-sanjay added inline comments.Jul 19 2017, 10:33 PM
lib/CodeGen/PPCGCodeGeneration.cpp
58 ↗(On Diff #107423)

"polly-debug-print-bounds" ;-)

1169 ↗(On Diff #107423)

I'm interested to know as well. Please comment as soon as you get to know.

test/GPGPU/host-control-flow.ll
27 ↗(On Diff #107423)

maybe this should've been code generated for the "free_device" User node.

bollu added a comment.Jul 20 2017, 2:07 AM

@grosser: I wanted to rebase and get a clean history, but PPCGCodeGeneration can be reviewed. The test cases I am dubious about are:

  1. test/GPGPU/mostly-sequential.ll since we pull in an entire loop into the kernel
  2. test/GPGPU/non-read-only-scalars.ll since the data flow is wrong.
  3. test/GPGPU/phi-nodes-in-kernel.ll since we stopped sending a phi node (this makes sense, but I want a double check)
lib/CodeGen/PPCGCodeGeneration.cpp
58 ↗(On Diff #107423)

DO NOT COMMIT! :)
It's around for debugging.

1169 ↗(On Diff #107423)

Sure. For now, I'm relying on the fact that it worked before, so extra annotations shouldn't actually hurt. However, we do seem to mis-compile in 2 of the test cases. I'll have to see what code PPCG emits.

test/GPGPU/host-control-flow.ll
27 ↗(On Diff #107423)

Yes, possibly. However, we generate this already without PPCG helping us, do we not?

singam-sanjay added inline comments.Jul 20 2017, 4:17 AM
lib/CodeGen/PPCGCodeGeneration.cpp
1169 ↗(On Diff #107423)

GPUNodeBuilder has already been handling these cases. I think the schedule tree just become more detailed.

Do these nodes reside in a Block ? in the order { "init_device", "to_device"+, "kernel", "from_device"+, "clear_device" } ?

lib/External/ppcg/cuda.c
508–509

You were right ! We've already been doing stuff under "init_device" and "clear_device".

You could do something like,

if (!strcmp(Str, "init_device") ) {
   initializeAfterRTH();
   preloadInvariantLoads();
   isl_ast_node_free(UserStmt);
   isl_ast_expr_free(Expr);
   return;
 }
 if ( !strcmp(Str, "clear_device") ) {
   finalize();
   isl_ast_node_free(UserStmt);
   isl_ast_expr_free(Expr);
   return;
 }
bollu updated this revision to Diff 107481.Jul 20 2017, 4:28 AM
bollu edited edge metadata.

[Polly] [PPCG] [1/3] Bump up PPCG to vanilla 0.07

This commit *WILL BREAK*. We are first upgrading to PPCG. We make Polly
specific changes in a separate commit.

This makes upgrading PPCG the next time easier since the changes made to PPCG
for Polly will be clearly documented in a separate commit.

bollu abandoned this revision.Jul 20 2017, 4:29 AM

Hm, perhaps updating this diff is too confusing. I am abandoning this huge change and splitting this into 3 chunks.