Skip to content

Commit 0b5e9d2

Browse files
anchurajanchuraj
committed
R5: Addressing review comments
1 parent 565d104 commit 0b5e9d2

File tree

16 files changed

+225
-202
lines changed

16 files changed

+225
-202
lines changed

‎flang/include/flang/Frontend/TargetOptions.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ class TargetOptions {
5454
/// Print verbose assembly
5555
bool asmVerbose = false;
5656

57-
/// Atomic control options for AMD gpu
58-
bool ignoreDenormalMode = false;
59-
bool remoteMemory = false;
60-
bool fineGrainedMemory = false;
57+
/// Atomic control options
58+
bool atomicIgnoreDenormalMode = false;
59+
bool atomicRemoteMemory = false;
60+
bool atomicFineGrainedMemory = false;
6161
};
6262

6363
} // end namespace Fortran::frontend

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,21 @@ void setTargetCPU(mlir::ModuleOp mod, llvm::StringRef cpu);
5858
/// Get the target CPU string from the Module or return a null reference.
5959
llvm::StringRef getTargetCPU(mlir::ModuleOp mod);
6060

61-
// Setters and getters for atomic control options.
62-
void setIgnoreDenormalMode(mlir::ModuleOp mod);
63-
bool getIgnoreDenormalMode(mlir::ModuleOp mod);
64-
void setFineGrainedMemory(mlir::ModuleOp mod);
65-
bool getFineGrainedMemory(mlir::ModuleOp mod);
66-
void setRemoteMemory(mlir::ModuleOp mod);
67-
bool getRemoteMemory(mlir::ModuleOp mod);
61+
/// Sets that Denormal Mode can be ignored for lowering of floating point atomic
62+
/// operations.
63+
void setAtomicIgnoreDenormalMode(mlir::ModuleOp mod, bool value);
64+
/// Gets whether Denormal Mode can be ignored or not for lowering of floating
65+
/// point atomic operations.
66+
bool getAtomicIgnoreDenormalMode(mlir::ModuleOp mod);
67+
/// Sets that fine grained memory is used for lowering of atomic operations.
68+
void setAtomicFineGrainedMemory(mlir::ModuleOp mod, bool value);
69+
/// Gets whether fine grained memory is used or not for lowering of atomic
70+
/// operations.
71+
bool getAtomicFineGrainedMemory(mlir::ModuleOp mod);
72+
/// Sets that remote memory is used for lowering of atomic operations.
73+
void setAtomicRemoteMemory(mlir::ModuleOp mod, bool value);
74+
/// Gets whether remote memory is used or not for lowering of atomic operations.
75+
bool getAtomicRemoteMemory(mlir::ModuleOp mod);
6876

6977
/// Set the tune CPU for the module. `cpu` must not be deallocated while
7078
/// module `mod` is still live.

‎flang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -496,13 +496,13 @@ static void parseTargetArgs(TargetOptions &opts, llvm::opt::ArgList &args) {
496496
args.getLastArg(clang::driver::options::OPT_triple))
497497
opts.triple = a->getValue();
498498

