Skip to content

Conversation

@abidh
Copy link
Contributor

@abidh abidh commented Aug 12, 2025

Fixes #153043.

This is another case of debug location not getting updated when the insert point is changed by the restoreIP. Fixed by using the wrapper function that updates the debug location.

Fixes llvm#153043.

This is another case of debug location not getting updated when the
insert point is changed by the restoreIP. Fixed by using the wrapper
function that updates the debug location.
@abidh abidh requested review from TIFitis and skatrak August 12, 2025 13:40
@llvmbot llvmbot added clang Clang issues not falling into any other category flang:openmp clang:openmp OpenMP related changes to Clang labels Aug 12, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 12, 2025

@llvm/pr-subscribers-flang-openmp

Author: Abid Qadeer (abidh)

Changes

Fixes #153043.

This is another case of debug location not getting updated when the insert point is changed by the restoreIP. Fixed by using the wrapper function that updates the debug location.


Full diff: https://github.com/llvm/llvm-project/pull/153190.diff

2 Files Affected:

  • (added) clang/test/OpenMP/amdgcn_debug_nowait.c (+16)
  • (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+1-1)
diff --git a/clang/test/OpenMP/amdgcn_debug_nowait.c b/clang/test/OpenMP/amdgcn_debug_nowait.c
new file mode 100644
index 0000000000000..d691327512ff7
--- /dev/null
+++ b/clang/test/OpenMP/amdgcn_debug_nowait.c
@@ -0,0 +1,16 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -debug-info-kind=line-tables-only -fopenmp -triple x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-host.bc
+
+int test() {
+  int c;
+
+#pragma omp target data map(tofrom: c)
+{
+  #pragma omp target nowait
+  {
+      c = 2;
+  }
+}
+  return c;
+}
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 2ac9a0d3fbd66..c16b0dde1a3da 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -7247,7 +7247,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTargetData(
           BodyGenCB(Builder.saveIP(), BodyGenTy::NoPriv);
       if (!AfterIP)
         return AfterIP.takeError();
-      Builder.restoreIP(*AfterIP);
+      restoreIPandDebugLoc(Builder, *AfterIP);
 
       if (IfCond)
         return emitIfClause(IfCond, EndThenGen, EndElseGen, AllocaIP);

@llvmbot
Copy link
Member

llvmbot commented Aug 12, 2025

@llvm/pr-subscribers-clang

Author: Abid Qadeer (abidh)

Changes

Fixes #153043.

This is another case of debug location not getting updated when the insert point is changed by the restoreIP. Fixed by using the wrapper function that updates the debug location.


Full diff: https://github.com/llvm/llvm-project/pull/153190.diff

2 Files Affected:

  • (added) clang/test/OpenMP/amdgcn_debug_nowait.c (+16)
  • (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+1-1)
diff --git a/clang/test/OpenMP/amdgcn_debug_nowait.c b/clang/test/OpenMP/amdgcn_debug_nowait.c
new file mode 100644
index 0000000000000..d691327512ff7
--- /dev/null
+++ b/clang/test/OpenMP/amdgcn_debug_nowait.c
@@ -0,0 +1,16 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -debug-info-kind=line-tables-only -fopenmp -triple x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-host.bc
+
+int test() {
+  int c;
+
+#pragma omp target data map(tofrom: c)
+{
+  #pragma omp target nowait
+  {
+      c = 2;
+  }
+}
+  return c;
+}
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 2ac9a0d3fbd66..c16b0dde1a3da 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -7247,7 +7247,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTargetData(
           BodyGenCB(Builder.saveIP(), BodyGenTy::NoPriv);
       if (!AfterIP)
         return AfterIP.takeError();
-      Builder.restoreIP(*AfterIP);
+      restoreIPandDebugLoc(Builder, *AfterIP);
 
       if (IfCond)
         return emitIfClause(IfCond, EndThenGen, EndElseGen, AllocaIP);

Copy link
Member

@TIFitis TIFitis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Nit: Is there any existing test file we can add this to instead of creating a new one? Otherwise I'm okay.

@skatrak
Copy link
Member

skatrak commented Aug 12, 2025

Should we replace all restoreIP calls in the OMPIRBuilder with restoreIPandDebugLoc or is that only done in specific situations?

@abidh
Copy link
Contributor Author

abidh commented Aug 12, 2025

Should we replace all restoreIP calls in the OMPIRBuilder with restoreIPandDebugLoc or is that only done in specific situations?

Currently I have been doing it in selective cases. But I think we should do the change en-masse to better future proof us. I will work on it after this PR. Please note that there are 2 different code patterns we need to handle (More details in the description of the #151306). All the occurrences of the 2nd pattern need this change.

@abidh
Copy link
Contributor Author

abidh commented Aug 12, 2025

LGTM.

Nit: Is there any existing test file we can add this to instead of creating a new one? Otherwise I'm okay.

Thanks for the prompt review. I looked at the existing testcases and did not see any that fits this exact pattern.

@abidh abidh merged commit 62d0b71 into llvm:main Aug 12, 2025
13 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 12, 2025

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building clang,llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/22483

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: lang/c/local_variables/TestLocalVariables.py (755 of 2305)
PASS: lldb-api :: lang/c/offsetof/TestOffsetof.py (756 of 2305)
PASS: lldb-api :: lang/c/non-mangled/TestCNonMangled.py (757 of 2305)
XFAIL: lldb-api :: lang/c/modules/TestCModules.py (758 of 2305)
PASS: lldb-api :: lang/c/record_decl_in_expr/TestRecordDeclInExpr.py (759 of 2305)
PASS: lldb-api :: lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py (760 of 2305)
PASS: lldb-api :: lang/c/register_variables/TestRegisterVariables.py (761 of 2305)
PASS: lldb-api :: lang/c/sizeof/TestCSizeof.py (762 of 2305)
PASS: lldb-api :: lang/c/set_values/TestSetValues.py (763 of 2305)
UNRESOLVED: lldb-api :: functionalities/statusline/TestStatusline.py (764 of 2305)
******************** TEST 'lldb-api :: functionalities/statusline/TestStatusline.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --arch aarch64 --build-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --cmake-build-type Release /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/functionalities/statusline -p TestStatusline.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision 62d0b712b76b2f6c8e734cd75be50120ff465f24)
  clang revision 62d0b712b76b2f6c8e734cd75be50120ff465f24
  llvm revision 62d0b712b76b2f6c8e734cd75be50120ff465f24
Skipping the following test categories: ['libc++', 'msvcstl', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test (TestStatusline.TestStatusline)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_deadlock (TestStatusline.TestStatusline)
lldb-server exiting...
FAIL: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_modulelist_deadlock (TestStatusline.TestStatusline)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_no_color (TestStatusline.TestStatusline)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_no_target (TestStatusline.TestStatusline)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_resize (TestStatusline.TestStatusline)
======================================================================
ERROR: test_modulelist_deadlock (TestStatusline.TestStatusline)
   Regression test for a deadlock that occurs when the status line is enabled before connecting
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/functionalities/statusline/TestStatusline.py", line 199, in test_modulelist_deadlock
    self.expect(
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbpexpect.py", line 95, in expect
    self.expect_prompt()
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbpexpect.py", line 19, in expect_prompt
    self.child.expect_exact(self.PROMPT)
  File "/usr/local/lib/python3.10/dist-packages/pexpect/spawnbase.py", line 432, in expect_exact
    return exp.expect_loop(timeout)
  File "/usr/local/lib/python3.10/dist-packages/pexpect/expect.py", line 179, in expect_loop
    return self.eof(e)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category flang:openmp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[OMPIRBuilder][clang] Verification failure with nowait on target construct and -g.

5 participants