Hi all, I made some progress on implementing Hal's proposal [1] for implementing OpenMP support in LLVM. The patch I've attached just barely compiles, but I'd like to get some input on the design early on to prevent grief later. I'd especially like some input on the following points: * Not dropping any metadata I think it is better to have an analysis pass that provides a consistent view of the current !parallel nodes. By addRequired<> ing it (called ParallizationMetadata currently) in the lowering pass and only indirectly accessing the metadata through the analysis pass, we can assure ourselves that we don't lower unsafe regions. * No information is optional It simplifies the implementation greatly if we assume that things like task affinity etc. aren't optional. I don't think this restriction shifts any significant complexity to the frontends. * Loops don't have lists of special handling regions Since loops are always nested inside parallel regions, can't we maintain this list in the parent region itself? * Blurry line between asserting and silently ignoring I'm not very clear on when it is "right" to silently drop metadata and when to assert. Right now I assert only when a !parallel MDNode has an incorrect number of children (this is one thing simplified by making all fields compulsory). The verifier probably needs to know about this constraint, something I haven't addressed yet. Should our assertions be stricter or must we not assert at all? (The code is also up on Github [2]). Thanks! [1] http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-August/052472.html [2] https://github.com/sanjoy/llvm/tree/parallel-md -- Sanjoy Das http://playingwithpointers.com -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-Create-a-class-structure-represent-the-various-kinds.patch Type: application/octet-stream Size: 25207 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120910/c0c3e063/attachment.obj>
Hi all, I made some progress on implementing Hal's proposal [1] for implementing OpenMP support in LLVM. The patch I've attached just barely compiles, but I'd like to get some input on the design early on to prevent trouble later. I'd especially like some input on the following points: * Metadata is never mutated or dropped I think it is better to have an analysis pass that simply provides a consistent "view" of the current !parallel nodes instead of one that mutates the IR in order to make it consistent. By addRequired<> ing it (called ParallizationMetadata currently) in the lowering pass and only indirectly accessing the metadata through the analysis pass, we are assured that we don't parallelize regions with inconsistent metadata. * No information is optional It simplifies the implementation greatly if we change the spec to assume this. I don't think this restriction shifts any significant complexity to the frontends. * Loops don't have lists of special handling regions Since loops are always nested inside parallel regions, can't we maintain this list in the parent region itself? * Tripping assertions vs. silently ignoring metadata I'm not very clear on when it is "right" to silently ignore metadata and when to assert. Right now I assert only when a !parallel MDNode has an incorrect number of children (this is one thing simplified by making all fields compulsory). The verifier probably needs to know about this constraint, something I haven't addressed yet. Should our assertions be stricter or must we not assert at all? (The code is also up on Github [2]). Thanks! [1] http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-August/052472.html [2] https://github.com/sanjoy/llvm/tree/parallel-md -- Sanjoy Das http://playingwithpointers.com -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-Create-a-class-structure-represent-the-various-kinds.patch Type: application/octet-stream Size: 25207 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120911/ed5ee30a/attachment.obj>
On Tue, 11 Sep 2012 01:21:59 +0530 Sanjoy Das <sanjoy at playingwithpointers.com> wrote: Sanjoy, Thanks for working on this! Comments below...> Hi all, > > I made some progress on implementing Hal's proposal [1] for > implementing OpenMP support in LLVM. The patch I've attached just > barely compiles, but I'd like to get some input on the design early on > to prevent trouble later. I'd especially like some input on the > following points: > > * Metadata is never mutated or dropped > > I think it is better to have an analysis pass that simply provides a > consistent "view" of the current !parallel nodes instead of one that > mutates the IR in order to make it consistent. By addRequired<> ing > it (called ParallizationMetadata currently) in the lowering pass and > only indirectly accessing the metadata through the analysis pass, we > are assured that we don't parallelize regions with inconsistent > metadata.My rationale for proposing the self-consistent metadata solution was that it seemed to be the safest option. If we simply insist that all relevant passes use the ParallizationMetadata pass, without any verification, then we could end up with parallelization-unaware passes silently miscompiling code when parallelization is enabled. Do you have a way of avoiding that?> > * No information is optional > > It simplifies the implementation greatly if we change the spec to > assume this. I don't think this restriction shifts any significant > complexity to the frontends.Okay.> > * Loops don't have lists of special handling regions > > Since loops are always nested inside parallel regions, can't we > maintain this list in the parent region itself?I'm not sure. We do need to make sure, for example, that an 'ordered' region stays properly nested within its parent loop.> > * Tripping assertions vs. silently ignoring metadata > > I'm not very clear on when it is "right" to silently ignore metadata > and when to assert. Right now I assert only when a !parallel MDNode > has an incorrect number of children (this is one thing simplified by > making all fields compulsory). The verifier probably needs to know > about this constraint, something I haven't addressed yet. Should our > assertions be stricter or must we not assert at all?We should assert when an individual metadata entry is malformed; we should drop non-self-consistent sets of metadata entries (as these might have resulted from the actions of non-parallelization-aware transformation passes. -Hal> > (The code is also up on Github [2]). > > Thanks! > > [1] http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-August/052472.html > [2] https://github.com/sanjoy/llvm/tree/parallel-md > > > -- > Sanjoy Das > http://playingwithpointers.com-- Hal Finkel Postdoctoral Appointee Leadership Computing Facility Argonne National Laboratory
On 09/10/2012 10:51 PM, Sanjoy Das wrote:> I made some progress on implementing Hal's proposal [1] for > implementing OpenMP support in LLVM.I can generate parallel loops from OpenCL multi-WI work groups in pocl kernel compilation now. Please let me know when this work is in a state I could test it for OpenCL. I need the ability to mark the parallel loops and some pass that actually does something with the parallelism info. Thanks! -- Pekka