This is an archive of the discontinued LLVM Phabricator instance.

Add support for annotated minimum dependence distance
AbandonedPublic

Authored by ArchDRobison on Aug 29 2014, 8:49 AM.

Details

Summary

This patch enables producers of LLVM IR to tell the LoopVectorizer (and other tools) that there is a minimum dependence distance, such as with the OpenMP 4.0 "safelen" clause. See email chain http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-August/075701.html for discussion. The updates to LangRef explain the final design.

Diff Detail

Event Timeline

ArchDRobison retitled this revision from to Add support for annotated minimum dependence distance.
ArchDRobison updated this object.
ArchDRobison edited the test plan for this revision. (Show Details)
ArchDRobison added a subscriber: Unknown Object (MLST).

Can I enlist one of you to review this patch? I've updated to adjust for Renato's recent updates to hint processing..

Generally speaking, I think this is fine. You definitely need to run this through clang-format (which you can do directly on the patch file, IIRC), or manually fix the formatting to comply with the LLVM coding conventions.

However, if the main use case for this is dealing with OpenMP's safelen (and possibly other similar user-provided safety assertions), then I would like to get a resolution on the lexical dependency question (i.e. whether we really also need some ordering construct here).

Also, for future reference, please upload full-context diffs (see http://llvm.org/docs/Phabricator.html for instructions).

Update adjusts patch for this morning's LLVM trunk, and was cleaned up with clang-format-diff.py.

rengolin edited edge metadata.Mar 10 2015, 4:39 AM

Any progress in this? If not, can we close this review?

I believe that we had discussed this in a different thread, and decided that we needed to record a partial ordering in this metadata. It looks like a new patch has not yet been developed.

I believe that support for forward lexical dependences was the item that required partial ordering support, but was orthogonal to this patch, which addresses minimum dependence distance.

  • Arch

I believe that support for forward lexical dependences was the item that required partial ordering support, but was orthogonal to this patch, which addresses minimum dependence distance.

I was not under the impression that it is orthogonal (or it is a prerequisite). Right now, we only have a way to specify that a loop is completely data parallel (no loop carried dependencies at all), so it does not really matter. It does matter, however, if we have a minimum dependency distance, because that distance can have a validity dependence on lexical dependencies (forward and backward) that are sensitive to potential reordering.

Also, as a separate matter, the unroller, when it does partial unrolling, needs to update this metadata (to divide the distances by the unrolling factor).

ArchDRobison abandoned this revision.Mar 11 2015, 9:02 AM

Thanks for the reminder about the reordering issue. I'll abandon this patch.