Commit 2dc8f08
[flang][openacc][openmp] Support implicit casting on the atomic interface (llvm#114390)
ACCMP atomics do not support type conversion. Specifically, I have
encountered semantically incorrect code for atomic reads.
Example:
```
program main
implicit none
real(8) :: n
integer :: x
x = 1.0
!$acc atomic capture
n = x
x = n
!$acc end atomic
end program main
```
We have this error when compiling it with flang-new: `error:
loc("rep.f90":6:9): expected three operations in atomic.capture region
(one terminator, and two atomic ops)`
Yet, in the following generated FIR code, we observe three issues.
1. `fir.convert` intrudes into the capture region.
2. An incorrect temporary (`%2`) is being updated instead of `n`.
3. If we allow `n` in place of `%2`, the operand types of `atomic.read`
do not match. Introducing a `!fir.ref<i32> -> !fir.ref<f64>` conversion
on `x` is inaccurate because we need to convert the value of `x`.
```
%2 = "fir.alloca"() <{in_type = i32, operandSegmentSizes = array<i32: 0, 0>}> : () -> !fir.ref<i32>
%3 = "fir.alloca"() <{bindc_name = "n", in_type = f64, operandSegmentSizes = array<i32: 0, 0>, uniq_name = "_QFEn"}> : () -> !fir.ref<f64>
%4:2 = "hlfir.declare"(%3) <{operandSegmentSizes = array<i32: 1, 0, 0, 0>, uniq_name = "_QFEn"}> : (!fir.ref<f64>) -> (!fir.ref<f64>, !fir.ref<f64>)
%5 = "fir.alloca"() <{bindc_name = "x", in_type = i32, operandSegmentSizes = array<i32: 0, 0>, uniq_name = "_QFEx"}> : () -> !fir.ref<i32>
%6:2 = "hlfir.declare"(%5) <{operandSegmentSizes = array<i32: 1, 0, 0, 0>, uniq_name = "_QFEx"}> : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
%7 = "arith.constant"() <{value = 1 : i32}> : () -> i32
"hlfir.assign"(%7, %6#0) : (i32, !fir.ref<i32>) -> ()
%8 = "fir.load"(%4#0) : (!fir.ref<f64>) -> f64
%9 = "fir.convert"(%8) : (f64) -> i32
"fir.store"(%9, %2) : (i32, !fir.ref<i32>) -> ()
%10 = "fir.load"(%6#0) : (!fir.ref<i32>) -> i32
%11 = "fir.convert"(%10) : (i32) -> f64
"acc.atomic.capture"() ({
"acc.atomic.read"(%2, %6#1) <{element_type = f64}> : (!fir.ref<i32>, !fir.ref<i32>) -> ()
%12 = "fir.convert"(%11) : (f64) -> i32
"acc.atomic.write"(%2, %12) : (!fir.ref<i32>, i32) -> ()
"acc.terminator"() : () -> ()
}) : () -> ()
```
This PR updates `flang/lib/Lower/DirectivesCommon.h` to solve the issues
by taking the following approaches (from top to bottom):
1. Move `fir.convert` for `atomic.write` out of the capture region.
2. Remove the `!fir.ref<i32> -> !fir.ref<f64>` conversion found in
`genOmpAccAtomicRead`.
3. Eliminate unnecessary `genExprAddr` calls on the RHS, which create an
invalid temporary for `x = 1.0`.
4. When generating a capture operation, refer to the original LHS
instead of the type-casted RHS.
Here, we have to allow for the cases where the operand types of
`atomic.read` differ from one another. Thus, this PR also removes the
`AllTypesMatch` trait from both `acc.atomic.read` and `omp.atomic.read`.
The example code is converted as follows:
```
%0 = fir.alloca f64 {bindc_name = "n", uniq_name = "_QFEn"}
%1:2 = hlfir.declare %0 {uniq_name = "_QFEn"} : (!fir.ref<f64>) -> (!fir.ref<f64>, !fir.ref<f64>)
%2 = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"}
%3:2 = hlfir.declare %2 {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
%c1_i32 = arith.constant 1 : i32
hlfir.assign %c1_i32 to %3#0 : i32, !fir.ref<i32>
%4 = fir.load %1#0 : !fir.ref<f64>
%5 = fir.convert %4 : (f64) -> i32
acc.atomic.capture {
acc.atomic.read %1#1 = %3#1 : !fir.ref<f64>, !fir.ref<i32>, i32
acc.atomic.write %3#1 = %5 : !fir.ref<i32>, i32
}
```
Fixes llvm#112911.1 parent 04ba09f commit 2dc8f08
File tree
17 files changed
+290
-211
lines changed- flang
- lib/Lower
- test
- Fir
- Lower
- OpenACC
- OpenMP
- mlir
- include/mlir/Dialect
- OpenACC
- OpenMP
- test
- Conversion/OpenMPToLLVM
- Dialect
- OpenACC
- OpenMP
- Target/LLVMIR
17 files changed
+290
-211
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
179 | 179 | | |
180 | 180 | | |
181 | 181 | | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
182 | 185 | | |
| 186 | + | |
183 | 187 | | |
184 | 188 | | |
185 | 189 | | |
| |||
410 | 414 | | |
411 | 415 | | |
412 | 416 | | |
413 | | - | |
414 | | - | |
415 | | - | |
416 | | - | |
417 | 417 | | |
418 | 418 | | |
419 | 419 | | |
| |||
497 | 497 | | |
498 | 498 | | |
499 | 499 | | |
500 | | - | |
501 | | - | |
502 | 500 | | |
503 | | - | |
504 | | - | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
505 | 505 | | |
506 | | - | |
507 | | - | |
508 | | - | |
509 | | - | |
510 | | - | |
511 | | - | |
512 | | - | |
513 | | - | |
514 | | - | |
515 | | - | |
516 | | - | |
517 | 506 | | |
518 | 507 | | |
519 | 508 | | |
| |||
545 | 534 | | |
546 | 535 | | |
547 | 536 | | |
548 | | - | |
| 537 | + | |
549 | 538 | | |
550 | | - | |
| 539 | + | |
551 | 540 | | |
552 | 541 | | |
553 | 542 | | |
554 | | - | |
| 543 | + | |
555 | 544 | | |
556 | 545 | | |
557 | 546 | | |
558 | 547 | | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
559 | 552 | | |
560 | 553 | | |
561 | | - | |
| 554 | + | |
562 | 555 | | |
563 | | - | |
| 556 | + | |
564 | 557 | | |
565 | 558 | | |
566 | 559 | | |
567 | | - | |
| 560 | + | |
568 | 561 | | |
569 | 562 | | |
570 | 563 | | |
571 | 564 | | |
572 | 565 | | |
573 | | - | |
574 | 566 | | |
575 | 567 | | |
576 | | - | |
577 | | - | |
578 | | - | |
579 | | - | |
580 | | - | |
581 | | - | |
| 568 | + | |
582 | 569 | | |
583 | 570 | | |
584 | 571 | | |
585 | 572 | | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
586 | 577 | | |
587 | 578 | | |
588 | 579 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
781 | 781 | | |
782 | 782 | | |
783 | 783 | | |
784 | | - | |
| 784 | + | |
785 | 785 | | |
786 | 786 | | |
787 | 787 | | |
788 | | - | |
| 788 | + | |
789 | 789 | | |
790 | 790 | | |
791 | 791 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
50 | | - | |
| 50 | + | |
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
| |||
82 | 82 | | |
83 | 83 | | |
84 | 84 | | |
85 | | - | |
| 85 | + | |
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
| |||
118 | 118 | | |
119 | 119 | | |
120 | 120 | | |
121 | | - | |
| 121 | + | |
122 | 122 | | |
123 | 123 | | |
124 | 124 | | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
125 | 214 | | |
126 | 215 | | |
127 | 216 | | |
| |||
136 | 225 | | |
137 | 226 | | |
138 | 227 | | |
139 | | - | |
| 228 | + | |
140 | 229 | | |
141 | 230 | | |
142 | 231 | | |
| |||
163 | 252 | | |
164 | 253 | | |
165 | 254 | | |
166 | | - | |
| 255 | + | |
167 | 256 | | |
168 | 257 | | |
169 | 258 | | |
| |||
184 | 273 | | |
185 | 274 | | |
186 | 275 | | |
187 | | - | |
| 276 | + | |
188 | 277 | | |
189 | 278 | | |
190 | 279 | | |
| |||
215 | 304 | | |
216 | 305 | | |
217 | 306 | | |
218 | | - | |
| 307 | + | |
219 | 308 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | 44 | | |
45 | | - | |
| 45 | + | |
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
48 | | - | |
| 48 | + | |
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
| |||
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
91 | | - | |
| 91 | + | |
92 | 92 | | |
0 commit comments