This is an archive of the discontinued LLVM Phabricator instance.

[Polly] Allow mapping scalar MemoryAccesses to array elements.
ClosedPublic

Authored by Meinersbur on Aug 27 2016, 10:02 AM.

Details

Summary

Change the code around setNewAccessRelation to allow to use a an existing array element for memory instead of an ad-hoc malloc. This facility will be used for DeLICM/DeGVN to convert scalar dependencies into regular ones.

The changes necessary include:

  • Make the code generator use the implicit locations instead of the alloca ones.
  • A test case
  • Make the JScop importer accept changes of scalar accesses for that test case.
  • Adapt the MemoryAccess interface to the fact that the MemoryKind can change. They are named (get|is)OriginalXXX() to get the status of the memory access before any change by setNewAccessRelation() (some properties such as getIncoming() do not change even if the kind is changed and are still required). To get the modified properties, there is (get|is)LatestXXX(). The old accessors without Original|Latest are synonyms of the (get|is)OriginalXXX() to not make functional changes in unrelated code.

Diff Detail

Repository
rL LLVM

Event Timeline

Meinersbur retitled this revision from to [Polly] Allow mapping scalar MemoryAccesses to array elements..
Meinersbur updated this object.
Meinersbur added a project: Restricted Project.
Meinersbur added subscribers: llvm-commits, pollydev.

Since we can only change scalar accesses to array accesses (not the other way around), instead of the Original/Latest scheme I could also imagine the following accessors:

isArrayTarget()
getTargetScopArrayInfo()
getTargetBaseAddress()
getTargetAccessRelation()

that would always return the latest information, and keep the old ones.

grosser accepted this revision.Aug 27 2016, 12:05 PM
grosser edited edge metadata.

Hi Michael,

very nice patch. Just one minor comment.

Best,
Tobias

test/Isl/CodeGen/MemAccess/map_scalar_access___%outer.for---%return.jscop.transformed
1 ↗(On Diff #69493)

Could you possibly also commit the original .jscop file. I tried to do this consistently to make it easy to see the changes by just calling 'diff' between the original and the transformed jscop file.

This revision is now accepted and ready to land.Aug 27 2016, 12:05 PM

Thank you for the review. Do you have any comment on the get(Original|Latest)XXX naming scheme? It is what I originally used, but it feels clunky. Ideally I'd distinguish between whether the access is explicit (caused by a MemInst)/implicit (cross-Stmt registers+PHIs) and what it accesses (array/scalar alloca).

test/Isl/CodeGen/MemAccess/map_scalar_access___%outer.for---%return.jscop.transformed
1 ↗(On Diff #69493)

OK. Didn't know the purpose of those files.

This revision was automatically updated to reflect the committed changes.