This is an archive of the discontinued LLVM Phabricator instance.

Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.
ClosedPublic

Authored by Meinersbur on Sep 14 2018, 12:04 PM.

Details

Summary

Instead of generating llvm.mem.parallel_loop_access metadata, generate llvm.access.group on instructions and llvm.loop.parallel_accesses on loops. There is one access group per generated loop.

This is clang part of D52116.

Diff Detail

Repository
rC Clang

Event Timeline

  • Rebase
  • Use call access group if instruction's access group is not set
Meinersbur updated this revision to Diff 168096.Oct 3 2018, 4:04 AM
  • Upload diff for clang portion (instead of D52116)

I glimpsed over this without spotting anything crucial. My Clang code base knowledge is a bit lightweight though so you might want to wait for an another reviewer. On the other hand, the semantics seem to be retained so it might be safe to commit this in case the tests still pass.

We need to remember to update pocl to produce this format then.

lib/CodeGen/CGLoopInfo.cpp
335

typo 'propert'

test/CodeGenCXX/pragma-loop-safety-nested.cpp
6

Can you add a test case of a nested loop that is not "perfect", that is, has accesses also in the outer loop bodies?

This revision is now accepted and ready to land.Oct 3 2018, 11:20 PM
Meinersbur updated this revision to Diff 168251.Oct 4 2018, 3:08 AM
Meinersbur marked 2 inline comments as done.
hfinkel added inline comments.Dec 3 2018, 4:55 PM
lib/CodeGen/CGLoopInfo.cpp
340

reverse(Active).begin() looks odd. Can we get the same thing by calling last()?

Meinersbur updated this revision to Diff 177320.Dec 7 2018, 2:34 PM
Meinersbur marked an inline comment as done.
  • Allow multiple access groups per instructions, i.e. an instruction can be in multiple access groups. This allows a simple 'union' operation that occurs when inlining into another function. A memory access is considered parallel when at least one access group is listed in llvm.loop.parallel_accesses. This is prioritized over the 'intersect' case for combining instructions which would be dual. We only do best-effort here.
Meinersbur updated this revision to Diff 177324.Dec 7 2018, 2:38 PM
  • Fix wrong patch upload
  • Simplify access group emission ... .. possible due to the added possibility for instructions to belong to multiple access groups in D52116. However, the number of access groups is not minimized anymore.
Meinersbur edited the summary of this revision. (Show Details)Dec 7 2018, 2:40 PM
hfinkel accepted this revision.Dec 18 2018, 5:37 PM

Minor typo noted below, but otherwise, LGTM (to avoid any misunderstanding: this should be committed after the LLVM change lands).

lib/CodeGen/CGLoopInfo.cpp
341

ever -> every

This revision was automatically updated to reflect the committed changes.