From 50db7a7d269b42f0cda63eb005aadfdbe25f56cb Mon Sep 17 00:00:00 2001 From: Slava Zakharin Date: Fri, 18 Apr 2025 17:19:12 -0700 Subject: [PATCH] [flang] Fixed fir.dummy_scope generation to work for TBAA. (#136382) 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. --- flang/lib/Lower/Bridge.cpp | 5 +- flang/test/Driver/emit-mlir.f90 | 1 + flang/test/Lower/HLFIR/calls-f77.f90 | 1 + flang/test/Lower/HLFIR/dummy-scope.f90 | 52 +++++++++ flang/test/Lower/OpenMP/real10.f90 | 2 +- flang/test/Lower/main_location.f90 | 2 + .../Transforms/tbaa-with-dummy-scope2.fir | 106 ++++++++++++++++++ 7 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 flang/test/Lower/HLFIR/dummy-scope.f90 create mode 100644 flang/test/Transforms/tbaa-with-dummy-scope2.fir diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp index b4d1197822a4..1652a86ed7e6 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -5508,7 +5508,10 @@ private: 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(toLocation()); setDummyArgsScope(scopeOp); } diff --git a/flang/test/Driver/emit-mlir.f90 b/flang/test/Driver/emit-mlir.f90 index 2345a7cf53e4..de5a62d6bc7f 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 64b90b83b828..450f8811eb5e 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 : () -> () ! 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 000000000000..4b1a3324c077 --- /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 {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, !fir.dscope) -> (!fir.ref, !fir.ref) +! 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 {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) -> (!fir.ref, !fir.ref) +! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %[[VAL_1]] {uniq_name = "_QFfunc_argEx"} : (!fir.ref, !fir.dscope) -> (!fir.ref, !fir.ref) +! CHECK: %[[VAL_5:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref +! CHECK: hlfir.assign %[[VAL_5]] to %[[VAL_3]]#0 : i32, !fir.ref +! CHECK: %[[VAL_6:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref +! 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) -> (!fir.ref, !fir.ref) +! CHECK: %[[VAL_3:.*]] = arith.constant 1 : i32 +! CHECK: hlfir.assign %[[VAL_3]] to %[[VAL_2]]#0 : i32, !fir.ref +! CHECK: %[[VAL_4:.*]] = fir.load %[[VAL_2]]#0 : !fir.ref +! CHECK: return %[[VAL_4]] : i32 +! CHECK: } diff --git a/flang/test/Lower/OpenMP/real10.f90 b/flang/test/Lower/OpenMP/real10.f90 index 0a092ebf6317..a31d2ace8004 100644 --- a/flang/test/Lower/OpenMP/real10.f90 +++ b/flang/test/Lower/OpenMP/real10.f90 @@ -2,7 +2,7 @@ !RUN: %flang_fc1 -emit-hlfir -fopenmp -triple amdgcn -fopenmp -fopenmp-is-target-device -o - %s | FileCheck %s -!CHECK: hlfir.declare %0 {uniq_name = "_QFEx"} : (!fir.ref) -> (!fir.ref, !fir.ref) +!CHECK: hlfir.declare %{{.*}} {uniq_name = "_QFEx"} : (!fir.ref) -> (!fir.ref, !fir.ref) program p real(10) :: x diff --git a/flang/test/Lower/main_location.f90 b/flang/test/Lower/main_location.f90 index db63339288f0..73943f845f1c 100644 --- a/flang/test/Lower/main_location.f90 +++ b/flang/test/Lower/main_location.f90 @@ -12,6 +12,7 @@ endif 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 @@ endif 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 000000000000..a28b4d56f442 --- /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 + %2 = fir.declare %1 {uniq_name = "_QMmEglob"} : (!fir.ref) -> !fir.ref + %3 = fir.dummy_scope : !fir.dscope + %4 = fir.declare %2 dummy_scope %3 {uniq_name = "_QFtest1FinnerEx"} : (!fir.ref, !fir.dscope) -> !fir.ref + %5 = fir.declare %0 {uniq_name = "_QFtest1FinnerEy"} : (!fir.ref) -> !fir.ref + fir.store %c1_i32 to %5 : !fir.ref + %6 = fir.load %5 : !fir.ref + fir.store %6 to %4 : !fir.ref + fir.store %c2_i32 to %2 : !fir.ref + return +} +// CHECK: #[[$ATTR_0:.+]] = #llvm.tbaa_root +// CHECK: #[[$ATTR_1:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_2:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_3:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_4:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_5:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_6:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_7:.+]] = #llvm.tbaa_tag +// CHECK: #[[$ATTR_8:.+]] = #llvm.tbaa_tag +// 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 +// CHECK: %[[VAL_4:.*]] = fir.declare %[[VAL_3]] {uniq_name = "_QMmEglob"} : (!fir.ref) -> !fir.ref +// CHECK: %[[VAL_5:.*]] = fir.dummy_scope : !fir.dscope +// CHECK: %[[VAL_6:.*]] = fir.declare %[[VAL_4]] dummy_scope %[[VAL_5]] {uniq_name = "_QFtest1FinnerEx"} : (!fir.ref, !fir.dscope) -> !fir.ref +// CHECK: %[[VAL_7:.*]] = fir.declare %[[VAL_2]] {uniq_name = "_QFtest1FinnerEy"} : (!fir.ref) -> !fir.ref +// CHECK: fir.store %{{.*}} to %[[VAL_7]] : !fir.ref +// CHECK: %[[VAL_8:.*]] = fir.load %[[VAL_7]] : !fir.ref +// CHECK: fir.store %[[VAL_8]] to %[[VAL_6]] {tbaa = [#[[$ATTR_7]]]} : !fir.ref +// CHECK: fir.store %{{.*}} to %[[VAL_4]] {tbaa = [#[[$ATTR_8]]]} : !fir.ref + +// ----- + +// 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 + %2 = fir.declare %1 {uniq_name = "_QMmEglob"} : (!fir.ref) -> !fir.ref + %3 = fir.dummy_scope : !fir.dscope + %4 = fir.declare %2 dummy_scope %3 {uniq_name = "_QFtest2FinnerEx"} : (!fir.ref, !fir.dscope) -> !fir.ref + %5 = fir.declare %0 {uniq_name = "_QFtest2FinnerEy"} : (!fir.ref) -> !fir.ref + fir.store %c1_i32 to %5 : !fir.ref + %6 = fir.load %5 : !fir.ref + fir.store %6 to %4 : !fir.ref + fir.store %c2_i32 to %2 : !fir.ref + return +} +// CHECK: #[[$ATTR_0:.+]] = #llvm.tbaa_root +// CHECK: #[[$ATTR_1:.+]] = #llvm.tbaa_root +// CHECK: #[[$ATTR_2:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_3:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_4:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_5:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_6:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_7:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_8:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_9:.+]] = #llvm.tbaa_type_desc}> +// CHECK: #[[$ATTR_10:.+]] = #llvm.tbaa_tag +// CHECK: #[[$ATTR_11:.+]] = #llvm.tbaa_tag +// 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 +// CHECK: %[[VAL_5:.*]] = fir.declare %[[VAL_4]] {uniq_name = "_QMmEglob"} : (!fir.ref) -> !fir.ref +// CHECK: %[[VAL_6:.*]] = fir.dummy_scope : !fir.dscope +// CHECK: %[[VAL_7:.*]] = fir.declare %[[VAL_5]] dummy_scope %[[VAL_6]] {uniq_name = "_QFtest2FinnerEx"} : (!fir.ref, !fir.dscope) -> !fir.ref +// CHECK: %[[VAL_8:.*]] = fir.declare %[[VAL_2]] {uniq_name = "_QFtest2FinnerEy"} : (!fir.ref) -> !fir.ref +// CHECK: fir.store %{{.*}} to %[[VAL_8]] : !fir.ref +// CHECK: %[[VAL_9:.*]] = fir.load %[[VAL_8]] : !fir.ref +// CHECK: fir.store %[[VAL_9]] to %[[VAL_7]] {tbaa = [#[[$ATTR_10]]]} : !fir.ref +// CHECK: fir.store %{{.*}} to %[[VAL_5]] {tbaa = [#[[$ATTR_11]]]} : !fir.ref