[flang-commits] [flang] [llvm] [flang] Remove dead restoreIP in OpenMP taskloop lowering (PR #187222)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Sairudra More (Saieiei)

<details>
<summary>Changes</summary>

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.

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


2 Files Affected:

- (added) flang/test/Lower/OpenMP/taskloop-bounds-cast.f90 (+26) 
- (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (-2) 


``````````diff
diff --git a/flang/test/Lower/OpenMP/taskloop-bounds-cast.f90 b/flang/test/Lower/OpenMP/taskloop-bounds-cast.f90
new file mode 100644
index 0000000000000..9a09188bfaa2f
--- /dev/null
+++ b/flang/test/Lower/OpenMP/taskloop-bounds-cast.f90
@@ -0,0 +1,26 @@
+! Test that taskloop LLVM IR codegen does not crash when generating the
+! bounds casts in the PostOutlineCB.  This is a regression test for a
+! use-after-free triggered by a dead saveIP/restoreIP pair in
+! OpenMPIRBuilder::createTaskloop that manipulated the IRBuilder state
+! unnecessarily before an immediately following SetInsertPoint override.
+
+! RUN: %flang_fc1 -emit-llvm -fopenmp -o /dev/null %s
+! 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 2a0a017fbb0f3..2e73c1791992f 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2185,7 +2185,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");
@@ -2193,7 +2192,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);
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/187222