[PATCH] D109321: [clang][OpenMP] Fix the bug in codegen for ordered directive

[Peixin Qiao via Phabricator via cfe-commits]

peixin added a comment.

In D109321#2987783 <https://reviews.llvm.org/D109321#2987783>, @jdoerfert wrote:

> As noted before, this is not the right fix. Not inlining should not cause a semantic difference here, the problem is something else.

@jdoerfert Yes, I agree that this is not the right fix. What I mean is to abandon this PR and post this bug in bugzilla to let someone who knows more about optimization passes to solve it considering this may not be one frontend bug according to the result of the command `clang++ -fopenmp simd.cpp -O1 -Xclang -disable-llvm-passes`. I have to admit that always inlining the outlined function gives correct results since optmization passes vectorize the statements if there is no dependence (eg. a[i] = 1.0) and add memcheck if there may be dependence (eg. a[i] = b[i] + 1.0). Although this vectorization violate the definition of `ordered` construct in OpenMP 5.0 standard, it's safe to do it. Anyway, I think it's not one big problem here.

> Just to prevent the IR from changing when optimizations aren't enabled.

@jhuber6  Thanks for the reply. I think not generating outlined function is the right way when there is no optimization. Using the code for `ordered threads` like the following should be ok. This should not be one big deal since few users use `O0` to run their applications. Do you think if we should make this change?

  diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
  index 6ede4c3189d4..0ca5c7b71084 100644
  --- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
  +++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
  @@ -5443,11 +5443,12 @@ void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {
         CGM.getOpenMPRuntime().emitDoacrossOrdered(*this, DC);
       return;
     }
  +  unsigned OptLevel = CGM.getCodeGenOpts().OptimizationLevel;
     const auto *C = S.getSingleClause<OMPSIMDClause>();
  -  auto &&CodeGen = [&S, C, this](CodeGenFunction &CGF,
  -                                 PrePostActionTy &Action) {
  +  auto &&CodeGen = [&S, OptLevel, C, this](CodeGenFunction &CGF,
  +                                           PrePostActionTy &Action) {
       const CapturedStmt *CS = S.getInnermostCapturedStmt();
  -    if (C) {
  +    if (C && OptLevel > 0) {
         llvm::SmallVector<llvm::Value *, 16> CapturedVars;
         CGF.GenerateOpenMPCapturedVars(*CS, CapturedVars);
         llvm::Function *OutlinedFn =


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109321/new/

https://reviews.llvm.org/D109321