nicebert wrote: > before commenting on details, I'd like to ask a few clarifying questions > because I'm a bit confused what's going on here. > > Afaiu, no-loop is essentially a loop where we can size the grid such that > there is a 1:1 mapping between threads and loop iterations such that no > thread has to actually loop. Basically, we need to do two things to make that > happen: adapt the index computation and the grid size selection.*
Roughly, with a few clarifications. No-loop isn't necessarily a 1:1 mapping between threads and loop iterations. The grid covers the whole iteration space, which means threads past the trip count just don't run the body. In addition to the two things you state, we need a third to emit a loop-free kernel. > The index computation will look like `idx = blockIdx * blockDim + threadIdx` > (if idx < #elements). I wonder: that should be the exact same idx computation > that is also used for big-jump loop, no? Aka, isn't no-loop just a variant of > big-jump loop? The index computation is the same, yes. It exists twice in the device runtime, once behind the fused schedule, which I think is why you're referring to big-jump loop, and once behind the distribute-for entry point that Flang uses, and this PR goes through the second one. I wouldn't say no-loop is a variant of big-jump loop, they're both worksharing loops that use the same method for index computation. > Regarding the grid size selection: afaiu, we key it using the no-loop exec > mode. But is that really necessary? Can't we just say for _every_ loop: hey, > if the grid size allows us to do the 1:1 mapping between threads and > iterations (or if we're allowed to embiggen the grid size such that this > becomes possible), then let's do it since it's unconditionally beneficial. > (Although the "unconditionally beneficial" part might be a question, not > something I know or have measured at this point.) The "if the grid size allows" part is the problem: at codegen time we don't know the grid size and we're not free to choose it. Teams and threads can be set through the environment, so a smaller grid than the iteration space is legal. If we set the flag anyway, every thread returns after one iteration and everything the grid doesn't cover is never executed. That's why no-loop is gated behind the user asserting that the grid will be big enough to cover the whole iteration space. > *From this POV, I wonder why this PR does so much work? If I understand > correctly, the goal is to converge clang and OMPIRBuilder/flang? But then, > there should probably be an appropriate refactoring, if needed, because the > work duplication for no-loop in `clang/lib/CodeGen/CGStmtOpenMP.cpp` seems > very error-prone to me, tbh. Naively speaking, I don't really see why we need > that much just to have a different index computation. Index computation as well as grid size selection are wiring rather than new code: setting the execution mode on the kernel and using OpenMPIRBuilder for the worksharing call with the appropriate flag, reusing the code that already exists for Flang. The volume comes from the clause handling that the loop would otherwise carry. That's the same thing you spotted on the early return: we take the IRBuilder worksharing call instead of clang's classic worksharing loop codegen, and that codegen is also what handles the clauses, so the no-loop branch privatizes the private variables and the loop counters that need it, and does the lastprivate initialization and the final copy itself. For the omission of the outer distribute loop I reuse the flattened team and thread iteration space your fused distribute schedule already sets up. Flang needs the same handling but does it while lowering to MLIR, so by the time it reaches OMPIRBuilder the privatization is already expressed in the dialect and there's nothing left to do at the call. Doing it that way in clang would mean CIR, and OpenMP there is still not yet implemented for the directives this PR covers, target teams distribute parallel for included, so for now it has to happen in CodeGen. On the refactoring, do you mean the clause handling should go through the same lowering flang uses? That's the CIR answer above, and I don't think it's reachable today. If you mean something narrower that could be done in the current structure, could you point at the part of the diff you'd restructure? https://github.com/llvm/llvm-project/pull/205325 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
