[PATCH] D131272: [Clang][OpenMP] Fix the issue that `llvm.lifetime.end` is emitted too early for variables captured in linear clause

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
Herald added subscribers: pengfei, guansong, yaxunl.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Currently if an OpenMP program uses `linear` clause, and is compiled with
optimization, `llvm.lifetime.end` for variables listed in `linear` clause are
emitted too early such that there could still be uses after that. Let's take the
following code as example:

  c
  // loop.c
  int j;
  int *u;
  
  void loop(int n) {
    int i;
    for (i = 0; i < n; ++i) {
      ++j;
      u = &j;
    }
  }

We compile using the command:

  shell
  clang -cc1 -fopenmp-simd -O3 -x c -triple x86_64-apple-darwin10 -emit-llvm loop.c -o loop.ll

The following IR (simplified) will be generated:

  llvm
  @j = local_unnamed_addr global i32 0, align 4
  @u = local_unnamed_addr global ptr null, align 8
  
  define void @loop(i32 noundef %n) local_unnamed_addr {
  entry:
    %j = alloca i32, align 4
    %cmp = icmp sgt i32 %n, 0
    br i1 %cmp, label %simd.if.then, label %simd.if.end
  
  simd.if.then:                                     ; preds = %entry
    call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %j)
    store ptr %j, ptr @u, align 8
    call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)
    %0 = load i32, ptr %j, align 4
    store i32 %0, ptr @j, align 4
    br label %simd.if.end
  
  simd.if.end:                                      ; preds = %simd.if.then, %entry
    ret void
  }

The most important part is:

  llvm
    call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)
    %0 = load i32, ptr %j, align 4
    store i32 %0, ptr @j, align 4

`%j` is still loaded after `@llvm.lifetime.end.p0(i64 4, ptr nonnull %j)`. This
could cause the backend incorrectly optimizes the code and further generates
incorrect code. The root cause is, when we emit a construct that could have
`linear` clause, it usually has the following pattern:

  cpp
  EmitOMPLinearClauseInit(S)
  {
    OMPPrivateScope LoopScope(*this);
    ...
    EmitOMPLinearClause(S, LoopScope);
    ...
    (void)LoopScope.Privatize();
    ...
  }
  EmitOMPLinearClauseFinal(S, [](CodeGenFunction &) { return nullptr; });

Variables that need to be privatized are added into `LoopScope`, which also
serves as a RAII object. When `LoopScope` is destructed and if optimization is
enabled, a `@llvm.lifetime.end` is also emitted for each privatized variable.
However, the writing back to original variables in `linear` clause happens after
the scope in `EmitOMPLinearClauseFinal`, causing the issue we see above.

A quick "fix" seems to be, moving `EmitOMPLinearClauseFinal` inside the scope.
However, it doesn't work. That's because the local variable map has been updated
by `LoopScope` such that a variable declaration is mapped to the privatized
variable, instead of the actual one. In that way, the following code will be
generated:

  llvm
    %0 = load i32, ptr %j, align 4
    store i32 %0, ptr %j, align 4
    call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %j)

Well, now the life time is correct, but apparently the writing back is broken.

In this patch, a new function `OMPPrivateScope::restoreMap` is added and called
before calling `EmitOMPLinearClauseFinal`. This can make sure that
`EmitOMPLinearClauseFinal` can find the orignal varaibls to write back.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131272

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/bug56913.c
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_lastprivate_codegen.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131272.450324.patch
Type: text/x-patch
Size: 8157 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220805/6906cb61/attachment.bin>