This is an archive of the discontinued LLVM Phabricator instance.

[Polly] Check validity of new access relations. NFC.
ClosedPublic

Authored by Meinersbur on Aug 26 2016, 4:00 AM.

Details

Summary

There are some constraints on maps that can be access relations. In builds with assertions enabled, verify

  • The access domain is the same space as the statement's domain (modulo parameters)
  • Whether an access is defined for every instance of the statement (codegen does not yet support partial access relations)
  • Whether the access range links to an array, represented by a ScopArrayInfo
  • The number of access dimensions equals the dimensions of the array.

Diff Detail

Event Timeline

Meinersbur updated this revision to Diff 69339.Aug 26 2016, 4:00 AM
Meinersbur retitled this revision from to [Polly] Check validity of new access relations. NFC..
Meinersbur updated this object.
Meinersbur added a project: Restricted Project.
Meinersbur added subscribers: llvm-commits, pollydev.

The test Isl/CodeGen/MemAccess/create_arrays.ll fails this with the assertions:

Assertion failed: isl_space_dim(NewAccessSpace, isl_dim_set) == Dims && "Access dims must match array dims"

It creates an array with 3 dimensions ("E") but adds an access with only 2 indices ("E[i2, i0]").

grosser edited edge metadata.Aug 26 2016, 4:36 AM

Hi Michael.

these changes look good to me, but we should probably coordinate with Roman before committing them. Thanks for already reaching out to him.

@Roman: As you have been working in this area, could you please also review this change?

Best,
Tobias

include/polly/Support/ScopHelper.h
451 ↗(On Diff #69339)

This really belongs into isl. We can probably add this here for now, as having a local copy has minimal cost, but we should probably contribute a corresponding patch upstream.

gareevroman edited edge metadata.Aug 26 2016, 6:53 AM

Hi Michael,

thank you for the patch. It looks good also to me.

include/polly/Support/ScopHelper.h
451 ↗(On Diff #69339)

We should possibly rename it to, for example, SpaceIsCompatible, if we want to declare it here.

gareevroman added inline comments.Aug 26 2016, 6:54 AM
include/polly/Support/ScopHelper.h
451 ↗(On Diff #69339)

"spaceIsCompatible"

Meinersbur updated this revision to Diff 69959.Sep 1 2016, 1:51 AM
Meinersbur updated this object.
Meinersbur edited edge metadata.

Use ISL's new isl_space_has_equal_tuples

grosser accepted this revision.Sep 1 2016, 1:54 AM
grosser edited edge metadata.
This revision is now accepted and ready to land.Sep 1 2016, 1:54 AM
Meinersbur updated this revision to Diff 70048.Sep 1 2016, 12:22 PM
Meinersbur edited edge metadata.

rebase; also check for indirect array accesses

This revision was automatically updated to reflect the committed changes.