From 665c2e91e29ddfc2a60746d7d2e6bc60fe5d3e08 Mon Sep 17 00:00:00 2001 From: Sairudra More Date: Wed, 25 Mar 2026 10:23:52 +0530 Subject: [PATCH] [Flang][OpenMP] Remove dead restoreIP in OpenMP taskloop lowering (#187222) This fixes an intermittent crash in `OpenMP` taskloop lowering. In `OMPIRBuilder::createTaskloop`, the `restoreIP` in `PostOutlineCB` was immediately overwritten by the following `Builder.SetInsertPoint(StaleCI)` with no instructions created in between, so it was effectively dead. This patch removes that dead restore, which is the smallest change and preserves the intended IR placement. Adds a regression test that compiles a taskloop to LLVM IR and verifies the bounds casts and __kmpc_taskloop call are present. --- .../OpenMP/taskloop-bounds-cast.f90 | 33 +++++++++++++++++++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 2 -- .../LLVMIR/openmp-taskloop-bounds-cast.mlir | 27 +++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 flang/test/Integration/OpenMP/taskloop-bounds-cast.f90 create mode 100644 mlir/test/Target/LLVMIR/openmp-taskloop-bounds-cast.mlir diff --git a/flang/test/Integration/OpenMP/taskloop-bounds-cast.f90 b/flang/test/Integration/OpenMP/taskloop-bounds-cast.f90 new file mode 100644 index 000000000000..9397b98b9ecd --- /dev/null +++ b/flang/test/Integration/OpenMP/taskloop-bounds-cast.f90 @@ -0,0 +1,33 @@ +!===----------------------------------------------------------------------===! +! This directory can be used to add Integration tests involving multiple +! stages of the compiler (for eg. from Fortran to LLVM IR). It should not +! contain executable tests. We should only add tests here sparingly and only +! if there is no other way to test. Repeat this message in each test that is +! added to this directory and sub-directories. +!===----------------------------------------------------------------------===! + +! Regression test for an intermittent crash in taskloop LLVM IR codegen. +! A dead saveIP/restoreIP pair in OpenMPIRBuilder::createTaskloop introduced +! stale IRBuilder/debug-location state on the restore path before an +! immediately following SetInsertPoint override. Removing the dead restore +! avoids that restore path entirely. + +! RUN: %flang_fc1 -emit-llvm -fopenmp -o - %s | FileCheck %s + +! Verify that the taskloop runtime call is present in the generated LLVM IR +! and that the i64 bounds casts required by the ABI are emitted. + +! CHECK-LABEL: define void @test_taskloop_( + +! CHECK: sext i32 {{.*}} to i64 +! CHECK: call void @__kmpc_taskloop( + +subroutine test_taskloop(n) + integer, intent(in) :: n + integer :: i + + !$omp taskloop + do i = 1, n + end do + !$omp end taskloop +end subroutine test_taskloop diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index d14959379526..9193bf5a0c85 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -2190,7 +2190,6 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTaskloop( /* Create the casting for the Bounds Values that can be used when outlining * to replace the uses of the fakes with real values */ BasicBlock *CodeReplBB = StaleCI->getParent(); - IRBuilderBase::InsertPoint CurrentIp = Builder.saveIP(); Builder.SetInsertPoint(CodeReplBB->getFirstInsertionPt()); Value *CastedLBVal = Builder.CreateIntCast(LBVal, Builder.getInt64Ty(), true, "lb64"); @@ -2198,7 +2197,6 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTaskloop( Builder.CreateIntCast(UBVal, Builder.getInt64Ty(), true, "ub64"); Value *CastedStepVal = Builder.CreateIntCast(StepVal, Builder.getInt64Ty(), true, "step64"); - Builder.restoreIP(CurrentIp); Builder.SetInsertPoint(StaleCI); diff --git a/mlir/test/Target/LLVMIR/openmp-taskloop-bounds-cast.mlir b/mlir/test/Target/LLVMIR/openmp-taskloop-bounds-cast.mlir new file mode 100644 index 000000000000..41121705aa2c --- /dev/null +++ b/mlir/test/Target/LLVMIR/openmp-taskloop-bounds-cast.mlir @@ -0,0 +1,27 @@ +// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s + +// Regression test: a dead saveIP/restoreIP pair in +// OpenMPIRBuilder::createTaskloop PostOutlineCB could introduce stale +// IRBuilder debug-location state and cause an intermittent crash during +// finalize(). Removing the dead pair avoids that restore path entirely. +// Verifies that an i32 loop is lowered correctly with a __kmpc_taskloop call. + +omp.private {type = private} @_QPtest_taskloop_boundsEi_private_i32 : i32 + +llvm.func @_QPtest_taskloop_bounds() { + %0 = llvm.mlir.constant(1 : i64) : i64 + %1 = llvm.alloca %0 x i32 {bindc_name = "i"} : (i64) -> !llvm.ptr + %lb = llvm.mlir.constant(1 : i32) : i32 + %ub = llvm.mlir.constant(10 : i32) : i32 + %step = llvm.mlir.constant(1 : i32) : i32 + omp.taskloop private(@_QPtest_taskloop_boundsEi_private_i32 %1 -> %arg0 : !llvm.ptr) { + omp.loop_nest (%arg1) : i32 = (%lb) to (%ub) inclusive step (%step) { + llvm.store %arg1, %arg0 : i32, !llvm.ptr + omp.yield + } + } + llvm.return +} + +// CHECK-LABEL: define void @_QPtest_taskloop_bounds( +// CHECK: call void @__kmpc_taskloop(