Page MenuHomePhabricator

[Polly] [PPCGCodeGeneration] Skip arrays with empty extent.
ClosedPublic

Authored by bollu on Jul 24 2017, 5:23 AM.

Details

Summary

Invariant load hoisted scalars, and arrays whose size we can
statically compute to be 0 do not need to be allocated as arrays.

Invariant load hoisted scalars are sent to the kernel directly as
parameters. Earlier, we used to allocate 0 bytes of memory for these.
Now, since we don't track them as arrays, this problem does not occur
anymore.

Diff Detail

Repository
rL LLVM

Event Timeline

bollu created this revision.Jul 24 2017, 5:23 AM
bollu added inline comments.Jul 24 2017, 5:25 AM
test/GPGPU/invariant-load-of-scalar.ll
39 ↗(On Diff #107887)

I test it this way because checking for the entire line is too long. Is there some way to split a test into multiple lines? the FileCheck page on LLVM doesn't indicate any.

The actual test would be:

@FUNC_checkPrivatization_SCOP_0_KERNEL_0(i8 addrspace(1)* %MemRef_A, i32 %tmp, i32 %tmp2, i32 %polly.access.begin.load, i32 %polly.access.end.load)
bollu added inline comments.Jul 24 2017, 5:45 AM
lib/CodeGen/PPCGCodeGeneration.cpp
2808 ↗(On Diff #107887)

Not sure if make_filter_range makes the code cleaner or not.

2808 ↗(On Diff #107887)

I wanted to take a const ScopArrayInfo *SAI (because it's a filter), but unfortunately getExtent(ScopArrayInfo *Array) is not const, even though it could be.

@grosser - may I change it to const in a later NFC patch?

philip.pfaffe added inline comments.
test/GPGPU/invariant-load-of-scalar.ll
39 ↗(On Diff #107887)

FileCheck has CHECK-SAME for checking mulitple things on the same line.

singam-sanjay edited edge metadata.Jul 24 2017, 7:34 AM

How are invariant loads to pointers turned into scalar accesses ? Do we need -polly-invariant-load-hoisting for that ?

lib/CodeGen/PPCGCodeGeneration.cpp
2803 ↗(On Diff #107887)

Nitpick:
Otherwise, this will cause Polly to consider the following kinds of
(Or)
Otherwise, the following kinds of empty arrays would be considered:

grosser accepted this revision.Jul 24 2017, 8:19 AM

Otherwise. this LGTM.

test/GPGPU/invariant-load-of-scalar.ll
35 ↗(On Diff #107887)

Rather then checking for the absence of a very specific string, I would check that allocateMemoryForDevice is called exactly X times. This is more robust, instead of array order or names change.

39 ↗(On Diff #107887)

Just break the line with "\" at the end of the line (or make it more than 80 characters)

This revision is now accepted and ready to land.Jul 24 2017, 8:19 AM

@singam-sanjay: Yep, all of this needs invariant load hoisting.

bollu updated this revision to Diff 108016.Jul 25 2017, 1:22 AM
  • [NFC wrt patch] refine test case to be more readable and less breakable
grosser added inline comments.Jul 25 2017, 2:19 AM
test/GPGPU/invariant-load-of-scalar.ll
35 ↗(On Diff #107887)

I suggest to just check for:

HOST-IR-NOT: polly_allocateMemoryForDevice

We do not want ANY other memory location. This is more robust than checking for specific names.

bollu updated this revision to Diff 108047.Jul 25 2017, 4:29 AM
  • [NFC wrt patch] make CHECK-NOT more general

Very cool. I think this one is now good to

This revision was automatically updated to reflect the committed changes.