499-
opts.ignoreDenormalMode = args.hasFlag(
499+
opts.atomicIgnoreDenormalMode = args.hasFlag(
500500
clang::driver::options::OPT_fatomic_ignore_denormal_mode,
501501
clang::driver::options::OPT_fno_atomic_ignore_denormal_mode, false);
502-
opts.fineGrainedMemory = args.hasFlag(
502+
opts.atomicFineGrainedMemory = args.hasFlag(
503503
clang::driver::options::OPT_fatomic_fine_grained_memory,
504504
clang::driver::options::OPT_fno_atomic_fine_grained_memory, false);
505-
opts.remoteMemory =
505+
opts.atomicRemoteMemory =
506506
args.hasFlag(clang::driver::options::OPT_fatomic_remote_memory,
507507
clang::driver::options::OPT_fno_atomic_remote_memory, false);
508508

‎flang/lib/Lower/Bridge.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6666,12 +6666,10 @@ Fortran::lower::LoweringBridge::LoweringBridge(
66666666
fir::setKindMapping(*module, kindMap);
66676667
fir::setTargetCPU(*module, targetMachine.getTargetCPU());
66686668
fir::setTuneCPU(*module, targetOpts.cpuToTuneFor);
6669-
if (targetOpts.ignoreDenormalMode)
6670-
fir::setIgnoreDenormalMode(*module);
6671-
if (targetOpts.fineGrainedMemory)
6672-
fir::setFineGrainedMemory(*module);
6673-
if (targetOpts.remoteMemory)
6674-
fir::setRemoteMemory(*module);
6669+
fir::setAtomicIgnoreDenormalMode(*module,
6670+
targetOpts.atomicIgnoreDenormalMode);
6671+
fir::setAtomicFineGrainedMemory(*module, targetOpts.atomicFineGrainedMemory);
6672+
fir::setAtomicRemoteMemory(*module, targetOpts.atomicRemoteMemory);
66756673
fir::setTargetFeatures(*module, targetMachine.getTargetFeatureString());
66766674
fir::support::setMLIRDataLayout(*module, targetMachine.createDataLayout());
66776675
fir::setIdent(*module, Fortran::common::getFlangFullVersion());

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

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -88,34 +88,51 @@ void fir::setTuneCPU(mlir::ModuleOp mod, llvm::StringRef cpu) {
8888
mod->setAttr(tuneCpuName, mlir::StringAttr::get(ctx, cpu));
8989
}
9090

91-
static constexpr const char *ignoreDenormalModeName =
92-
"fir.ignore.denormal.mode";
93-
void fir::setIgnoreDenormalMode(mlir::ModuleOp mod) {
94-
auto *ctx = mod.getContext();
95-
mod->setAttr(ignoreDenormalModeName, mlir::UnitAttr::get(ctx));
91+
static constexpr const char *atomicIgnoreDenormalModeName =
92+
"fir.atomic_ignore_denormal_mode";
93+
void fir::setAtomicIgnoreDenormalMode(mlir::ModuleOp mod, bool value) {
94+
if (value) {
95+
auto *ctx = mod.getContext();
96+
mod->setAttr(atomicIgnoreDenormalModeName, mlir::UnitAttr::get(ctx));
97+
} else {
98+
if (mod->hasAttr(atomicIgnoreDenormalModeName))
99+
mod->removeAttr(atomicIgnoreDenormalModeName);
100+
}
96101
}
97102

98-
bool fir::getIgnoreDenormalMode(mlir::ModuleOp mod) {
99-
return mod->hasAttrOfType<mlir::UnitAttr>(ignoreDenormalModeName);
103+
bool fir::getAtomicIgnoreDenormalMode(mlir::ModuleOp mod) {
104+
return mod->hasAttrOfType<mlir::UnitAttr>(atomicIgnoreDenormalModeName);
100105
}
101106

102-
static constexpr const char *fineGrainedMemoryName = "fir.fine.grained.memory";
103-
void fir::setFineGrainedMemory(mlir::ModuleOp mod) {
104-
auto *ctx = mod.getContext();
105-
mod->setAttr(fineGrainedMemoryName, mlir::UnitAttr::get(ctx));
107+
static constexpr const char *atomicFineGrainedMemoryName =
108+
"fir.atomic_fine_grained_memory";
109+
void fir::setAtomicFineGrainedMemory(mlir::ModuleOp mod, bool value) {
110+
if (value) {
111+
auto *ctx = mod.getContext();
112+
mod->setAttr(atomicFineGrainedMemoryName, mlir::UnitAttr::get(ctx));
113+
} else {
114+
if (mod->hasAttr(atomicFineGrainedMemoryName))
115+
mod->removeAttr(atomicFineGrainedMemoryName);
116+
}
106117
}
107118

108-
bool fir::getFineGrainedMemory(mlir::ModuleOp mod) {
109-
return mod->hasAttrOfType<mlir::UnitAttr>(fineGrainedMemoryName);
119+
bool fir::getAtomicFineGrainedMemory(mlir::ModuleOp mod) {
120+
return mod->hasAttrOfType<mlir::UnitAttr>(atomicFineGrainedMemoryName);
110121
}
111-
static constexpr const char *remoteMemoryName = "fir.remote.memory";
112-
void fir::setRemoteMemory(mlir::ModuleOp mod) {
113-
auto *ctx = mod.getContext();
114-
mod->setAttr(remoteMemoryName, mlir::UnitAttr::get(ctx));
122+
static constexpr const char *atomicRemoteMemoryName =
123+
"fir.atomic_remote_memory";
124+
void fir::setAtomicRemoteMemory(mlir::ModuleOp mod, bool value) {
125+
if (value) {
126+
auto *ctx = mod.getContext();
127+
mod->setAttr(atomicRemoteMemoryName, mlir::UnitAttr::get(ctx));
128+
} else {
129+
if (mod->hasAttr(atomicRemoteMemoryName))
130+
mod->removeAttr(atomicRemoteMemoryName);
131+
}
115132
}
116133

117-
bool fir::getRemoteMemory(mlir::ModuleOp mod) {
118-
return mod->hasAttrOfType<mlir::UnitAttr>(remoteMemoryName);
134+
bool fir::getAtomicRemoteMemory(mlir::ModuleOp mod) {
135+
return mod->hasAttrOfType<mlir::UnitAttr>(atomicRemoteMemoryName);
119136
}
120137

121138
llvm::StringRef fir::getTuneCPU(mlir::ModuleOp mod) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ func.func @_QPs() {
619619
%c1_i32 = arith.constant 1 : i32
620620
%1 = arith.addi %arg0, %c1_i32 : i32
621621
omp.yield(%1 : i32)
622-
} {atomic_control = #omp.atomic_control<>}
622+
}
623623
return
624624
}
625625
fir.global internal @_QFsEc : i32 {
@@ -634,7 +634,7 @@ fir.global internal @_QFsEc : i32 {
634634
// CHECK: %[[CONST_1:.*]] = llvm.mlir.constant(1 : i32) : i32
635635
// CHECK: %[[OUT_VAL:.*]] = llvm.add %[[IN_VAL]], %[[CONST_1]] : i32
636636
// CHECK: omp.yield(%[[OUT_VAL]] : i32)
637-
// CHECK: } {atomic_control = #omp.atomic_control<>}
637+
// CHECK: }
638638
// CHECK: llvm.return
639639
// CHECK: }
640640
// CHECK: llvm.mlir.global internal @[[GLOBAL]]() {{.*}} : i32 {

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,21 @@ class OpenMP_Attr<string name, string attrMnemonic, list<Trait> traits = [],
2222
let mnemonic = attrMnemonic;
2323
}
2424

25+
//===----------------------------------------------------------------------===//
26+
// AtomicControlAttr
27+
//===----------------------------------------------------------------------===//
28+
29+
// Atomic control attributes hold information about architectural
30+
// characteristics which are required for lowering atomic operations.
31+
def AtomicControlAttr : OpenMP_Attr<"AtomicControl", "atomic_control"> {
32+
let parameters =
33+
(ins DefaultValuedParameter<"bool", "false">:$ignore_denormal_mode,
34+
DefaultValuedParameter<"bool", "false">:$fine_grained_memory,
35+
DefaultValuedParameter<"bool", "false">:$remote_memory);
36+
37+
let assemblyFormat = "`<` struct(params) `>`";
38+
}
39+
2540
//===----------------------------------------------------------------------===//
2641
// DeclareTargetAttr
2742
//===----------------------------------------------------------------------===//
@@ -54,21 +69,6 @@ def FlagsAttr : OpenMP_Attr<"Flags", "flags"> {
5469
let assemblyFormat = "`<` struct(params) `>`";
5570
}
5671

57-
//===----------------------------------------------------------------------===//
58-
// AtomicControlAttr
59-
//===----------------------------------------------------------------------===//
60-
61-
// Atomic control attributes hold information about architectural
62-
// characteristics which are required for lowering atomic operations.
63-
def AtomicControlAttr : OpenMP_Attr<"AtomicControl", "atomic_control"> {
64-
let parameters =
65-
(ins DefaultValuedParameter<"bool", "false">:$ignore_denormal_mode,
66-
DefaultValuedParameter<"bool", "false">:$fine_grained_memory,
67-
DefaultValuedParameter<"bool", "false">:$remote_memory);
68-
69-
let assemblyFormat = "`<` struct(params) `>`";
70-
}
71-
7272
//===----------------------------------------------------------------------===//
7373
// TaskDependArrayAttr
7474
//===----------------------------------------------------------------------===//

‎mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1546,7 +1546,7 @@ def AtomicUpdateOp : OpenMP_Op<"atomic.update", traits = [
15461546
let arguments = !con(
15471547
(ins Arg<OpenMP_PointerLikeType,
15481548
"Address of variable to be updated", [MemRead, MemWrite]>:$x,
1549-
AtomicControlAttr:$atomic_control),
1549+
OptionalAttr<AtomicControlAttr>:$atomic_control),
15501550
clausesArgs);
15511551

15521552
// Override region definition.

‎mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func.func @atomic_update() {
133133
%1 = arith.constant 1 : i32
134134
%2 = arith.addi %arg0, %1 : i32
135135
omp.yield(%2 : i32)
136-
} {atomic_control = #omp.atomic_control<>}
136+
}
137137
return
138138
}
139139
llvm.mlir.global internal @_QFsEc() : i32 {

‎mlir/test/Dialect/OpenMP/canonicalize.mlir

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ func.func @update_no_op(%x : memref<i32>) {
44
omp.atomic.update %x : memref<i32> {
55
^bb0(%xval : i32):
66
omp.yield(%xval : i32)
7-
} {atomic_control = #omp.atomic_control<>}
7+
}
88
return
99
}
1010

@@ -17,7 +17,7 @@ func.func @update_write_op(%x : memref<i32>, %value: i32) {
1717
omp.atomic.update %x : memref<i32> {
1818
^bb0(%xval : i32):
1919
omp.yield(%value : i32)
20-
} {atomic_control = #omp.atomic_control<>}
20+
}
2121
return
2222
}
2323

@@ -33,7 +33,7 @@ func.func @update_normal(%x : memref<i32>, %value: i32) {
3333
^bb0(%xval : i32):
3434
%newval = arith.addi %xval, %value : i32
3535
omp.yield(%newval : i32)
36-
} {atomic_control = #omp.atomic_control<>}
36+
}
3737
return
3838
}
3939

@@ -50,7 +50,7 @@ func.func @update_unnecessary_computations(%x: memref<i32>) {
5050
^bb0(%xval: i32):
5151
%newval = arith.addi %xval, %c0 : i32
5252
omp.yield(%newval: i32)
53-
} {atomic_control = #omp.atomic_control<>}
53+
}
5454
return
5555
}
5656

@@ -65,7 +65,7 @@ func.func @update_unnecessary_computations(%x: memref<i32>) {
6565
^bb0(%xval: i32):
6666
%newval = arith.muli %xval, %c0 : i32
6767
omp.yield(%newval: i32)
68-
} {atomic_control = #omp.atomic_control<>}
68+
}
6969
return
7070
}
7171

0 commit comments

Comments
 (0)