Skip to content

[flang] Fixed fir.dummy_scope generation to work for TBAA. #136382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 19, 2025

Conversation

vzakhari
Copy link
Contributor

The nesting of fir.dummy_scope operations defines the roots
of the TBAA forest. If we do not generate fir.dummy_scope
in functions that do not have any dummy arguments, then
the globals accessed in the function and the dummy arguments
accessed by the callee may end up in different sub-trees
of the same root. The added tbaa-with-dummy-scope2.fir
demonstrates the issue.

@llvmbot llvmbot added flang:driver flang Flang issues not falling into any other category flang:fir-hlfir labels Apr 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2025

@llvm/pr-subscribers-flang-openmp
@llvm/pr-subscribers-flang-driver

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

Author: Slava Zakharin (vzakhari)

Changes

The nesting of fir.dummy_scope operations defines the roots
of the TBAA forest. If we do not generate fir.dummy_scope
in functions that do not have any dummy arguments, then
the globals accessed in the function and the dummy arguments
accessed by the callee may end up in different sub-trees
of the same root. The added tbaa-with-dummy-scope2.fir
demonstrates the issue.


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

6 Files Affected:

  • (modified) flang/lib/Lower/Bridge.cpp (+4-1)
  • (modified) flang/test/Driver/emit-mlir.f90 (+1)
  • (modified) flang/test/Lower/HLFIR/calls-f77.f90 (+1)
  • (added) flang/test/Lower/HLFIR/dummy-scope.f90 (+52)
  • (modified) flang/test/Lower/main_location.f90 (+2)
  • (added) flang/test/Transforms/tbaa-with-dummy-scope2.fir (+106)
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index b4d1197822a43..1652a86ed7e63 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -5508,7 +5508,10 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     for (const Fortran::lower::CalleeInterface::PassedEntity &arg :
          callee.getPassedArguments())
       mapPassedEntity(arg);
