[PATCH] D31417: [OpenMP] Add support for omp simd pragmas without runtime

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 11 08:40:08 PDT 2017


ABataev added inline comments.


================
Comment at: lib/Parse/ParseOpenMP.cpp:174
+      case OMPD_target_teams_distribute_simd:
+        DKind = OMPD_simd;
+        break;
----------------
huntergr wrote:
> ABataev wrote:
> > huntergr wrote:
> > > rengolin wrote:
> > > > I'd like @ABataev to confirm this is the right semantics.
> > > Yes, would be good. I don't think there's a formal spec for this feature, but it's possible that directives intended for a different target than the cpu shouldn't apply with this flag.
> > I don't think you need it here. Instead, you should keep an existing logic in Sema/Parsing code, but you need to create your own OpenMPRuntime support class, that supports only emission of simd part of the constructs.
> Thanks for taking a look.
> 
> I tried this method first, but came to the conclusion it was too invasive. I've now tried it a second time and came to the same conclusion.
> 
> The problem isn't in generating 'simd' or 'declare simd' from a custom runtime class, but in generating everything else. Take a 'parallel for' directive -- I have to generate something close to unmodified code, but 'emitCommonOMPParallelDirective' in CGStmtOpenMP.cpp will only pass the loop codegen lambda through to the runtime class's 'emitParallelOutlinedFunction', which doesn't take the current CodeGenFunction as an argument (as it's expected to create a new one). There doesn't seem to be a good way to look up the CGF that the call will be made from.
> 
> You also won't get unmodified code in that instance anyway, as the loop is generated by CodeGenFunction::EmitWorkSharingLoop, and a cancellation region may be created; very little of this is under the control of the runtime class, so it would need extensive modification to work.
> 
> So I'll switch to using a simd-only 'runtime' class, but I still think we need to filter out non-simd directives before we get to codegen. Will post updated patch soon.
1. I don't think it is a good idea to skip the checks for non-simd constructs in this new mode. I'd prefer to emit all diagnostics as usual. Thus, we can't simply translate all simd-related constructs into 'omp simd' only, as we're going to loose checks/diagnostics.
Keep in minŠ² that you will need to update all diagnostics-specific tests to check that the same errors/warnings are emitted even in `-fopenmp-simd` mode.
2. You should not rewrite everything. You can do something like this:
file lib/CodeGen/CGStmt.cpp, function `void CodeGenFunction::EmitStmt(const Stmt *S)`
Insert before `switch (S->getStmtClass())`:
```
if (LangOpts.OpenMPSimd && isa<OMPExecutableDirective>(S)) {
  if (isOpenMPSimdDirective(S))
    EmitOMPSimdDirective(S);
  else
    EmitOMPInlinedDirective(S);
  return;
}
```
`EmitOMPSimdDirective(S)` shall emit the simd-specific code for all simd-related directives, while `EmitOMPInlinedDirective(S)` shall emit the code for all other constructs, ingnoring OpenMP directives.


https://reviews.llvm.org/D31417





More information about the cfe-commits mailing list