Skip to content
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

[flang][openacc][NFC] Get rid of use-desc-for-alloc=false option in atomic tests #72291

Merged
merged 3 commits into from
Nov 14, 2023

Conversation

clementval
Copy link
Contributor

@clementval clementval commented Nov 14, 2023

This option change the default lowering and should not be used. Make the atomic tests more homogenous to the rest of the OpenACC tests. No OpenACC tests use this option.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir openacc labels Nov 14, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 14, 2023

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

@llvm/pr-subscribers-openacc

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

This option change the default lowering and should not be used. Make the atomic tests more homogenous.


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

2 Files Affected:

  • (modified) flang/test/Lower/OpenACC/acc-atomic-read.f90 (+9-18)
  • (modified) flang/test/Lower/OpenACC/acc-atomic-update.f90 (+13-20)
diff --git a/flang/test/Lower/OpenACC/acc-atomic-read.f90 b/flang/test/Lower/OpenACC/acc-atomic-read.f90
index 46217d86f01fbf4..985029d3b92d187 100644
--- a/flang/test/Lower/OpenACC/acc-atomic-read.f90
+++ b/flang/test/Lower/OpenACC/acc-atomic-read.f90
@@ -1,4 +1,4 @@
-! RUN: bbc --use-desc-for-alloc=false -fopenacc -emit-fir -hlfir=false %s -o - | FileCheck %s
+! RUN: bbc -fopenacc -emit-fir -hlfir=false %s -o - | FileCheck %s
 
 ! This test checks the lowering of atomic read
 
@@ -20,23 +20,14 @@ end program acc_atomic_test
 ! operation, instead of %[[VAL_3]] and %[[VAL_0]].
 
 !CHECK-LABEL: func.func @_QPatomic_read_pointer() {
-!CHECK:         %[[VAL_0:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "x", uniq_name = "_QFatomic_read_pointerEx"}
-!CHECK:         %[[VAL_1:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFatomic_read_pointerEx.addr"}
-!CHECK:         %[[VAL_2:.*]] = fir.zero_bits !fir.ptr<i32>
-!CHECK:         fir.store %[[VAL_2]] to %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
-!CHECK:         %[[VAL_3:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "y", uniq_name = "_QFatomic_read_pointerEy"}
-!CHECK:         %[[VAL_4:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFatomic_read_pointerEy.addr"}
-!CHECK:         %[[VAL_5:.*]] = fir.zero_bits !fir.ptr<i32>
-!CHECK:         fir.store %[[VAL_5]] to %[[VAL_4]] : !fir.ref<!fir.ptr<i32>>
-!CHECK:         %[[VAL_6:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
-!CHECK:         %[[VAL_7:.*]] = fir.load %[[VAL_4]] : !fir.ref<!fir.ptr<i32>>
-!CHECK:         acc.atomic.read %[[VAL_7]] = %[[VAL_6]]   : !fir.ptr<i32>, i32
-!CHECK:         %[[VAL_8:.*]] = fir.load %[[VAL_4]] : !fir.ref<!fir.ptr<i32>>
-!CHECK:         %[[VAL_9:.*]] = fir.load %[[VAL_8]] : !fir.ptr<i32>
-!CHECK:         %[[VAL_10:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
-!CHECK:         fir.store %[[VAL_9]] to %[[VAL_10]] : !fir.ptr<i32>
-!CHECK:         return
-!CHECK:       }
+! CHECK:   %[[X:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "x", uniq_name = "_QFatomic_read_pointerEx"}
+! CHECK:   %[[Y:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "y", uniq_name = "_QFatomic_read_pointerEy"}
+! CHECK:   %[[LOAD_X:.*]] = fir.load %[[X]] : !fir.ref<!fir.box<!fir.ptr<i32>>>
+! CHECK:   %[[BOX_ADDR_X:.*]] = fir.box_addr %[[LOAD_X]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+! CHECK:   %[[LOAD_Y:.*]] = fir.load %[[Y]] : !fir.ref<!fir.box<!fir.ptr<i32>>>
+! CHECK:   %[[BOX_ADDR_Y:.*]] = fir.box_addr %[[LOAD_Y]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+! CHECK:   acc.atomic.read %[[BOX_ADDR_Y]] = %[[BOX_ADDR_X]] : !fir.ptr<i32>, i32
+! CHECK: }
 
 subroutine atomic_read_pointer()
   integer, pointer :: x, y
diff --git a/flang/test/Lower/OpenACC/acc-atomic-update.f90 b/flang/test/Lower/OpenACC/acc-atomic-update.f90
index 1bca04320b7ef7c..01e1f7e2d5b008b 100644
--- a/flang/test/Lower/OpenACC/acc-atomic-update.f90
+++ b/flang/test/Lower/OpenACC/acc-atomic-update.f90
@@ -1,6 +1,6 @@
 ! This test checks lowering of atomic and atomic update constructs
-! RUN: bbc --use-desc-for-alloc=false -fopenacc -emit-fir -hlfir=false %s -o - | FileCheck %s
-! RUN: %flang_fc1 -mllvm --use-desc-for-alloc=false -emit-fir -flang-deprecated-no-hlfir -fopenacc %s -o - | FileCheck %s
+! RUN: bbc -fopenacc -emit-fir -emit-fir -hlfir=false %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenacc %s -o - | FileCheck %s
 
 program acc_atomic_update_test
     integer :: x, y, z
@@ -11,33 +11,25 @@ program acc_atomic_update_test
     a=>c
     b=>d
 
-!CHECK: func.func @_QQmain() attributes {fir.bindc_name = "acc_atomic_update_test"} {
 !CHECK: %[[A:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "a", uniq_name = "_QFEa"}
-!CHECK: %[[A_ADDR:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFEa.addr"}
-!CHECK: %{{.*}} = fir.zero_bits !fir.ptr<i32>
-!CHECK: fir.store %{{.*}} to %[[A_ADDR]] : !fir.ref<!fir.ptr<i32>>
 !CHECK: %[[B:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "b", uniq_name = "_QFEb"}
-!CHECK: %[[B_ADDR:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFEb.addr"}
-!CHECK: %{{.*}} = fir.zero_bits !fir.ptr<i32>
-!CHECK: fir.store %{{.*}} to %[[B_ADDR]] : !fir.ref<!fir.ptr<i32>>
 !CHECK: %[[C_ADDR:.*]] = fir.address_of(@_QFEc) : !fir.ref<i32>
 !CHECK: %[[D_ADDR:.*]] = fir.address_of(@_QFEd) : !fir.ref<i32>
 !CHECK: %[[I1:.*]] = fir.alloca i8 {bindc_name = "i1", uniq_name = "_QFEi1"}
 !CHECK: %[[X:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"}
 !CHECK: %[[Y:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFEy"}
 !CHECK: %[[Z:.*]] = fir.alloca i32 {bindc_name = "z", uniq_name = "_QFEz"}
-!CHECK: %{{.*}} = fir.convert %[[C_ADDR]] : (!fir.ref<i32>) -> !fir.ptr<i32>
-!CHECK: fir.store %{{.*}} to %[[A_ADDR]] : !fir.ref<!fir.ptr<i32>>
-!CHECK: %{{.*}} = fir.convert %[[D_ADDR]] : (!fir.ref<i32>) -> !fir.ptr<i32>
-!CHECK: fir.store {{.*}} to %[[B_ADDR]] : !fir.ref<!fir.ptr<i32>>
-!CHECK: %[[LOADED_A:.*]] = fir.load %[[A_ADDR]] : !fir.ref<!fir.ptr<i32>>
-!CHECK: %[[LOADED_B:.*]] = fir.load %[[B_ADDR]] : !fir.ref<!fir.ptr<i32>>
-!CHECK: %{{.*}} = fir.load %[[LOADED_B]] : !fir.ptr<i32>
-!CHECK:  acc.atomic.update   %[[LOADED_A]] : !fir.ptr<i32> {
-!CHECK:  ^bb0(%[[ARG:.*]]: i32):
-!CHECK:    %[[RESULT:.*]] = arith.addi %[[ARG]], %{{.*}} : i32
-!CHECK:    acc.yield %[[RESULT]] : i32
+!CHECK: %[[LOAD_A:.*]] = fir.load %[[A]] : !fir.ref<!fir.box<!fir.ptr<i32>>>
+!CHECK: %[[BOX_ADDR_A:.*]] = fir.box_addr %[[LOAD_A]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+!CHECK: %[[LOAD_B:.*]] = fir.load %[[B]] : !fir.ref<!fir.box<!fir.ptr<i32>>>
+!CHECK: %[[BOX_ADDR_B:.*]] = fir.box_addr %[[LOAD_B]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+!CHECK: %[[LOAD_BOX_ADDR_B:.*]] = fir.load %[[BOX_ADDR_B]] : !fir.ptr<i32>
+!CHECK: acc.atomic.update %[[BOX_ADDR_A]] : !fir.ptr<i32> {
+!CHECK: ^bb0(%[[ARG0:.*]]: i32):
+!CHECK:   %[[ADD:.*]] = arith.addi %[[ARG0]], %[[LOAD_BOX_ADDR_B]] : i32
+!CHECK:   acc.yield %[[ADD]] : i32
 !CHECK: }
+
     !$acc atomic update
         a = a + b 
 
@@ -72,3 +64,4 @@ program acc_atomic_update_test
 !CHECK:  return
 !CHECK: }
 end program acc_atomic_update_test
+

Copy link
Contributor

@razvanlupusoru razvanlupusoru left a comment

Choose a reason for hiding this comment

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

LGTM

@clementval clementval merged commit cc9f176 into llvm:main Nov 14, 2023
2 of 3 checks passed
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…tomic tests (llvm#72291)

This option change the default lowering and should not be used. Make the
atomic tests more homogenous to the rest of the OpenACC tests. No
OpenACC tests use this option.
@clementval clementval deleted the acc_atomic_test1 branch January 18, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category openacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants