R3: Addressing review comments

Author: anchuraj

flang/include/flang/Frontend/TargetOptions.h

@@ -54,7 +54,7 @@ class TargetOptions {
   /// Print verbose assembly
   bool asmVerbose = false;
 
-  /// Atomic Control Options for AMD GPU
+  /// Atomic control options for AMD gpu
   bool amdgpuIgnoreDenormalMode = false;
   bool amdgpuRemoteMemory = false;
   bool amdgpuFineGrainedMemory = false;

flang/include/flang/Optimizer/Dialect/Support/FIRContext.h

@@ -58,7 +58,7 @@ void setTargetCPU(mlir::ModuleOp mod, llvm::StringRef cpu);
 /// Get the target CPU string from the Module or return a null reference.
 llvm::StringRef getTargetCPU(mlir::ModuleOp mod);
 
-// Setters and Getters for atomic control options
+// Setters and getters for atomic control options.
 void setAmdgpuIgnoreDenormalMode(mlir::ModuleOp mod);
 bool getAmdgpuIgnoreDenormalMode(mlir::ModuleOp mod);
 void setAmdgpuFineGrainedMemory(mlir::ModuleOp mod);
@@ -70,7 +70,7 @@ bool getAmdgpuRemoteMemory(mlir::ModuleOp mod);
 /// module `mod` is still live.
 void setTuneCPU(mlir::ModuleOp mod, llvm::StringRef cpu);
 
-// set Atomic control options for amd gpu.
+// set atomic control options for AMD gpu.
 void setAmdgpuAtomicControlOptions(mlir::ModuleOp mod,
                                    bool amdgpuIgnoreDenormalMode,
                                    bool amdgpuNoFineGrainedMemory,

flang/lib/Lower/OpenMP/OpenMP.cpp

@@ -2851,13 +2851,14 @@ genAtomicUpdate(lower::AbstractConverter &converter, mlir::Location loc,
   }
 
   mlir::ModuleOp module = builder.getModule();
-  mlir::omp::AtomicControlAttr atomicControlAttr = mlir::omp::AtomicControlAttr::get(
-      builder.getContext(), fir::getAmdgpuIgnoreDenormalMode(module),
-      fir::getAmdgpuFineGrainedMemory(module),
-      fir::getAmdgpuRemoteMemory(module));
+  mlir::omp::AtomicControlAttr atomicControlAttr =
+      mlir::omp::AtomicControlAttr::get(
+          builder.getContext(), fir::getAmdgpuIgnoreDenormalMode(module),
+          fir::getAmdgpuFineGrainedMemory(module),
+          fir::getAmdgpuRemoteMemory(module));
   builder.restoreInsertionPoint(atomicAt);
-  auto updateOp =
-      builder.create<mlir::omp::AtomicUpdateOp>(loc, atomAddr, atomicControlAttr, hint, memOrder);
+  auto updateOp = builder.create<mlir::omp::AtomicUpdateOp>(
+      loc, atomAddr, atomicControlAttr, hint, memOrder);
 
   mlir::Region &region = updateOp->getRegion(0);
   mlir::Block *block = builder.createBlock(&region, {}, {atomType}, {loc});

flang/lib/Optimizer/Dialect/Support/FIRContext.cpp

@@ -96,8 +96,7 @@ void fir::setAmdgpuIgnoreDenormalMode(mlir::ModuleOp mod) {
 }
 
 bool fir::getAmdgpuIgnoreDenormalMode(mlir::ModuleOp mod) {
-  if (auto attr =
-          mod->getAttrOfType<mlir::UnitAttr>(amdgpuIgnoreDenormalModeName))
+  if (mod->hasAttrOfType<mlir::UnitAttr>(amdgpuIgnoreDenormalModeName))
     return true;
   return false;
 }
@@ -110,8 +109,7 @@ void fir::setAmdgpuFineGrainedMemory(mlir::ModuleOp mod) {
 }
 
 bool fir::getAmdgpuFineGrainedMemory(mlir::ModuleOp mod) {
-  if (auto attr =
-          mod->getAttrOfType<mlir::UnitAttr>(amdgpuFineGrainedMemoryName))
+  if (mod->hasAttrOfType<mlir::UnitAttr>(amdgpuFineGrainedMemoryName))
     return true;
   return false;
 }
@@ -123,7 +121,7 @@ void fir::setAmdgpuRemoteMemory(mlir::ModuleOp mod) {
 }
 
 bool fir::getAmdgpuRemoteMemory(mlir::ModuleOp mod) {
-  if (auto attr = mod->getAttrOfType<mlir::UnitAttr>(amdgpuRemoteMemoryName))
+  if (mod->hasAttrOfType<mlir::UnitAttr>(amdgpuRemoteMemoryName))
     return true;
   return false;
 }

flang/test/Fir/convert-to-llvm-openmp-and-fir.fir

@@ -619,7 +619,7 @@ func.func @_QPs() {
     %c1_i32 = arith.constant 1 : i32
     %1 = arith.addi %arg0, %c1_i32 : i32
     omp.yield(%1 : i32)
-  }
+  } {atomic_control = #omp.atomic_control<>}
   return
 }
 fir.global internal @_QFsEc : i32 {
@@ -634,7 +634,7 @@ fir.global internal @_QFsEc : i32 {
 // CHECK:      %[[CONST_1:.*]] = llvm.mlir.constant(1 : i32) : i32
 // CHECK:      %[[OUT_VAL:.*]] = llvm.add %[[IN_VAL]], %[[CONST_1]]  : i32
 // CHECK:      omp.yield(%[[OUT_VAL]] : i32)
-// CHECK:    }
+// CHECK:    } {atomic_control = #omp.atomic_control<>}
 // CHECK:    llvm.return
 // CHECK:  }
 // CHECK:  llvm.mlir.global internal @[[GLOBAL]]() {{.*}} : i32 {

mlir/include/mlir/Dialect/OpenMP/OpenMPAttrDefs.td

@@ -58,7 +58,8 @@ def FlagsAttr : OpenMP_Attr<"Flags", "flags"> {
 // AtomicControlAttr
 //===----------------------------------------------------------------------===//
 
-// Runtime library flags attribute that holds information for lowering to LLVM.
+// Atomic control attributes holds information about architectural
+// characteristics that are required for lowering of atomic operations.
 def AtomicControlAttr : OpenMP_Attr<"AtomicControl", "atomic_control"> {
   let parameters =
       (ins DefaultValuedParameter<"bool", "false">:$amdgpu_ignore_denormal_mode,