-    if (lowerToHighLevelFIR() && !callee.getPassedArguments().empty()) {
+
+    // Always generate fir.dummy_scope even if there are no arguments.
+    // It is currently used to create proper TBAA forest.
+    if (lowerToHighLevelFIR()) {
       mlir::Value scopeOp = builder->create<fir::DummyScopeOp>(toLocation());
       setDummyArgsScope(scopeOp);
     }
diff --git a/flang/test/Driver/emit-mlir.f90 b/flang/test/Driver/emit-mlir.f90
index 2345a7cf53e40..de5a62d6bc7f3 100644
--- a/flang/test/Driver/emit-mlir.f90
+++ b/flang/test/Driver/emit-mlir.f90
@@ -13,6 +13,7 @@
 ! CHECK-SAME: dlti.dl_spec =
 ! CHECK-SAME: llvm.data_layout =
 ! CHECK-LABEL: func @_QQmain() {
+! CHECK-NEXT:  fir.dummy_scope
 ! CHECK-NEXT:  return
 ! CHECK-NEXT: }
 ! CHECK-NEXT: func.func private @_FortranAProgramStart(i32, !llvm.ptr, !llvm.ptr, !llvm.ptr)
diff --git a/flang/test/Lower/HLFIR/calls-f77.f90 b/flang/test/Lower/HLFIR/calls-f77.f90
index 64b90b83b8280..450f8811eb5e0 100644
--- a/flang/test/Lower/HLFIR/calls-f77.f90
+++ b/flang/test/Lower/HLFIR/calls-f77.f90
@@ -9,6 +9,7 @@ subroutine call_no_arg()
   call void()
 end subroutine
 ! CHECK-LABEL: func.func @_QPcall_no_arg() {
+! CHECK-NEXT:  fir.dummy_scope
 ! CHECK-NEXT:  fir.call @_QPvoid() fastmath<contract> : () -> ()
 ! CHECK-NEXT:  return
 
diff --git a/flang/test/Lower/HLFIR/dummy-scope.f90 b/flang/test/Lower/HLFIR/dummy-scope.f90
new file mode 100644
index 0000000000000..4b1a3324c0777
--- /dev/null
+++ b/flang/test/Lower/HLFIR/dummy-scope.f90
@@ -0,0 +1,52 @@
+! RUN: bbc -emit-hlfir %s -o - | FileCheck %s
+
+! Test fir.dummy_scope assignment to argument x
+subroutine sub_arg(x)
+  integer :: x
+end subroutine sub_arg
+! CHECK-LABEL:   func.func @_QPsub_arg(
+! CHECK-SAME:                          %[[VAL_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !fir.ref<i32> {fir.bindc_name = "x"}) {
+! CHECK:           %[[VAL_1:.*]] = fir.dummy_scope : !fir.dscope
+! CHECK:           %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %[[VAL_1]] {uniq_name = "_QFsub_argEx"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:           return
+! CHECK:         }
+
+! Test fir.dummy_scope is created even when there are no arguments.
+subroutine sub_noarg
+end subroutine sub_noarg
+! CHECK-LABEL:   func.func @_QPsub_noarg() {
+! CHECK:           %[[VAL_0:.*]] = fir.dummy_scope : !fir.dscope
+! CHECK:           return
+! CHECK:         }
+
+! Test fir.dummy_scope assignment to argument x
+function func_arg(x)
+  integer :: x, func_arg
+  func_arg = x
+end function func_arg
+! CHECK-LABEL:   func.func @_QPfunc_arg(
+! CHECK-SAME:                           %[[VAL_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !fir.ref<i32> {fir.bindc_name = "x"}) -> i32 {
+! CHECK:           %[[VAL_1:.*]] = fir.dummy_scope : !fir.dscope
+! CHECK:           %[[VAL_2:.*]] = fir.alloca i32 {bindc_name = "func_arg", uniq_name = "_QFfunc_argEfunc_arg"}
+! CHECK:           %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_2]] {uniq_name = "_QFfunc_argEfunc_arg"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:           %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %[[VAL_1]] {uniq_name = "_QFfunc_argEx"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:           %[[VAL_5:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i32>
+! CHECK:           hlfir.assign %[[VAL_5]] to %[[VAL_3]]#0 : i32, !fir.ref<i32>
+! CHECK:           %[[VAL_6:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i32>
+! CHECK:           return %[[VAL_6]] : i32
+! CHECK:         }
+
+! Test fir.dummy_scope is created even when there are no arguments.
+function func_noarg
+  integer :: func_noarg
+  func_noarg = 1
+end function func_noarg
+! CHECK-LABEL:   func.func @_QPfunc_noarg() -> i32 {
+! CHECK:           %[[VAL_0:.*]] = fir.dummy_scope : !fir.dscope
+! CHECK:           %[[VAL_1:.*]] = fir.alloca i32 {bindc_name = "func_noarg", uniq_name = "_QFfunc_noargEfunc_noarg"}
+! CHECK:           %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_1]] {uniq_name = "_QFfunc_noargEfunc_noarg"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:           %[[VAL_3:.*]] = arith.constant 1 : i32
+! CHECK:           hlfir.assign %[[VAL_3]] to %[[VAL_2]]#0 : i32, !fir.ref<i32>
+! CHECK:           %[[VAL_4:.*]] = fir.load %[[VAL_2]]#0 : !fir.ref<i32>
+! CHECK:           return %[[VAL_4]] : i32
+! CHECK:         }
diff --git a/flang/test/Lower/main_location.f90 b/flang/test/Lower/main_location.f90
index db63339288f03..73943f845f1ca 100644
--- a/flang/test/Lower/main_location.f90
+++ b/flang/test/Lower/main_location.f90
@@ -12,6 +12,7 @@
 end
 
 ! TEST1: func.func @_QQmain() {
+! TEST1-NEXT: fir.dummy_scope : !fir.dscope loc("{{.*}}test1.f90":1:1)
 ! TEST1-NEXT: return loc("{{.*}}test1.f90":3:1)
 ! TEST1-NEXT: } loc("{{.*}}test1.f90":1:1)
 
@@ -22,5 +23,6 @@
 end program
 
 ! TEST2: func.func @_QQmain() {
+! TEST2-NEXT: fir.dummy_scope : !fir.dscope loc("{{.*}}test2.f90":2:1)
 ! TEST2-NEXT: return loc("{{.*}}test2.f90":4:1)
 ! TEST2-NEXT: } loc("{{.*}}test2.f90":2:1)
diff --git a/flang/test/Transforms/tbaa-with-dummy-scope2.fir b/flang/test/Transforms/tbaa-with-dummy-scope2.fir
new file mode 100644
index 0000000000000..a28b4d56f442c
--- /dev/null
+++ b/flang/test/Transforms/tbaa-with-dummy-scope2.fir
@@ -0,0 +1,106 @@
+// RUN: fir-opt --fir-add-alias-tags --split-input-file %s | FileCheck %s
+
+// This test demonstrates the need for fir.dummy_scope even
+// when a function does not have dummy arguments.
+//
+// The original source is:
+// module m
+//   integer :: glob
+// end module m
+// subroutine test1
+//   use m
+//   call inner(glob)
+//   glob = 2
+// contains
+//   subroutine inner(x)
+//     integer :: x
+//     integer :: y
+//     y = 1
+//     x = y
+//   end subroutine inner
+// end subroutine test1
+//
+// 'inner' function is manually inlined in FIR.
+// When fir.dummy_scope is missing, TBAA tags for glob and x
+// are placed into the same TBAA root. Since glob is a global
+// and x is a dummy argument, TBAA ends up reporting no-alias
+// for them, which is incorrect.
+func.func @_QPtest1() attributes {noinline} {
+  %c1_i32 = arith.constant 1 : i32
+  %c2_i32 = arith.constant 2 : i32
+  %0 = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFtest1FinnerEy"}
+  %1 = fir.address_of(@_QMmEglob) : !fir.ref<i32>
+  %2 = fir.declare %1 {uniq_name = "_QMmEglob"} : (!fir.ref<i32>) -> !fir.ref<i32>
+  %3 = fir.dummy_scope : !fir.dscope
+  %4 = fir.declare %2 dummy_scope %3 {uniq_name = "_QFtest1FinnerEx"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
+  %5 = fir.declare %0 {uniq_name = "_QFtest1FinnerEy"} : (!fir.ref<i32>) -> !fir.ref<i32>
+  fir.store %c1_i32 to %5 : !fir.ref<i32>
+  %6 = fir.load %5 : !fir.ref<i32>
+  fir.store %6 to %4 : !fir.ref<i32>
+  fir.store %c2_i32 to %2 : !fir.ref<i32>
+  return
+}
+// CHECK: #[[$ATTR_0:.+]] = #llvm.tbaa_root<id = "Flang function root _QPtest1">
+// CHECK: #[[$ATTR_1:.+]] = #llvm.tbaa_type_desc<id = "any access", members = {<#[[$ATTR_0]], 0>}>
+// CHECK: #[[$ATTR_2:.+]] = #llvm.tbaa_type_desc<id = "any data access", members = {<#[[$ATTR_1]], 0>}>
+// CHECK: #[[$ATTR_3:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data", members = {<#[[$ATTR_2]], 0>}>
+// CHECK: #[[$ATTR_4:.+]] = #llvm.tbaa_type_desc<id = "global data", members = {<#[[$ATTR_2]], 0>}>
+// CHECK: #[[$ATTR_5:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data/_QFtest1FinnerEx", members = {<#[[$ATTR_3]], 0>}>
+// CHECK: #[[$ATTR_6:.+]] = #llvm.tbaa_type_desc<id = "global data/_QMmEglob", members = {<#[[$ATTR_4]], 0>}>
+// CHECK: #[[$ATTR_7:.+]] = #llvm.tbaa_tag<base_type = #[[$ATTR_5]], access_type = #[[$ATTR_5]], offset = 0>
+// CHECK: #[[$ATTR_8:.+]] = #llvm.tbaa_tag<base_type = #[[$ATTR_6]], access_type = #[[$ATTR_6]], offset = 0>
+// CHECK-LABEL:   func.func @_QPtest1() attributes {noinline} {
+// CHECK:           %[[VAL_2:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFtest1FinnerEy"}
+// CHECK:           %[[VAL_3:.*]] = fir.address_of(@_QMmEglob) : !fir.ref<i32>
+// CHECK:           %[[VAL_4:.*]] = fir.declare %[[VAL_3]] {uniq_name = "_QMmEglob"} : (!fir.ref<i32>) -> !fir.ref<i32>
+// CHECK:           %[[VAL_5:.*]] = fir.dummy_scope : !fir.dscope
+// CHECK:           %[[VAL_6:.*]] = fir.declare %[[VAL_4]] dummy_scope %[[VAL_5]] {uniq_name = "_QFtest1FinnerEx"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
+// CHECK:           %[[VAL_7:.*]] = fir.declare %[[VAL_2]] {uniq_name = "_QFtest1FinnerEy"} : (!fir.ref<i32>) -> !fir.ref<i32>
+// CHECK:           fir.store %{{.*}} to %[[VAL_7]] : !fir.ref<i32>
+// CHECK:           %[[VAL_8:.*]] = fir.load %[[VAL_7]] : !fir.ref<i32>
+// CHECK:           fir.store %[[VAL_8]] to %[[VAL_6]] {tbaa = [#[[$ATTR_7]]]} : !fir.ref<i32>
+// CHECK:           fir.store %{{.*}} to %[[VAL_4]] {tbaa = [#[[$ATTR_8]]]} : !fir.ref<i32>
+
+// -----
+
+// This test has fir.dummy_scope in place, and TBAA is correct.
+func.func @_QPtest2() attributes {noinline} {
+  %c1_i32 = arith.constant 1 : i32
+  %c2_i32 = arith.constant 2 : i32
+  %0 = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFtest2FinnerEy"}
+  %test_dummy_scope = fir.dummy_scope : !fir.dscope
+  %1 = fir.address_of(@_QMmEglob) : !fir.ref<i32>
+  %2 = fir.declare %1 {uniq_name = "_QMmEglob"} : (!fir.ref<i32>) -> !fir.ref<i32>
+  %3 = fir.dummy_scope : !fir.dscope
+  %4 = fir.declare %2 dummy_scope %3 {uniq_name = "_QFtest2FinnerEx"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
+  %5 = fir.declare %0 {uniq_name = "_QFtest2FinnerEy"} : (!fir.ref<i32>) -> !fir.ref<i32>
+  fir.store %c1_i32 to %5 : !fir.ref<i32>
+  %6 = fir.load %5 : !fir.ref<i32>
+  fir.store %6 to %4 : !fir.ref<i32>
+  fir.store %c2_i32 to %2 : !fir.ref<i32>
+  return
+}
+// CHECK: #[[$ATTR_0:.+]] = #llvm.tbaa_root<id = "Flang function root _QPtest2 - Scope 1">
+// CHECK: #[[$ATTR_1:.+]] = #llvm.tbaa_root<id = "Flang function root _QPtest2">
+// CHECK: #[[$ATTR_2:.+]] = #llvm.tbaa_type_desc<id = "any access", members = {<#[[$ATTR_0]], 0>}>
+// CHECK: #[[$ATTR_3:.+]] = #llvm.tbaa_type_desc<id = "any access", members = {<#[[$ATTR_1]], 0>}>
+// CHECK: #[[$ATTR_4:.+]] = #llvm.tbaa_type_desc<id = "any data access", members = {<#[[$ATTR_2]], 0>}>
+// CHECK: #[[$ATTR_5:.+]] = #llvm.tbaa_type_desc<id = "any data access", members = {<#[[$ATTR_3]], 0>}>
+// CHECK: #[[$ATTR_6:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data", members = {<#[[$ATTR_4]], 0>}>
+// CHECK: #[[$ATTR_7:.+]] = #llvm.tbaa_type_desc<id = "global data", members = {<#[[$ATTR_5]], 0>}>
+// CHECK: #[[$ATTR_8:.+]] = #llvm.tbaa_type_desc<id = "dummy arg data/_QFtest2FinnerEx", members = {<#[[$ATTR_6]], 0>}>
+// CHECK: #[[$ATTR_9:.+]] = #llvm.tbaa_type_desc<id = "global data/_QMmEglob", members = {<#[[$ATTR_7]], 0>}>
+// CHECK: #[[$ATTR_10:.+]] = #llvm.tbaa_tag<base_type = #[[$ATTR_8]], access_type = #[[$ATTR_8]], offset = 0>
+// CHECK: #[[$ATTR_11:.+]] = #llvm.tbaa_tag<base_type = #[[$ATTR_9]], access_type = #[[$ATTR_9]], offset = 0>
+// CHECK-LABEL:   func.func @_QPtest2() attributes {noinline} {
+// CHECK:           %[[VAL_2:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFtest2FinnerEy"}
+// CHECK:           %[[VAL_3:.*]] = fir.dummy_scope : !fir.dscope
+// CHECK:           %[[VAL_4:.*]] = fir.address_of(@_QMmEglob) : !fir.ref<i32>
+// CHECK:           %[[VAL_5:.*]] = fir.declare %[[VAL_4]] {uniq_name = "_QMmEglob"} : (!fir.ref<i32>) -> !fir.ref<i32>
+// CHECK:           %[[VAL_6:.*]] = fir.dummy_scope : !fir.dscope
+// CHECK:           %[[VAL_7:.*]] = fir.declare %[[VAL_5]] dummy_scope %[[VAL_6]] {uniq_name = "_QFtest2FinnerEx"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
+// CHECK:           %[[VAL_8:.*]] = fir.declare %[[VAL_2]] {uniq_name = "_QFtest2FinnerEy"} : (!fir.ref<i32>) -> !fir.ref<i32>
+// CHECK:           fir.store %{{.*}} to %[[VAL_8]] : !fir.ref<i32>
+// CHECK:           %[[VAL_9:.*]] = fir.load %[[VAL_8]] : !fir.ref<i32>
+// CHECK:           fir.store %[[VAL_9]] to %[[VAL_7]] {tbaa = [#[[$ATTR_10]]]} : !fir.ref<i32>
+// CHECK:           fir.store %{{.*}} to %[[VAL_5]] {tbaa = [#[[$ATTR_11]]]} : !fir.ref<i32>

Copy link
Contributor

@ashermancinelli ashermancinelli left a comment

Choose a reason for hiding this comment

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

Lgtm! Thank you!

The nesting of fir.dummy_scope operations defines the roots
of the TBAA forest. If we do not generate fir.dummy_scope
in functions that do not have any dummy arguments, then
the globals accessed in the function and the dummy arguments
accessed by the callee may end up in different sub-trees
of the same root. The added tbaa-with-dummy-scope2.fir
demonstrates the issue.
@vzakhari vzakhari force-pushed the noarg_dummy_scope branch from a6d06fe to 96d0bb8 Compare April 18, 2025 23:49
@vzakhari vzakhari merged commit 50db7a7 into llvm:main Apr 19, 2025
12 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
The nesting of fir.dummy_scope operations defines the roots
of the TBAA forest. If we do not generate fir.dummy_scope
in functions that do not have any dummy arguments, then
the globals accessed in the function and the dummy arguments
accessed by the callee may end up in different sub-trees
of the same root. The added tbaa-with-dummy-scope2.fir
demonstrates the issue.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
The nesting of fir.dummy_scope operations defines the roots
of the TBAA forest. If we do not generate fir.dummy_scope
in functions that do not have any dummy arguments, then
the globals accessed in the function and the dummy arguments
accessed by the callee may end up in different sub-trees
of the same root. The added tbaa-with-dummy-scope2.fir
demonstrates the issue.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
The nesting of fir.dummy_scope operations defines the roots
of the TBAA forest. If we do not generate fir.dummy_scope
in functions that do not have any dummy arguments, then
the globals accessed in the function and the dummy arguments
accessed by the callee may end up in different sub-trees
of the same root. The added tbaa-with-dummy-scope2.fir
demonstrates the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:driver flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants