[flang-commits] [PATCH] D133174: [flang] Add atomic_fetch_or to the list of intrinsics

[Jean Perier via Phabricator via flang-commits]

jeanPerier added a comment.

A nit and a question, otherwise looks good to me.



================
Comment at: flang/lib/Evaluate/intrinsics.cpp:1632
+          argOk = false;
+        }
+        if (!argOk) {
----------------
nit: `argOk = rank == 0 && (IsCoarray(*arg) || ExtractCoarrayRef(*arg));` would be readable and shorter.


================
Comment at: flang/module/iso_fortran_env.f90:20
+    team_type => __builtin_team_type, &
+    __builtin_atomic_int_kind, &
+    __builtin_atomic_logical_kind
----------------
Is there a reason to define new parameters instead of doing renaming here (e.g., `atomic_int_kind 
 => __builtin_atomic_int_kind`) ?

If renames are possible, I think they should be favored because they avoid "spilling" the builtin symbols. With the current patch, a user using iso_fortran_env could directly use `__builtin_atomic_int_kind` and `__builtin_atomic_logical_kind`, and I do not think we should allow that to make it impossible for users to start relying on the builtin names for any weird reason as if it was an extension.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133174/new/

https://reviews.llvm.org/D133174