Page MenuHomePhabricator

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

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



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

Diff Detail


Event Timeline

bollu created this revision.Jul 24 2017, 5:23 AM
bollu added inline comments.Jul 24 2017, 5:25 AM
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
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.
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 ?

2803 ↗(On Diff #107887)

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

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

Otherwise. this LGTM.

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
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.