[llvm-branch-commits] [flang] [mlir] [mlir][flang][openmp] Rework wsloop reduction operations (PR #80019)
https://github.com/kiranchandramohan ready_for_review https://github.com/llvm/llvm-project/pull/80019 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/18.x: [AArch64][GlobalISel] Fail legalization for unknown libcalls. (#81873) (PR #82103)
https://github.com/kiranchandramohan approved this pull request. https://github.com/llvm/llvm-project/pull/82103 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Convert DataSharingProcessor to omp::Clause (PR #81629)
@@ -135,138 +133,135 @@ void DataSharingProcessor::insertBarrier() {
void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
bool cmpCreated = false;
mlir::OpBuilder::InsertPoint localInsPt = firOpBuilder.saveInsertionPoint();
- for (const Fortran::parser::OmpClause &clause : opClauseList.v) {
kiranchandramohan wrote:
Are there code changes here or is it only the replacement of
`parser::OmpClause` with `omp::Clause`?
https://github.com/llvm/llvm-project/pull/81629
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Convert unique clauses in ClauseProcessor (PR #81622)
https://github.com/kiranchandramohan approved this pull request. Mostly mechanical changes. LGTM. Please wait for @skatrak https://github.com/llvm/llvm-project/pull/81622 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Convert DataSharingProcessor to omp::Clause (PR #81629)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/81629 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][LLVM] erase call mappings in forgetMapping() (PR #84955)
https://github.com/kiranchandramohan approved this pull request. LGTM. Please wait a day incase others have comments. https://github.com/llvm/llvm-project/pull/84955 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang] support fir.alloca operations inside of omp reduction ops (PR #84952)
@@ -410,8 +410,15 @@ class FIROpConversion : public
mlir::ConvertOpToLLVMPattern {
mlir::ConversionPatternRewriter &rewriter) const {
auto thisPt = rewriter.saveInsertionPoint();
mlir::Operation *parentOp = rewriter.getInsertionBlock()->getParentOp();
-mlir::Block *insertBlock = getBlockForAllocaInsert(parentOp);
-rewriter.setInsertionPointToStart(insertBlock);
+if (mlir::isa(parentOp)) {
+ // ReductionDeclareOp has multiple child regions. We want to get the
first
+ // block of whichever of those regions we are currently in
+ mlir::Region *parentRegion = rewriter.getInsertionBlock()->getParent();
+ rewriter.setInsertionPointToStart(&parentRegion->front());
+} else {
+ mlir::Block *insertBlock = getBlockForAllocaInsert(parentOp);
+ rewriter.setInsertionPointToStart(insertBlock);
+}
kiranchandramohan wrote:
Is it not sufficient to modify `getBlockForAllocaInsert` to handle
ReductionDeclareOp?
https://github.com/llvm/llvm-project/pull/84952
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang] support fir.alloca operations inside of omp reduction ops (PR #84952)
https://github.com/kiranchandramohan approved this pull request. LG. Can we add a test? https://github.com/llvm/llvm-project/pull/84952 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][CodeGen] Run PreCGRewrite on omp reduction declare ops (PR #84954)
https://github.com/kiranchandramohan approved this pull request. LG. Please wait for @clementval. https://github.com/llvm/llvm-project/pull/84954 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][CodeGen] Run PreCGRewrite on omp reduction declare ops (PR #84954)
@@ -18,6 +18,7 @@ #include "flang/Optimizer/Dialect/FIROps.h" #include "flang/Optimizer/Dialect/FIRType.h" #include "flang/Optimizer/Dialect/Support/FIRContext.h" +#include "mlir/Dialect/OpenMP/OpenMPDialect.h" kiranchandramohan wrote: The header is not necessary now? https://github.com/llvm/llvm-project/pull/84954 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][LLVM] erase call mappings in forgetMapping() (PR #84955)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/84955 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][LLVM] erase call mappings in forgetMapping() (PR #84955)
@@ -0,0 +1,47 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// Test that we don't crash when there is a call operation in the combiner
+
+omp.reduction.declare @add_f32 : f32
+init {
+^bb0(%arg: f32):
+ %0 = llvm.mlir.constant(0.0 : f32) : f32
+ omp.yield (%0 : f32)
+}
+combiner {
+^bb1(%arg0: f32, %arg1: f32):
+// test this call here:
+ llvm.call @test_call() : () -> ()
+ %1 = llvm.fadd %arg0, %arg1 : f32
+ omp.yield (%1 : f32)
+}
+atomic {
+^bb2(%arg2: !llvm.ptr, %arg3: !llvm.ptr):
+ %2 = llvm.load %arg3 : !llvm.ptr -> f32
+ llvm.atomicrmw fadd %arg2, %2 monotonic : !llvm.ptr, f32
+ omp.yield
+}
+
+llvm.func @simple_reduction(%lb : i64, %ub : i64, %step : i64) {
+ %c1 = llvm.mlir.constant(1 : i32) : i32
+ %0 = llvm.alloca %c1 x i32 : (i32) -> !llvm.ptr
+ omp.parallel {
+omp.wsloop reduction(@add_f32 %0 -> %prv : !llvm.ptr)
+for (%iv) : i64 = (%lb) to (%ub) step (%step) {
+ %1 = llvm.mlir.constant(2.0 : f32) : f32
+ %2 = llvm.load %prv : !llvm.ptr -> f32
+ %3 = llvm.fadd %1, %2 : f32
+ llvm.store %3, %prv : f32, !llvm.ptr
+ omp.yield
+}
+omp.terminator
+ }
+ llvm.return
+}
+
+llvm.func @test_call() -> ()
kiranchandramohan wrote:
> Why do we need any OpenMp constructs here?
`forgetMapping` is only used in the OpenMP translation.
https://github.com/llvm/llvm-project/pull/84955
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][LLVM] erase call mappings in forgetMapping() (PR #84955)
@@ -0,0 +1,47 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// Test that we don't crash when there is a call operation in the combiner
+
+omp.reduction.declare @add_f32 : f32
+init {
+^bb0(%arg: f32):
+ %0 = llvm.mlir.constant(0.0 : f32) : f32
+ omp.yield (%0 : f32)
+}
+combiner {
+^bb1(%arg0: f32, %arg1: f32):
+// test this call here:
+ llvm.call @test_call() : () -> ()
+ %1 = llvm.fadd %arg0, %arg1 : f32
+ omp.yield (%1 : f32)
+}
+atomic {
+^bb2(%arg2: !llvm.ptr, %arg3: !llvm.ptr):
+ %2 = llvm.load %arg3 : !llvm.ptr -> f32
+ llvm.atomicrmw fadd %arg2, %2 monotonic : !llvm.ptr, f32
+ omp.yield
+}
kiranchandramohan wrote:
Can the atomic region be removed?
https://github.com/llvm/llvm-project/pull/84955
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][LLVM] erase call mappings in forgetMapping() (PR #84955)
https://github.com/kiranchandramohan commented: Some minor comments about the test. https://github.com/llvm/llvm-project/pull/84955 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][LLVM] erase call mappings in forgetMapping() (PR #84955)
@@ -0,0 +1,47 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// Test that we don't crash when there is a call operation in the combiner
+
+omp.reduction.declare @add_f32 : f32
+init {
+^bb0(%arg: f32):
+ %0 = llvm.mlir.constant(0.0 : f32) : f32
+ omp.yield (%0 : f32)
+}
+combiner {
+^bb1(%arg0: f32, %arg1: f32):
+// test this call here:
+ llvm.call @test_call() : () -> ()
+ %1 = llvm.fadd %arg0, %arg1 : f32
+ omp.yield (%1 : f32)
+}
+atomic {
+^bb2(%arg2: !llvm.ptr, %arg3: !llvm.ptr):
+ %2 = llvm.load %arg3 : !llvm.ptr -> f32
+ llvm.atomicrmw fadd %arg2, %2 monotonic : !llvm.ptr, f32
+ omp.yield
+}
+
+llvm.func @simple_reduction(%lb : i64, %ub : i64, %step : i64) {
+ %c1 = llvm.mlir.constant(1 : i32) : i32
+ %0 = llvm.alloca %c1 x i32 : (i32) -> !llvm.ptr
+ omp.parallel {
+omp.wsloop reduction(@add_f32 %0 -> %prv : !llvm.ptr)
kiranchandramohan wrote:
Can the reduction be moved to `omp.parallel`? And the `omp.wsloop` removed?
https://github.com/llvm/llvm-project/pull/84955
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/84958 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -92,10 +93,42 @@ std::string
ReductionProcessor::getReductionName(llvm::StringRef name,
if (isByRef)
byrefAddition = "_byref";
- return (llvm::Twine(name) +
- (ty.isIntOrIndex() ? llvm::Twine("_i_") : llvm::Twine("_f_")) +
- llvm::Twine(ty.getIntOrFloatBitWidth()) + byrefAddition)
- .str();
+ if (fir::isa_trivial(ty))
+return (llvm::Twine(name) +
+(ty.isIntOrIndex() ? llvm::Twine("_i_") : llvm::Twine("_f_")) +
+llvm::Twine(ty.getIntOrFloatBitWidth()) + byrefAddition)
+.str();
+
+ // creates a name like reduction_i_64_box_ux4x3
+ if (auto boxTy = mlir::dyn_cast_or_null(ty)) {
+// TODO: support for allocatable boxes:
+// !fir.box>>
+fir::SequenceType seqTy = fir::unwrapRefType(boxTy.getEleTy())
+ .dyn_cast_or_null();
+if (!seqTy)
+ return {};
+
+std::string prefix = getReductionName(
+name, fir::unwrapSeqOrBoxedSeqType(ty), /*isByRef=*/false);
+if (prefix.empty())
+ return {};
+std::stringstream tyStr;
+tyStr << prefix << "_box_";
+bool first = true;
+for (std::int64_t extent : seqTy.getShape()) {
+ if (first)
+first = false;
+ else
+tyStr << "x";
+ if (extent == seqTy.getUnknownExtent())
+tyStr << 'u'; // I'm not sure that '?' is safe in symbol names
+ else
+tyStr << extent;
+}
+return (tyStr.str() + byrefAddition).str();
+ }
+
+ return {};
kiranchandramohan wrote:
There is a getTypeAsString
(https://github.com/llvm/llvm-project/blob/07b18c5e1b7a8ac9347f945da5ffaecc4515f391/flang/lib/Optimizer/Dialect/FIRType.cpp#L520).
Can that be used? If it changes existing reduction names then we can move to
this in a separate patch.
https://github.com/llvm/llvm-project/pull/84958
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -283,13 +316,166 @@ mlir::Value ReductionProcessor::createScalarCombiner(
return reductionOp;
}
+/// Create reduction combiner region for reduction variables which are boxed
+/// arrays
+static void genBoxCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
+ ReductionProcessor::ReductionIdentifier redId,
+ fir::BaseBoxType boxTy, mlir::Value lhs,
+ mlir::Value rhs) {
+ fir::SequenceType seqTy =
+ mlir::dyn_cast_or_null(boxTy.getEleTy());
+ // TODO: support allocatable arrays: !fir.box>>
+ if (!seqTy || seqTy.hasUnknownShape())
+TODO(loc, "Unsupported boxed type in OpenMP reduction");
+
+ // load fir.ref>
+ mlir::Value lhsAddr = lhs;
+ lhs = builder.create(loc, lhs);
+ rhs = builder.create(loc, rhs);
+
+ const unsigned rank = seqTy.getDimension();
+ llvm::SmallVector extents;
+ extents.reserve(rank);
+ llvm::SmallVector lbAndExtents;
+ lbAndExtents.reserve(rank * 2);
+
+ // Get box lowerbounds and extents:
+ mlir::Type idxTy = builder.getIndexType();
+ for (unsigned i = 0; i < rank; ++i) {
+// TODO: ideally we want to hoist box reads out of the critical section.
+// We could do this by having box dimensions in block arguments like
+// OpenACC does
+mlir::Value dim = builder.createIntegerConstant(loc, idxTy, i);
+auto dimInfo =
+builder.create(loc, idxTy, idxTy, idxTy, lhs, dim);
+extents.push_back(dimInfo.getExtent());
+lbAndExtents.push_back(dimInfo.getLowerBound());
+lbAndExtents.push_back(dimInfo.getExtent());
+ }
+
+ auto shapeShiftTy = fir::ShapeShiftType::get(builder.getContext(), rank);
+ auto shapeShift =
+ builder.create(loc, shapeShiftTy, lbAndExtents);
+
+ // Iterate over array elements, applying the equivalent scalar reduction:
+
+ // A hlfir::elemental here gets inlined with a temporary so create the
+ // loop nest directly.
+ // This function already controls all of the code in this region so we
+ // know this won't miss any opportuinties for clever elemental inlining
+ hlfir::LoopNest nest =
+ hlfir::genLoopNest(loc, builder, extents, /*isUnordered=*/true);
+ builder.setInsertionPointToStart(nest.innerLoop.getBody());
+ mlir::Type refTy = fir::ReferenceType::get(seqTy.getEleTy());
+ auto lhsEleAddr = builder.create(
+ loc, refTy, lhs, shapeShift, /*slice=*/mlir::Value{},
+ nest.oneBasedIndices, /*typeparms=*/mlir::ValueRange{});
+ auto rhsEleAddr = builder.create(
+ loc, refTy, rhs, shapeShift, /*slice=*/mlir::Value{},
+ nest.oneBasedIndices, /*typeparms=*/mlir::ValueRange{});
+ auto lhsEle = builder.create(loc, lhsEleAddr);
+ auto rhsEle = builder.create(loc, rhsEleAddr);
+ mlir::Value scalarReduction = ReductionProcessor::createScalarCombiner(
+ builder, loc, redId, refTy, lhsEle, rhsEle);
+ builder.create(loc, scalarReduction, lhsEleAddr);
+
+ builder.setInsertionPointAfter(nest.outerLoop);
+ builder.create(loc, lhsAddr);
+}
+
+// generate combiner region for reduction operations
+static void genCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
+ReductionProcessor::ReductionIdentifier redId,
+mlir::Type ty, mlir::Value lhs, mlir::Value rhs,
+bool isByRef) {
+ ty = fir::unwrapRefType(ty);
+
+ if (fir::isa_trivial(ty)) {
+mlir::Value lhsLoaded = builder.loadIfRef(loc, lhs);
+mlir::Value rhsLoaded = builder.loadIfRef(loc, rhs);
+
+mlir::Value result = ReductionProcessor::createScalarCombiner(
+builder, loc, redId, ty, lhsLoaded, rhsLoaded);
+if (isByRef) {
+ builder.create(loc, result, lhs);
+ builder.create(loc, lhs);
+} else {
+ builder.create(loc, result);
+}
+return;
+ }
+ // all arrays should have been boxed
+ if (auto boxTy = mlir::dyn_cast(ty)) {
+genBoxCombiner(builder, loc, redId, boxTy, lhs, rhs);
+return;
+ }
+
+ TODO(loc, "OpenMP genCombiner for unsupported reduction variable type");
+}
+
+static mlir::Value
+createReductionInitRegion(fir::FirOpBuilder &builder, mlir::Location loc,
+ const ReductionProcessor::ReductionIdentifier redId,
+ mlir::Type type, bool isByRef) {
+ mlir::Type ty = fir::unwrapRefType(type);
+ mlir::Value initValue = ReductionProcessor::getReductionInitValue(
+ loc, fir::unwrapSeqOrBoxedSeqType(ty), redId, builder);
+
+ if (fir::isa_trivial(ty)) {
+if (isByRef) {
+ mlir::Value alloca = builder.create(loc, ty);
+ builder.createStoreWithConvert(loc, initValue, alloca);
+ return alloca;
+}
+// by val
+return initValue;
+ }
+
+ // all arrays are boxed
+ if (auto boxTy = mlir::dyn_cast_or_null(ty)) {
+assert(isByRef && "passing arrays by value is unsupported");
+// TODO: support allocatable arrays: !fir.box>>
+mlir::Type innerTy = fir::extractSequenceType(boxTy);
+if (!mlir::isa(innerTy))
+
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -0,0 +1,74 @@ +! RUN: bbc -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s +! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s + +program reduce +integer, dimension(3) :: i = 0 + +!$omp parallel reduction(+:i) +i(1) = 1 +i(2) = 2 +i(3) = 3 kiranchandramohan wrote: Is the test written to deliberately not use the reduction operation in the body? https://github.com/llvm/llvm-project/pull/84958 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
https://github.com/kiranchandramohan commented: Nice work. Have a few minor comments. https://github.com/llvm/llvm-project/pull/84958 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -283,13 +316,166 @@ mlir::Value ReductionProcessor::createScalarCombiner(
return reductionOp;
}
+/// Create reduction combiner region for reduction variables which are boxed
+/// arrays
+static void genBoxCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
+ ReductionProcessor::ReductionIdentifier redId,
+ fir::BaseBoxType boxTy, mlir::Value lhs,
+ mlir::Value rhs) {
+ fir::SequenceType seqTy =
+ mlir::dyn_cast_or_null(boxTy.getEleTy());
+ // TODO: support allocatable arrays: !fir.box>>
+ if (!seqTy || seqTy.hasUnknownShape())
+TODO(loc, "Unsupported boxed type in OpenMP reduction");
+
+ // load fir.ref>
+ mlir::Value lhsAddr = lhs;
+ lhs = builder.create(loc, lhs);
+ rhs = builder.create(loc, rhs);
+
+ const unsigned rank = seqTy.getDimension();
+ llvm::SmallVector extents;
+ extents.reserve(rank);
+ llvm::SmallVector lbAndExtents;
+ lbAndExtents.reserve(rank * 2);
+
+ // Get box lowerbounds and extents:
+ mlir::Type idxTy = builder.getIndexType();
+ for (unsigned i = 0; i < rank; ++i) {
+// TODO: ideally we want to hoist box reads out of the critical section.
+// We could do this by having box dimensions in block arguments like
+// OpenACC does
+mlir::Value dim = builder.createIntegerConstant(loc, idxTy, i);
+auto dimInfo =
+builder.create(loc, idxTy, idxTy, idxTy, lhs, dim);
+extents.push_back(dimInfo.getExtent());
+lbAndExtents.push_back(dimInfo.getLowerBound());
+lbAndExtents.push_back(dimInfo.getExtent());
+ }
+
+ auto shapeShiftTy = fir::ShapeShiftType::get(builder.getContext(), rank);
+ auto shapeShift =
+ builder.create(loc, shapeShiftTy, lbAndExtents);
+
+ // Iterate over array elements, applying the equivalent scalar reduction:
+
+ // A hlfir::elemental here gets inlined with a temporary so create the
+ // loop nest directly.
+ // This function already controls all of the code in this region so we
+ // know this won't miss any opportuinties for clever elemental inlining
+ hlfir::LoopNest nest =
+ hlfir::genLoopNest(loc, builder, extents, /*isUnordered=*/true);
+ builder.setInsertionPointToStart(nest.innerLoop.getBody());
+ mlir::Type refTy = fir::ReferenceType::get(seqTy.getEleTy());
+ auto lhsEleAddr = builder.create(
+ loc, refTy, lhs, shapeShift, /*slice=*/mlir::Value{},
+ nest.oneBasedIndices, /*typeparms=*/mlir::ValueRange{});
+ auto rhsEleAddr = builder.create(
+ loc, refTy, rhs, shapeShift, /*slice=*/mlir::Value{},
+ nest.oneBasedIndices, /*typeparms=*/mlir::ValueRange{});
+ auto lhsEle = builder.create(loc, lhsEleAddr);
+ auto rhsEle = builder.create(loc, rhsEleAddr);
+ mlir::Value scalarReduction = ReductionProcessor::createScalarCombiner(
+ builder, loc, redId, refTy, lhsEle, rhsEle);
+ builder.create(loc, scalarReduction, lhsEleAddr);
+
+ builder.setInsertionPointAfter(nest.outerLoop);
+ builder.create(loc, lhsAddr);
+}
+
+// generate combiner region for reduction operations
+static void genCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
+ReductionProcessor::ReductionIdentifier redId,
+mlir::Type ty, mlir::Value lhs, mlir::Value rhs,
+bool isByRef) {
+ ty = fir::unwrapRefType(ty);
+
+ if (fir::isa_trivial(ty)) {
+mlir::Value lhsLoaded = builder.loadIfRef(loc, lhs);
+mlir::Value rhsLoaded = builder.loadIfRef(loc, rhs);
+
+mlir::Value result = ReductionProcessor::createScalarCombiner(
+builder, loc, redId, ty, lhsLoaded, rhsLoaded);
+if (isByRef) {
+ builder.create(loc, result, lhs);
+ builder.create(loc, lhs);
+} else {
+ builder.create(loc, result);
+}
+return;
+ }
+ // all arrays should have been boxed
+ if (auto boxTy = mlir::dyn_cast(ty)) {
+genBoxCombiner(builder, loc, redId, boxTy, lhs, rhs);
+return;
+ }
+
+ TODO(loc, "OpenMP genCombiner for unsupported reduction variable type");
+}
+
+static mlir::Value
+createReductionInitRegion(fir::FirOpBuilder &builder, mlir::Location loc,
+ const ReductionProcessor::ReductionIdentifier redId,
+ mlir::Type type, bool isByRef) {
+ mlir::Type ty = fir::unwrapRefType(type);
+ mlir::Value initValue = ReductionProcessor::getReductionInitValue(
+ loc, fir::unwrapSeqOrBoxedSeqType(ty), redId, builder);
+
+ if (fir::isa_trivial(ty)) {
+if (isByRef) {
+ mlir::Value alloca = builder.create(loc, ty);
+ builder.createStoreWithConvert(loc, initValue, alloca);
+ return alloca;
+}
+// by val
+return initValue;
+ }
+
+ // all arrays are boxed
+ if (auto boxTy = mlir::dyn_cast_or_null(ty)) {
+assert(isByRef && "passing arrays by value is unsupported");
+// TODO: support allocatable arrays: !fir.box>>
+mlir::Type innerTy = fir::extractSequenceType(boxTy);
+if (!mlir::isa(innerTy))
+
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -465,8 +642,8 @@ void ReductionProcessor::addReductionDecl( if (auto declOp = symVal.getDefiningOp()) symVal = declOp.getBase(); auto redType = symVal.getType().cast(); -assert(redType.getEleTy().isIntOrIndexOrFloat() && - "Unsupported reduction type"); +if (!redType.getEleTy().isIntOrIndexOrFloat()) + TODO(currentLocation, "User Defined Reduction on arrays"); kiranchandramohan wrote: Nit: We don't support complex type now. So this could be due to being a complex type as well. https://github.com/llvm/llvm-project/pull/84958 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -92,10 +93,42 @@ std::string
ReductionProcessor::getReductionName(llvm::StringRef name,
if (isByRef)
byrefAddition = "_byref";
- return (llvm::Twine(name) +
- (ty.isIntOrIndex() ? llvm::Twine("_i_") : llvm::Twine("_f_")) +
- llvm::Twine(ty.getIntOrFloatBitWidth()) + byrefAddition)
- .str();
+ if (fir::isa_trivial(ty))
+return (llvm::Twine(name) +
+(ty.isIntOrIndex() ? llvm::Twine("_i_") : llvm::Twine("_f_")) +
+llvm::Twine(ty.getIntOrFloatBitWidth()) + byrefAddition)
+.str();
+
+ // creates a name like reduction_i_64_box_ux4x3
+ if (auto boxTy = mlir::dyn_cast_or_null(ty)) {
+// TODO: support for allocatable boxes:
+// !fir.box>>
+fir::SequenceType seqTy = fir::unwrapRefType(boxTy.getEleTy())
+ .dyn_cast_or_null();
+if (!seqTy)
+ return {};
+
+std::string prefix = getReductionName(
+name, fir::unwrapSeqOrBoxedSeqType(ty), /*isByRef=*/false);
+if (prefix.empty())
+ return {};
+std::stringstream tyStr;
+tyStr << prefix << "_box_";
+bool first = true;
+for (std::int64_t extent : seqTy.getShape()) {
+ if (first)
+first = false;
+ else
+tyStr << "x";
+ if (extent == seqTy.getUnknownExtent())
+tyStr << 'u'; // I'm not sure that '?' is safe in symbol names
+ else
+tyStr << extent;
+}
+return (tyStr.str() + byrefAddition).str();
+ }
+
+ return {};
kiranchandramohan wrote:
OK, that is fine.
https://github.com/llvm/llvm-project/pull/84958
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -0,0 +1,74 @@ +! RUN: bbc -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s +! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s + +program reduce +integer, dimension(3) :: i = 0 + +!$omp parallel reduction(+:i) +i(1) = 1 +i(2) = 2 +i(3) = 3 kiranchandramohan wrote: Yes, it would be good to have the test match the default way that it will be used. https://github.com/llvm/llvm-project/pull/84958 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -283,13 +316,166 @@ mlir::Value ReductionProcessor::createScalarCombiner(
return reductionOp;
}
+/// Create reduction combiner region for reduction variables which are boxed
+/// arrays
+static void genBoxCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
+ ReductionProcessor::ReductionIdentifier redId,
+ fir::BaseBoxType boxTy, mlir::Value lhs,
+ mlir::Value rhs) {
+ fir::SequenceType seqTy =
+ mlir::dyn_cast_or_null(boxTy.getEleTy());
+ // TODO: support allocatable arrays: !fir.box>>
+ if (!seqTy || seqTy.hasUnknownShape())
+TODO(loc, "Unsupported boxed type in OpenMP reduction");
+
+ // load fir.ref>
+ mlir::Value lhsAddr = lhs;
+ lhs = builder.create(loc, lhs);
+ rhs = builder.create(loc, rhs);
+
+ const unsigned rank = seqTy.getDimension();
+ llvm::SmallVector extents;
+ extents.reserve(rank);
+ llvm::SmallVector lbAndExtents;
+ lbAndExtents.reserve(rank * 2);
+
+ // Get box lowerbounds and extents:
+ mlir::Type idxTy = builder.getIndexType();
+ for (unsigned i = 0; i < rank; ++i) {
+// TODO: ideally we want to hoist box reads out of the critical section.
+// We could do this by having box dimensions in block arguments like
+// OpenACC does
+mlir::Value dim = builder.createIntegerConstant(loc, idxTy, i);
+auto dimInfo =
+builder.create(loc, idxTy, idxTy, idxTy, lhs, dim);
+extents.push_back(dimInfo.getExtent());
+lbAndExtents.push_back(dimInfo.getLowerBound());
+lbAndExtents.push_back(dimInfo.getExtent());
+ }
+
+ auto shapeShiftTy = fir::ShapeShiftType::get(builder.getContext(), rank);
+ auto shapeShift =
+ builder.create(loc, shapeShiftTy, lbAndExtents);
+
+ // Iterate over array elements, applying the equivalent scalar reduction:
+
+ // A hlfir::elemental here gets inlined with a temporary so create the
+ // loop nest directly.
+ // This function already controls all of the code in this region so we
+ // know this won't miss any opportuinties for clever elemental inlining
+ hlfir::LoopNest nest =
+ hlfir::genLoopNest(loc, builder, extents, /*isUnordered=*/true);
+ builder.setInsertionPointToStart(nest.innerLoop.getBody());
+ mlir::Type refTy = fir::ReferenceType::get(seqTy.getEleTy());
+ auto lhsEleAddr = builder.create(
+ loc, refTy, lhs, shapeShift, /*slice=*/mlir::Value{},
+ nest.oneBasedIndices, /*typeparms=*/mlir::ValueRange{});
+ auto rhsEleAddr = builder.create(
+ loc, refTy, rhs, shapeShift, /*slice=*/mlir::Value{},
+ nest.oneBasedIndices, /*typeparms=*/mlir::ValueRange{});
+ auto lhsEle = builder.create(loc, lhsEleAddr);
+ auto rhsEle = builder.create(loc, rhsEleAddr);
+ mlir::Value scalarReduction = ReductionProcessor::createScalarCombiner(
+ builder, loc, redId, refTy, lhsEle, rhsEle);
+ builder.create(loc, scalarReduction, lhsEleAddr);
+
+ builder.setInsertionPointAfter(nest.outerLoop);
+ builder.create(loc, lhsAddr);
+}
+
+// generate combiner region for reduction operations
+static void genCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
+ReductionProcessor::ReductionIdentifier redId,
+mlir::Type ty, mlir::Value lhs, mlir::Value rhs,
+bool isByRef) {
+ ty = fir::unwrapRefType(ty);
+
+ if (fir::isa_trivial(ty)) {
+mlir::Value lhsLoaded = builder.loadIfRef(loc, lhs);
+mlir::Value rhsLoaded = builder.loadIfRef(loc, rhs);
+
+mlir::Value result = ReductionProcessor::createScalarCombiner(
+builder, loc, redId, ty, lhsLoaded, rhsLoaded);
+if (isByRef) {
+ builder.create(loc, result, lhs);
+ builder.create(loc, lhs);
+} else {
+ builder.create(loc, result);
+}
+return;
+ }
+ // all arrays should have been boxed
+ if (auto boxTy = mlir::dyn_cast(ty)) {
+genBoxCombiner(builder, loc, redId, boxTy, lhs, rhs);
+return;
+ }
+
+ TODO(loc, "OpenMP genCombiner for unsupported reduction variable type");
+}
+
+static mlir::Value
+createReductionInitRegion(fir::FirOpBuilder &builder, mlir::Location loc,
+ const ReductionProcessor::ReductionIdentifier redId,
+ mlir::Type type, bool isByRef) {
+ mlir::Type ty = fir::unwrapRefType(type);
+ mlir::Value initValue = ReductionProcessor::getReductionInitValue(
+ loc, fir::unwrapSeqOrBoxedSeqType(ty), redId, builder);
+
+ if (fir::isa_trivial(ty)) {
+if (isByRef) {
+ mlir::Value alloca = builder.create(loc, ty);
+ builder.createStoreWithConvert(loc, initValue, alloca);
+ return alloca;
+}
+// by val
+return initValue;
+ }
+
+ // all arrays are boxed
+ if (auto boxTy = mlir::dyn_cast_or_null(ty)) {
+assert(isByRef && "passing arrays by value is unsupported");
+// TODO: support allocatable arrays: !fir.box>>
+mlir::Type innerTy = fir::extractSequenceType(boxTy);
+if (!mlir::isa(innerTy))
+
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
https://github.com/kiranchandramohan approved this pull request. For compile-time constant bounds we should switch to a non-box version sometime later. Thanks for the changes. https://github.com/llvm/llvm-project/pull/84958 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
@@ -1108,6 +1108,35 @@ hlfir::createTempFromMold(mlir::Location loc,
fir::FirOpBuilder &builder,
return {hlfir::Entity{declareOp.getBase()}, isHeapAlloc};
}
+hlfir::Entity hlfir::createStackTempFromMold(mlir::Location loc,
kiranchandramohan wrote:
@ergawy FYI: This patch has a `createStackTempFromMold` which might be helpful.
https://github.com/llvm/llvm-project/pull/84958
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] lower simple array reductions (PR #84958)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/84958 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] simplify getReductionName (PR #85666)
https://github.com/kiranchandramohan approved this pull request. > The idea was suggested by Kiran: > https://github.com/llvm/llvm-project/pull/84958#discussion_r1527604388 It was implement by @clementval and recommended by him in other patches. I was just forwarding the suggestion. Thanks for the patch. LGTM. https://github.com/llvm/llvm-project/pull/85666 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
kiranchandramohan wrote: I am away this week, will come back to this next week. https://github.com/llvm/llvm-project/pull/85989 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [MLIR][OpenMP] NFC: Remove LoopControl parsing/printing code (PR #88909)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/88909 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/85989 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
@@ -2012,34 +2012,87 @@ void OmpAttributeVisitor::Post(const parser::Name
&name) {
}
}
}
-std::vector defaultDSASymbols;
+
+// Implicitly determined DSAs
+// OMP 5.2 5.1.1 - Variables Referenced in a Construct
+Symbol *lastDeclSymbol = nullptr;
+std::optional prevDSA;
for (int dirDepth{0}; dirDepth < (int)dirContext_.size(); ++dirDepth) {
DirContext &dirContext = dirContext_[dirDepth];
- bool hasDataSharingAttr{false};
+ std::optional dsa;
+
for (auto symMap : dirContext.objectWithDSA) {
// if the `symbol` already has a data-sharing attribute
if (symMap.first->name() == name.symbol->name()) {
- hasDataSharingAttr = true;
+ dsa = symMap.second;
break;
}
}
- if (hasDataSharingAttr) {
-if (defaultDSASymbols.size())
- symbol = &MakeAssocSymbol(symbol->name(), *defaultDSASymbols.back(),
+
+ // When handling each implicit rule, either a new private symbol is
+ // declared or the last declared symbol is used.
+ // In the latter case, it's necessary to insert a new symbol in the scope
+ // being processed, associated with the last declared symbol, to avoid
+ // "inheriting" the enclosing context's symbol and its flags.
kiranchandramohan wrote:
Can you expand the `latter case`? On a first look it feels like we should be OK
with just inheriting the enclosing context's symbol and its flags. Probably you
have to say something like we have to create a new symbol to capture the fact
that although we are using the last declared symbol its datasharing attribute
could be different in this scope and give an example.
https://github.com/llvm/llvm-project/pull/85989
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
https://github.com/kiranchandramohan approved this pull request. LG. Nice work. https://github.com/llvm/llvm-project/pull/85989 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
@@ -2012,34 +2012,87 @@ void OmpAttributeVisitor::Post(const parser::Name
&name) {
}
}
}
-std::vector defaultDSASymbols;
+
+// Implicitly determined DSAs
+// OMP 5.2 5.1.1 - Variables Referenced in a Construct
+Symbol *lastDeclSymbol = nullptr;
+std::optional prevDSA;
for (int dirDepth{0}; dirDepth < (int)dirContext_.size(); ++dirDepth) {
DirContext &dirContext = dirContext_[dirDepth];
- bool hasDataSharingAttr{false};
+ std::optional dsa;
+
for (auto symMap : dirContext.objectWithDSA) {
// if the `symbol` already has a data-sharing attribute
if (symMap.first->name() == name.symbol->name()) {
- hasDataSharingAttr = true;
+ dsa = symMap.second;
break;
}
}
- if (hasDataSharingAttr) {
-if (defaultDSASymbols.size())
- symbol = &MakeAssocSymbol(symbol->name(), *defaultDSASymbols.back(),
+
+ // When handling each implicit rule, either a new private symbol is
+ // declared or the last declared symbol is used.
+ // In the latter case, it's necessary to insert a new symbol in the scope
+ // being processed, associated with the last declared symbol, to avoid
+ // "inheriting" the enclosing context's symbol and its flags.
kiranchandramohan wrote:
Thanks @luporl. It is very clear now.
https://github.com/llvm/llvm-project/pull/85989
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Implement getIterationVariableSymbol helper function,… (PR #90087)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/90087 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [llvm] [flang][OpenMP] Decompose compound construccts, do recursive lowering (PR #90098)
kiranchandramohan wrote: Is it possible to add tests for this? Can we move the decomposition logic into a separate file? https://github.com/llvm/llvm-project/pull/90098 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Pass symTable to all genXYZ functions, NFC (PR #90090)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/90090 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [MLIR][OpenMP] Support clause-based representation of operations (PR #92519)
kiranchandramohan wrote: @skatrak Could you copy the summary of the patch and create an RFC? https://github.com/llvm/llvm-project/pull/92519 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Handle SECTION construct from within SECTIONS (PR #77759)
kiranchandramohan wrote: > This makes it unnecessary to embed OpenMPSectionConstruct inside of > OpenMPSectionConstruct anymore. What do you mean by this? Is there a typo? Would we be able to remove `OpenMPSectionConstruct` from `struct OpenMPConstruct` after this change? https://github.com/llvm/llvm-project/blob/2f2217a8f7ad68b2d9374e0515f02e6752acd126/flang/include/flang/Parser/parse-tree.h#L3990 https://github.com/llvm/llvm-project/pull/77759 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Restructure recursive lowering in `createBodyOfOp` (PR #77761)
kiranchandramohan wrote: @tblah mentions that this patch fixes the unstructured code issue in worksharing loop with collapse. https://github.com/llvm/llvm-project/pull/77329. https://github.com/llvm/llvm-project/pull/77761 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][Lower] Attach target_cpu and target_features attributes to MLIR functions (PR #78289)
@@ -4290,6 +4291,18 @@ class FirConverter : public
Fortran::lower::AbstractConverter {
assert(blockId == 0 && "invalid blockId");
assert(activeConstructStack.empty() && "invalid construct stack state");
+// Set target_cpu and target_features attributes to be passed through to
the
+// llvm.func operation during lowering.
+const llvm::TargetMachine &targetMachine = bridge.getTargetMachine();
+if (auto targetCPU = targetMachine.getTargetCPU(); !targetCPU.empty())
+ func->setAttr("target_cpu",
+mlir::StringAttr::get(func.getContext(), targetCPU));
+
+if (auto targetFeatures = targetMachine.getTargetFeatureString();
+!targetFeatures.empty())
+ func->setAttr("target_features", mlir::LLVM::TargetFeaturesAttr::get(
+ func.getContext(), targetFeatures));
kiranchandramohan wrote:
The default question here would be whether we are going to use this information
in HLFIR/FIR transformations and if not whether it is better to delay adding
this information closer to conversion to LLVM dialect or potentially later.
If it is closer to conversion to LLVM dialect then you might be able to reuse
the pass that added `frame-pointer` attribute.
https://github.com/llvm/llvm-project/pull/74598
Please wait for opinion from others as well.
https://github.com/llvm/llvm-project/pull/78289
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [Flang][MLIR][OpenMP] Use function-attached target attributes for OpenMP lowering (PR #78291)
https://github.com/kiranchandramohan approved this pull request. LGTM. Please wait for @DominikAdamski Not for this patch, but would we need to add this info for all outlined functions? https://github.com/llvm/llvm-project/pull/78291 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [Flang][Lower] Attach target_cpu and target_features attributes to MLIR functions (PR #78289)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/78289 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [flang] [Flang][Lower] Attach target_cpu and target_features attributes to MLIR functions (PR #78289)
https://github.com/kiranchandramohan approved this pull request. LG. Please wait a day before you merge to allow time for other reviewers. https://github.com/llvm/llvm-project/pull/78289 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [Flang][Lower] Attach target_cpu and target_features attributes to MLIR functions (PR #78289)
@@ -4290,6 +4291,18 @@ class FirConverter : public
Fortran::lower::AbstractConverter {
assert(blockId == 0 && "invalid blockId");
assert(activeConstructStack.empty() && "invalid construct stack state");
+// Set target_cpu and target_features attributes to be passed through to
the
+// llvm.func operation during lowering.
+const llvm::TargetMachine &targetMachine = bridge.getTargetMachine();
+if (auto targetCPU = targetMachine.getTargetCPU(); !targetCPU.empty())
+ func->setAttr("target_cpu",
+mlir::StringAttr::get(func.getContext(), targetCPU));
+
+if (auto targetFeatures = targetMachine.getTargetFeatureString();
+!targetFeatures.empty())
+ func->setAttr("target_features", mlir::LLVM::TargetFeaturesAttr::get(
+ func.getContext(), targetFeatures));
kiranchandramohan wrote:
I am OK with it being here.
But as you can imagine there could be other opinions and also the fact that
some function attributes are added in lowering, some in passes and what is the
right place to do this for a future function attribute.
https://github.com/llvm/llvm-project/pull/78289
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [llvm] [OpenMPIRBuilder][MLIR] Pass target-cpu and target-features to outlined functions (PR #80283)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/80283 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] 268ff38 - [MLIR][OpenMP] Attribute to include WsLoop upperbound
Author: Kiran Chandramohan
Date: 2021-01-08T14:42:18Z
New Revision: 268ff38a716157c362b8d463e2e5655f25972e42
URL:
https://github.com/llvm/llvm-project/commit/268ff38a716157c362b8d463e2e5655f25972e42
DIFF:
https://github.com/llvm/llvm-project/commit/268ff38a716157c362b8d463e2e5655f25972e42.diff
LOG: [MLIR][OpenMP] Attribute to include WsLoop upperbound
This patch adds an attribute `inclusive` which if present causes
the upperbound to be included in the loop iteration interval.
Reviewed By: ftynse
Differential Revision: https://reviews.llvm.org/D94235
Added:
Modified:
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
mlir/test/Target/openmp-llvm.mlir
Removed:
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 6c6230f0c2e8..92e418fba7af 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -122,7 +122,9 @@ def WsLoopOp : OpenMP_Op<"wsloop",
[AttrSizedOperandSegments]> {
The workshare loop construct specifies that the iterations of the loop(s)
will be executed in parallel by threads in the current context. These
iterations are spread across threads that already exist in the enclosing
-parallel region.
+parallel region. The lower and upper bounds specify a half-open range: the
+range includes the lower bound but does not include the upper bound. If the
+`inclusive` attribute is specified then the upper bound is also included.
The body region can contain any number of blocks. The region is terminated
by "omp.yield" instruction without operands.
@@ -174,9 +176,10 @@ def WsLoopOp : OpenMP_Op<"wsloop",
[AttrSizedOperandSegments]> {
OptionalAttr:$schedule_val,
Optional:$schedule_chunk_var,
Confined, [IntMinValue<0>]>:$collapse_val,
- OptionalAttr:$nowait,
+ UnitAttr:$nowait,
Confined, [IntMinValue<0>]>:$ordered_val,
- OptionalAttr:$order_val);
+ OptionalAttr:$order_val,
+ UnitAttr:$inclusive);
let builders = [
OpBuilderDAG<(ins "ValueRange":$lowerBound, "ValueRange":$upperBound,
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index f4b76b635128..907ba65c07b7 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -386,7 +386,8 @@ void WsLoopOp::build(OpBuilder &builder, OperationState
&state,
/*linear_vars=*/ValueRange(), /*linear_step_vars=*/ValueRange(),
/*schedule_val=*/nullptr, /*schedule_chunk_var=*/nullptr,
/*collapse_val=*/nullptr,
-/*nowait=*/nullptr, /*ordered_val=*/nullptr, /*order_val=*/nullptr);
+/*nowait=*/false, /*ordered_val=*/nullptr, /*order_val=*/nullptr,
+/*inclusive=*/false);
state.addAttributes(attributes);
}
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 492025ba37b4..70e35c7c7997 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -590,13 +590,12 @@ LogicalResult
ModuleTranslation::convertOmpWsLoop(Operation &opInst,
// Delegate actual loop construction to the OpenMP IRBuilder.
// TODO: this currently assumes WsLoop is semantically similar to SCF loop,
- // i.e. it has a positive step, uses signed integer semantics, and its upper
- // bound is not included. Reconsider this code when WsLoop clearly supports
- // more cases.
+ // i.e. it has a positive step, uses signed integer semantics. Reconsider
+ // this code when WsLoop clearly supports more cases.
llvm::BasicBlock *insertBlock = builder.GetInsertBlock();
llvm::CanonicalLoopInfo *loopInfo = ompBuilder->createCanonicalLoop(
ompLoc, bodyGen, lowerBound, upperBound, step, /*IsSigned=*/true,
- /*InclusiveStop=*/false);
+ /*InclusiveStop=*/loop.inclusive());
if (failed(bodyGenStatus))
return failure();
@@ -606,9 +605,8 @@ LogicalResult ModuleTranslation::convertOmpWsLoop(Operation
&opInst,
// Put them at the start of the current block for now.
llvm::OpenMPIRBuilder::InsertPointTy allocaIP(
insertBlock, insertBlock->getFirstInsertionPt());
- loopInfo = ompBuilder->createStaticWorkshareLoop(
- ompLoc, loopInfo, allocaIP,
- !loop.nowait().hasValue() || loop.nowait().getValue(), chunk);
+ loopInfo = ompBuilder->createStaticWorkshareLoop(ompLoc, loopInfo, allocaIP,
+ !loop.nowait(), chunk);
// Continue building IR after the loop.
builder.restoreIP(loopInfo->getAfterIP());
diff --git a/mlir/test/Target/openmp-llvm.mlir
b/mlir/test
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing support for iterator in DEPEND clause (PR #113622)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/113622 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing support for iterator in DEPEND clause (PR #113622)
@@ -3307,6 +3307,15 @@ void OmpStructureChecker::Enter(const
parser::OmpClause::Depend &x) {
}
}
}
+if (std::get>(inOut->t)) {
+ unsigned version{context_.langOptions().OpenMPVersion};
+ unsigned allowedInVersion = 50;
kiranchandramohan wrote:
```suggestion
unsigned allowedInVersion{50};
```
https://github.com/llvm/llvm-project/pull/113622
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing support for iterator in DEPEND clause (PR #113622)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/113622 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Rename some `Type` members in OpenMP clauses (PR #117784)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/117784 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP Declare Mapper directive (PR #117046)
@@ -2701,7 +2701,42 @@ static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareMapperConstruct &declareMapperConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareMapperConstruct");
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ lower::StatementContext stmtCtx;
+ const auto &spec =
+ std::get(declareMapperConstruct.t);
+ const auto &mapperName{std::get>(spec.t)};
+ const auto &varType{std::get(spec.t)};
+ const auto &varName{std::get(spec.t)};
+ assert(varType.declTypeSpec->category() ==
+ semantics::DeclTypeSpec::Category::TypeDerived &&
+ "Expected derived type");
+
+ std::string mapperNameStr;
+ if (mapperName.has_value())
+mapperNameStr = mapperName->ToString();
+ else
+mapperNameStr =
+"default_" + varType.declTypeSpec->derivedTypeSpec().name().ToString();
+
+ mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
+ firOpBuilder.setInsertionPointToStart(converter.getModuleOp().getBody());
+ auto mlirType = converter.genType(varType.declTypeSpec->derivedTypeSpec());
+ auto varVal = firOpBuilder.createTemporaryAlloc(
+ converter.getCurrentLocation(), mlirType, varName.ToString());
kiranchandramohan wrote:
Looks like @skatrak is also thinking similarly, see his comment in
https://github.com/llvm/llvm-project/pull/117045#discussion_r1858907868
> A new mapper function is emitted, and a pointer to this function is passed on
> to the runtime kernel call for target. The OMPRTL takes offload_mappers as
> the last argument, which is an array of pointers to the user defined mapper
> funcs.
If this is the way it is implemented in Clang, I think you can do it this way
by creating the mapper function from the MLIR region-based declare mapper
specification operation.
https://github.com/llvm/llvm-project/pull/117046
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP Declare Mapper directive (PR #117046)
@@ -2701,7 +2701,42 @@ static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareMapperConstruct &declareMapperConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareMapperConstruct");
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ lower::StatementContext stmtCtx;
+ const auto &spec =
+ std::get(declareMapperConstruct.t);
+ const auto &mapperName{std::get>(spec.t)};
+ const auto &varType{std::get(spec.t)};
+ const auto &varName{std::get(spec.t)};
+ assert(varType.declTypeSpec->category() ==
+ semantics::DeclTypeSpec::Category::TypeDerived &&
+ "Expected derived type");
+
+ std::string mapperNameStr;
+ if (mapperName.has_value())
+mapperNameStr = mapperName->ToString();
+ else
+mapperNameStr =
+"default_" + varType.declTypeSpec->derivedTypeSpec().name().ToString();
+
+ mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
+ firOpBuilder.setInsertionPointToStart(converter.getModuleOp().getBody());
+ auto mlirType = converter.genType(varType.declTypeSpec->derivedTypeSpec());
+ auto varVal = firOpBuilder.createTemporaryAlloc(
+ converter.getCurrentLocation(), mlirType, varName.ToString());
kiranchandramohan wrote:
> Also, I'm not sure what we stand to gain from this approach.
In the approach that you are proposing, there is a variable and map.info that
is created in the function scope with an alloca. This is not correct since the
existence of a declare mapper does not cause the creation of a variable or a
mapping.
Worst case,
-> this can appear to be aliasing with other memory locations and reduce
performance.
-> increase live-ins of openmp regions when outlined
> Every other instance of where a directive has a mapClause associated with it,
> does so by creating the map related Ops just above it. Why are we nesting
> them in case of declMapperOp?
Because it is a declaration. It will only have an effect when it is used either
via its usage in implicit/default mapping of the derived type for which it was
created or when it is directly referred to by a map(mapper...) clause.
I think you have a reasonable point here because of the following:
```
1. If a DECLARE MAPPER directive is not specified for a type DT, a predefined
mapper exists for type DT as if the type DT had appeared in the directive as
follows:
!$OMP DECLARE MAPPER (DT :: var) MAP (TOFROM: var)
2. If a variable is not a scalar then it is treated as if it had appeared in a
map clause with a map-type of tofrom. Which is effectively equivalent to the
following and extending declare mapper for non-derived types:
!$OMP DECLARE MAPPER (T :: var) MAP (TOFROM: var)
```
We can thus argue that all mapping should also generate declarations that are
ultimately all handled in a separate pass in flang/mlir. May be 1 and 2 are
trivial and that is why it is not done that way.
https://github.com/llvm/llvm-project/pull/117046
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP Declare Mapper directive (PR #117046)
@@ -2701,7 +2701,42 @@ static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareMapperConstruct &declareMapperConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareMapperConstruct");
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ lower::StatementContext stmtCtx;
+ const auto &spec =
+ std::get(declareMapperConstruct.t);
+ const auto &mapperName{std::get>(spec.t)};
+ const auto &varType{std::get(spec.t)};
+ const auto &varName{std::get(spec.t)};
+ assert(varType.declTypeSpec->category() ==
+ semantics::DeclTypeSpec::Category::TypeDerived &&
+ "Expected derived type");
+
+ std::string mapperNameStr;
+ if (mapperName.has_value())
+mapperNameStr = mapperName->ToString();
+ else
+mapperNameStr =
+"default_" + varType.declTypeSpec->derivedTypeSpec().name().ToString();
+
+ mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
+ firOpBuilder.setInsertionPointToStart(converter.getModuleOp().getBody());
+ auto mlirType = converter.genType(varType.declTypeSpec->derivedTypeSpec());
+ auto varVal = firOpBuilder.createTemporaryAlloc(
+ converter.getCurrentLocation(), mlirType, varName.ToString());
kiranchandramohan wrote:
Besides the representation, we should also clarify
-> where we create the map_entries for the relevant operations (like target)
for which the declare mapper implicitly applies
-> where the composition of map-types (map-type decay) from the map clause of
declare mapper and the map clause of the relevant operation (like target)
happens
Note: All these are for discussion and should be treated as suggestions.
https://github.com/llvm/llvm-project/pull/117046
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP Declare Mapper directive (PR #117046)
@@ -2701,7 +2701,42 @@ static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareMapperConstruct &declareMapperConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareMapperConstruct");
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ lower::StatementContext stmtCtx;
+ const auto &spec =
+ std::get(declareMapperConstruct.t);
+ const auto &mapperName{std::get>(spec.t)};
+ const auto &varType{std::get(spec.t)};
+ const auto &varName{std::get(spec.t)};
+ assert(varType.declTypeSpec->category() ==
+ semantics::DeclTypeSpec::Category::TypeDerived &&
+ "Expected derived type");
+
+ std::string mapperNameStr;
+ if (mapperName.has_value())
+mapperNameStr = mapperName->ToString();
+ else
+mapperNameStr =
+"default_" + varType.declTypeSpec->derivedTypeSpec().name().ToString();
+
+ mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
+ firOpBuilder.setInsertionPointToStart(converter.getModuleOp().getBody());
+ auto mlirType = converter.genType(varType.declTypeSpec->derivedTypeSpec());
+ auto varVal = firOpBuilder.createTemporaryAlloc(
+ converter.getCurrentLocation(), mlirType, varName.ToString());
kiranchandramohan wrote:
Isn't `declare mapper` providing a specification which is to be used for
mapping in the associated regions? The variable in the `declare mapper` has its
scope inside the construct since it is only used for specifying how to map.
Similar to what @tblah is saying, you could do something like the following.
Assuming %x is the variable that is being mapped by the rules of the declare
mapper:
```
omp.declare_mapper @default_my_type {
^bb0(%[[VAR:.*]]: [[TYPE]]):
%v1 = omp.bounds
%v2 = omp.map.info var_ptr(%[[VAR:.*]])
omp.yield %v2
}
%x_map = omp.map.info var_ptr(%x) decl_map(@default_my_type)
omp.target_enter_data map_entries(%x_map)
```
or
```
omp.declare_mapper @default_my_type {
^bb0(%[[VAR:.*]]: [[TYPE]]):
%v1 = omp.bounds
%v2 = omp.map.info var_ptr(%[[VAR:.*]])
omp.yield %v2
}
omp.target_enter_data map_entries(%x : @default_my_type)
```
The alternative would be to not have a `declare mapper` construct and apply the
declare mapper during lowering itself. But this would be early lowering and we
wont be able to share code with Clang as well.
https://github.com/llvm/llvm-project/pull/117046
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP Declare Mapper directive (PR #117046)
@@ -2701,7 +2701,42 @@ static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
const parser::OpenMPDeclareMapperConstruct &declareMapperConstruct) {
- TODO(converter.getCurrentLocation(), "OpenMPDeclareMapperConstruct");
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ lower::StatementContext stmtCtx;
+ const auto &spec =
+ std::get(declareMapperConstruct.t);
+ const auto &mapperName{std::get>(spec.t)};
+ const auto &varType{std::get(spec.t)};
+ const auto &varName{std::get(spec.t)};
+ assert(varType.declTypeSpec->category() ==
+ semantics::DeclTypeSpec::Category::TypeDerived &&
+ "Expected derived type");
+
+ std::string mapperNameStr;
+ if (mapperName.has_value())
+mapperNameStr = mapperName->ToString();
+ else
+mapperNameStr =
+"default_" + varType.declTypeSpec->derivedTypeSpec().name().ToString();
+
+ mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
+ firOpBuilder.setInsertionPointToStart(converter.getModuleOp().getBody());
+ auto mlirType = converter.genType(varType.declTypeSpec->derivedTypeSpec());
+ auto varVal = firOpBuilder.createTemporaryAlloc(
+ converter.getCurrentLocation(), mlirType, varName.ToString());
kiranchandramohan wrote:
You should feel free to proceed with the approval of @agozillon and @skatrak.
You can set up a quick call with them and describe your approach and come to a
conclusion. You can report the outcomes somewhere in this patch or write in
discourse.
> Inside DeclMapperOp's region I've introduce a new DeclMapperInfoOp to which
> I've attached the MapClause. Not having any MapClause explicitly associated
> seemed weird to me, also walking through the region collecting all the
> MapInfoOps for processing in various places in the code base seemed like a
> bad design to me.
The idea would generally be to inline the whole declare mapper operation
region, replacing the block_arg with the real variable that is going to be
mapped. Would an alloca always be created for all kinds of mappings? If not
committing to an alloca might mean you have to delete it in some circumstances.
> We can drop the entire DeclMapperOp including the region once it reaches
> OpenMPTOLLVMIRTranslation.
As in just drop it without using it? Or using it create the
`@.omp_mapper._ZTS1T.deep_copy` function for the declare mapper (`#pragma omp
declare mapper(deep_copy : T abc) map(abc, abc.ptr[ : abc.buf_size])`) in the C
example you gave below.
You have not talked about the following two points.
```
-> where we create the map_entries for the relevant operations (like target)
for which the declare mapper implicitly applies. Currently, this is done during
lowering. But in this patch you have solely focused on creating the declare
mapper.
-> where the composition of map-types (map-type decay) from the map clause of
declare mapper and the map clause of the relevant operation (like target)
happens
```
Couple of other things that came to my mind :
-> Since declare mappers are in the specification section, it can also occur in
modules. We have not added code to propagate it to the module file. If this is
not urgent for you, I can fix it sometime.
-> You should test the case where the declare mapper is in the host subroutine
```
subroutine A
type t
end type
declare mapper(t)
contains
subroutine B
!$omp target
! use a var of type t
!$omp end target
end subroutine
```
https://github.com/llvm/llvm-project/pull/117046
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parse METADIRECTIVE in specification part (PR #123397)
https://github.com/kiranchandramohan approved this pull request. LG. Declarative directives have to be propagated to module files but for the purpose of generating TODOs, this is not required. https://github.com/llvm/llvm-project/pull/123397 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP Declare Mapper directive (PR #117046)
kiranchandramohan wrote: > @kiranchandramohan I discussed the current approach with @skatrak today. When > trying to implement the mapper lowering for the map clause, it became > apparent that we need to add the `declMapperOp` name to the `SymbolTable`. As > such, we would also need to hoist the `declareMapperOp` to the `ModuleOp`. > > I am however struggling with name mangling the `declMapperOp` such that > scoping information is preserved, and we don't have clashing `declMapperOps` > from different nested scopes with the same name. > > Take the following example: > > ``` > program my_prog >type my_type > integer :: num >end type >!$omp declare mapper (my_mapper : my_type :: my_var) map (my_var) > contains >subroutine test > type(my_type):: xyz > !$omp target enter data map(mapper(my_mapper), to: xyz) >end subroutine test > end program my_prog > ``` > > Here after mangling the, `declMapperOp` symbol name becomes `_QQFmy_mapper`. > But when trying to mangle the name that occurs in the mapClause before lookup > results in `_QQFFtestmy_mapper`. > > Do you know any mechanism in Fortran lowering that could help resolve this > issue? The frontend symbols also are implementing a symbol table. For the example you showed, I am assuming the symbol for `my_mapper` in `target enter data map` is same as the symbol for my_mapper in `declare mapper`. So, naturally they should be mangling to the same name. This is also similar to a variable defined in the program being used in the contained subroutine. The mangling will follow the program's mangling and not the subroutine's mangling. Are the symbols not being resolved properly? Or did I miss a point? https://github.com/llvm/llvm-project/pull/117046 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Avoid early returns, NFC (PR #117231)
https://github.com/kiranchandramohan approved this pull request. LG. Thanks. https://github.com/llvm/llvm-project/pull/117231 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parse METADIRECTIVE in specification part (PR #123397)
https://github.com/kiranchandramohan approved this pull request. LG. The specification part has to be emitted in module files. But this is not necessary for producing the TODOs. https://github.com/llvm/llvm-project/pull/123397 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [llvm] [flang][OpenMP] Parse WHEN, OTHERWISE, MATCH clauses plus METADIRECTIVE (PR #121817)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/121817 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] ea3aa97 - Avoid module name clashes by choosing unique names
Author: Kiran Chandramohan
Date: 2025-01-20T16:23:54Z
New Revision: ea3aa97c17ce30df40f8fc8c2ebb89332c83c5b8
URL:
https://github.com/llvm/llvm-project/commit/ea3aa97c17ce30df40f8fc8c2ebb89332c83c5b8
DIFF:
https://github.com/llvm/llvm-project/commit/ea3aa97c17ce30df40f8fc8c2ebb89332c83c5b8.diff
LOG: Avoid module name clashes by choosing unique names
Added:
Modified:
flang/test/Lower/zero_init.f90
flang/test/Lower/zero_init_default_init.f90
Removed:
diff --git a/flang/test/Lower/zero_init.f90 b/flang/test/Lower/zero_init.f90
index 5ed6f2247de3b2..16b11158bfce27 100644
--- a/flang/test/Lower/zero_init.f90
+++ b/flang/test/Lower/zero_init.f90
@@ -5,16 +5,16 @@
! RUN: bbc -finit-global-zero -emit-hlfir -o - %s | FileCheck
--check-prefix=CHECK-DEFAULT %s
! RUN: bbc -finit-global-zero=false -emit-hlfir -o - %s | FileCheck
--check-prefix=CHECK-NO-ZERO-INIT %s
-module m1
+module zeroInitM1
real :: x
-end module m1
+end module zeroInitM1
-!CHECK-DEFAULT: fir.global @_QMm1Ex : f32 {
+!CHECK-DEFAULT: fir.global @_QMzeroinitm1Ex : f32 {
!CHECK-DEFAULT: %[[UNDEF:.*]] = fir.zero_bits f32
!CHECK-DEFAULT: fir.has_value %[[UNDEF]] : f32
!CHECK-DEFAULT: }
-!CHECK-NO-ZERO-INIT: fir.global @_QMm1Ex : f32 {
+!CHECK-NO-ZERO-INIT: fir.global @_QMzeroinitm1Ex : f32 {
!CHECK-NO-ZERO-INIT: %[[UNDEF:.*]] = fir.undefined f32
!CHECK-NO-ZERO-INIT: fir.has_value %[[UNDEF]] : f32
!CHECK-NO-ZERO-INIT: }
diff --git a/flang/test/Lower/zero_init_default_init.f90
b/flang/test/Lower/zero_init_default_init.f90
index e2d1f545e35a57..8ca3b33b8ef5c1 100644
--- a/flang/test/Lower/zero_init_default_init.f90
+++ b/flang/test/Lower/zero_init_default_init.f90
@@ -7,16 +7,16 @@
! Test that the flag does not affect globals with default init
-module m2
+module zeroInitM2
type val
integer :: my_val = 1
end type val
type(val) :: v1
-end module m2
+end module zeroInitM2
-!CHECK: fir.global @_QMm2Ev1 : !fir.type<_QMm2Tval{my_val:i32}> {
-!CHECK:%[[V1:.*]] = fir.undefined !fir.type<_QMm2Tval{my_val:i32}>
+!CHECK: fir.global @_QMzeroinitm2Ev1 :
!fir.type<_QMzeroinitm2Tval{my_val:i32}> {
+!CHECK:%[[V1:.*]] = fir.undefined !fir.type<_QMzeroinitm2Tval{my_val:i32}>
!CHECK:%[[ONE:.*]] = arith.constant 1 : i32
-!CHECK:%[[V1_INIT:.*]] = fir.insert_value %[[V1]], %[[ONE]], ["my_val",
!fir.type<_QMm2Tval{my_val:i32}>] : (!fir.type<_QMm2Tval{my_val:i32}>, i32) ->
!fir.type<_QMm2Tval{my_val:i32}>
-!CHECK:fir.has_value %[[V1_INIT]] : !fir.type<_QMm2Tval{my_val:i32}>
+!CHECK:%[[V1_INIT:.*]] = fir.insert_value %[[V1]], %[[ONE]], ["my_val",
!fir.type<_QMzeroinitm2Tval{my_val:i32}>] :
(!fir.type<_QMzeroinitm2Tval{my_val:i32}>, i32) ->
!fir.type<_QMzeroinitm2Tval{my_val:i32}>
+!CHECK:fir.has_value %[[V1_INIT]] :
!fir.type<_QMzeroinitm2Tval{my_val:i32}>
!CHECK: }
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [llvm] [flang][OpenMP] Parse WHEN, OTHERWISE, MATCH clauses plus METADIRECTIVE (PR #121817)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/121817 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [llvm] [flang][OpenMP] Parse WHEN, OTHERWISE, MATCH clauses plus METADIRECTIVE (PR #121817)
@@ -214,6 +214,11 @@ class AssociatedLoopChecker {
};
bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) {
+ // Do not do clause checks while processing METADIRECTIVE.
kiranchandramohan wrote:
Could you add the reason also?
https://github.com/llvm/llvm-project/pull/121817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [llvm] [flang][OpenMP] Parse WHEN, OTHERWISE, MATCH clauses plus METADIRECTIVE (PR #121817)
@@ -845,7 +851,8 @@ def OMP_Metadirective : Directive<"metadirective"> {
VersionedClause,
];
let allowedOnceClauses = [
-VersionedClause,
+VersionedClause,
+VersionedClause,
kiranchandramohan wrote:
Is this tested in Flang?
https://github.com/llvm/llvm-project/pull/121817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [llvm] [flang][OpenMP] Parse WHEN, OTHERWISE, MATCH clauses plus METADIRECTIVE (PR #121817)
https://github.com/kiranchandramohan commented: Minor comments. https://github.com/llvm/llvm-project/pull/121817 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [llvm] [flang][OpenMP] Parse WHEN, OTHERWISE, MATCH clauses plus METADIRECTIVE (PR #121817)
@@ -265,6 +265,7 @@ def OMPC_Map : Clause<"map"> {
let flangClass = "OmpMapClause";
}
def OMPC_Match : Clause<"match"> {
+ let flangClass = "OmpMatchClause";
kiranchandramohan wrote:
Is there a test for Match?
https://github.com/llvm/llvm-project/pull/121817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
@@ -3474,6 +3477,138 @@ WRAPPER_CLASS(OmpObjectList, std::list);
#define MODIFIERS() std::optional>
+inline namespace traits {
+// trait-property-name ->
+//identifier | string-literal
+struct OmpTraitPropertyName {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitPropertyName, std::string);
kiranchandramohan wrote:
Sure. Thanks for the explanation. Can you add this explanation to the
representation?
https://github.com/llvm/llvm-project/pull/121815
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
https://github.com/kiranchandramohan approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/121815 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [MLIR][OpenMP] Add OMP Mapper field to MapInfoOp (PR #120994)
@@ -1000,6 +1000,7 @@ def MapInfoOp : OpenMP_Op<"map.info",
[AttrSizedOperandSegments]> {
OptionalAttr:$members_index,
Variadic:$bounds, /* rank-0 to
rank-{n-1} */
OptionalAttr:$map_type,
+ OptionalAttr:$mapper_id,
kiranchandramohan wrote:
If it can only be the name of a declare mapper then it might be good to add
that to the verifier.
https://github.com/llvm/llvm-project/pull/120994
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [MLIR][OpenMP] Add OMP Mapper field to MapInfoOp (PR #120994)
@@ -1000,6 +1000,7 @@ def MapInfoOp : OpenMP_Op<"map.info",
[AttrSizedOperandSegments]> {
OptionalAttr:$members_index,
Variadic:$bounds, /* rank-0 to
rank-{n-1} */
OptionalAttr:$map_type,
+ OptionalAttr:$mapper_id,
kiranchandramohan wrote:
Please add a description for this argument.
https://github.com/llvm/llvm-project/pull/120994
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/121815 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
@@ -153,6 +153,81 @@ static TypeDeclarationStmt
makeIterSpecDecl(std::list &&names) {
makeEntityList(std::move(names)));
}
+// --- Parsers for context traits -
+
+TYPE_PARSER(sourced(construct( //
+(space >> charLiteralConstantWithoutKind) ||
+applyMem(&Name::ToString, Parser{}
+
+TYPE_PARSER(sourced(construct( //
+"SCORE" >> parenthesized(scalarIntExpr
+
+TYPE_PARSER(sourced(construct(
+// Parse nested extension first.
+construct(
+indirect(Parser{})) ||
+construct(
+Parser{}) ||
+construct(scalarExpr
+
+TYPE_PARSER(sourced(construct( //
+Parser{},
+parenthesized(nonemptySeparated(
+Parser{}, ","_tok)
+
+TYPE_PARSER(sourced(construct(
+// Try clause first, then extension before OmpTraitPropertyName.
+construct(indirect(Parser{})) ||
+construct(Parser{}) ||
+construct(Parser{}) ||
+construct(scalarExpr
+
+TYPE_PARSER(construct(
+"ARCH" >> pure(OmpTraitSelectorName::Value::Arch) ||
+"ATOMIC_DEFAULT_MEM_ORDER" >>
+pure(OmpTraitSelectorName::Value::Atomic_Default_Mem_Order) ||
+"CONDITION" >> pure(OmpTraitSelectorName::Value::Condition) ||
+"DEVICE_NUM" >> pure(OmpTraitSelectorName::Value::Device_Num) ||
+"EXTENSION" >> pure(OmpTraitSelectorName::Value::Extension) ||
+"ISA" >> pure(OmpTraitSelectorName::Value::Isa) ||
+"KIND" >> pure(OmpTraitSelectorName::Value::Kind) ||
+"REQUIRES" >> pure(OmpTraitSelectorName::Value::Requires) ||
+"SIMD" >> pure(OmpTraitSelectorName::Value::Simd) ||
+"UID" >> pure(OmpTraitSelectorName::Value::Uid) ||
+"VENDOR" >> pure(OmpTraitSelectorName::Value::Vendor)))
+
+TYPE_PARSER(sourced(construct(
+// Parse predefined names first (because of SIMD).
+construct(Parser{}) ||
+construct(OmpDirectiveNameParser{}
+
+TYPE_PARSER(construct(
+maybe(Parser{} / ":"_tok),
+nonemptySeparated(Parser{}, ","_tok)))
+
+TYPE_PARSER(sourced(construct( //
+Parser{}, //
+maybe(parenthesized(Parser{})
+
+TYPE_PARSER(construct(
+"CONSTRUCT" >> pure(OmpTraitSetSelectorName::Value::Construct) ||
+"DEVICE" >> pure(OmpTraitSetSelectorName::Value::Device) ||
+"IMPLEMENTATION" >> pure(OmpTraitSetSelectorName::Value::Implementation) ||
+"TARGET_DEVICE" >> pure(OmpTraitSetSelectorName::Value::Target_Device) ||
+"USER" >> pure(OmpTraitSetSelectorName::Value::User)))
+
+TYPE_PARSER(sourced(construct(
+Parser{})))
+
+TYPE_PARSER(sourced(construct( //
+Parser{},
+"=" >> braced(nonemptySeparated(Parser{}, ","_tok)
+
+TYPE_PARSER(sourced(construct(
+nonemptySeparated(Parser{}, ","_tok
+
+// Parser ==
Parser
kiranchandramohan wrote:
Commented code?
https://github.com/llvm/llvm-project/pull/121815
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
@@ -3474,6 +3477,138 @@ WRAPPER_CLASS(OmpObjectList, std::list);
#define MODIFIERS() std::optional>
+inline namespace traits {
+// trait-property-name ->
+//identifier | string-literal
+struct OmpTraitPropertyName {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitPropertyName, std::string);
+};
+
+// trait-score ->
+//SCORE(non-negative-const-integer-expression)
+struct OmpTraitScore {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitScore, ScalarIntExpr);
+};
+
+// trait-property-extension ->
+//trait-property-name (trait-property-value, ...)
+// trait-property-value ->
+//trait-property-name |
+//scalar-integer-expression |
+//trait-property-extension
+//
+// The grammar in OpenMP 5.2+ spec is ambiguous, the above is a different
+// version (but equivalent) that doesn't have ambiguities.
+// The ambiguity is in
+// trait-property:
+// trait-property-name <- (a)
+// trait-property-clause
+// trait-property-expression<- (b)
+// trait-property-extension <- this conflicts with (a) and (b)
+// trait-property-extension:
+// trait-property-name <- conflict with (a)
+// identifier(trait-property-extension[, trait-property-extension[, ...]])
+// constant integer expression <- conflict with (b)
+//
+struct OmpTraitPropertyExtension {
+ CharBlock source;
+ TUPLE_CLASS_BOILERPLATE(OmpTraitPropertyExtension);
+ struct ExtensionValue {
+CharBlock source;
+UNION_CLASS_BOILERPLATE(ExtensionValue);
+std::variant>
+u;
+ };
+ using ExtensionList = std::list;
+ std::tuple t;
+};
+
+// trait-property ->
+//trait-property-name | OmpClause |
+//trait-property-expression | trait-property-extension
+// trait-property-expression ->
+//scalar-logical-expression | scalar-integer-expression
+//
+// The parser for a logical expression will accept an integer expression,
+// and if it's not logical, it will flag an error later. The same thing
+// will happen if the scalar integer expression sees a logical expresion.
+// To avoid this, parse all expressions as scalar expressions.
+struct OmpTraitProperty {
+ CharBlock source;
+ UNION_CLASS_BOILERPLATE(OmpTraitProperty);
+ std::variant,
+ ScalarExpr, // trait-property-expresion
+ OmpTraitPropertyExtension>
+ u;
+};
+
+// trait-selector-name ->
+//KIND | DT // name-list (host, nohost, +/add-def-doc)
+//ISA | DT // name-list (isa_name, ... /impl-defined)
+//ARCH | DT // name-list (arch_name, ... /impl-defined)
+//directive-name |C// no properties
+//SIMD | C// clause-list (from declare_simd)
+// // (at least simdlen, inbranch/notinbranch)
+//DEVICE_NUM |T// device-number
+//UID | T// unique-string-id /impl-defined
+//VENDOR |I// name-list (vendor-id /add-def-doc)
+//EXTENSION | I// name-list (ext_name /impl-defined)
+//ATOMIC_DEFAULT_MEM_ORDER I | // value of admo
+//REQUIRES | I// clause-list (from requires)
+//CONDITION U// logical-expr
+//
+// Trait-set-selectors:
+//[D]evice, [T]arget_device, [C]onstruct, [I]mplementation, [U]ser.
+struct OmpTraitSelectorName {
+ CharBlock source;
+ UNION_CLASS_BOILERPLATE(OmpTraitSelectorName);
+ ENUM_CLASS(Value, Arch, Atomic_Default_Mem_Order, Condition, Device_Num,
+ Extension, Isa, Kind, Requires, Simd, Uid, Vendor)
+ std::variant u;
+};
+
+// trait-selector ->
+//trait-selector-name |
+//trait-selector-name ([trait-score:] trait-property, ...)
+struct OmpTraitSelector {
+ CharBlock source;
+ TUPLE_CLASS_BOILERPLATE(OmpTraitSelector);
+ struct Properties {
+TUPLE_CLASS_BOILERPLATE(Properties);
+std::tuple, std::list> t;
+ };
+ std::tuple> t;
+};
+
+// trait-set-selector-name ->
+//CONSTRUCT | DEVICE | IMPLEMENTATION | USER | // since 5.0
+//TARGET_DEVICE // since 5.1
+struct OmpTraitSetSelectorName {
+ CharBlock source;
+ ENUM_CLASS(Value, Construct, Device, Implementation, Target_Device, User)
kiranchandramohan wrote:
Same question as above.
https://github.com/llvm/llvm-project/pull/121815
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
@@ -3474,6 +3477,138 @@ WRAPPER_CLASS(OmpObjectList, std::list);
#define MODIFIERS() std::optional>
+inline namespace traits {
+// trait-property-name ->
+//identifier | string-literal
+struct OmpTraitPropertyName {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitPropertyName, std::string);
+};
+
+// trait-score ->
+//SCORE(non-negative-const-integer-expression)
+struct OmpTraitScore {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitScore, ScalarIntExpr);
+};
+
+// trait-property-extension ->
+//trait-property-name (trait-property-value, ...)
+// trait-property-value ->
+//trait-property-name |
+//scalar-integer-expression |
+//trait-property-extension
+//
+// The grammar in OpenMP 5.2+ spec is ambiguous, the above is a different
+// version (but equivalent) that doesn't have ambiguities.
+// The ambiguity is in
+// trait-property:
+// trait-property-name <- (a)
+// trait-property-clause
+// trait-property-expression<- (b)
+// trait-property-extension <- this conflicts with (a) and (b)
+// trait-property-extension:
+// trait-property-name <- conflict with (a)
+// identifier(trait-property-extension[, trait-property-extension[, ...]])
+// constant integer expression <- conflict with (b)
+//
+struct OmpTraitPropertyExtension {
+ CharBlock source;
+ TUPLE_CLASS_BOILERPLATE(OmpTraitPropertyExtension);
+ struct ExtensionValue {
+CharBlock source;
+UNION_CLASS_BOILERPLATE(ExtensionValue);
+std::variant>
+u;
+ };
+ using ExtensionList = std::list;
+ std::tuple t;
+};
+
+// trait-property ->
+//trait-property-name | OmpClause |
+//trait-property-expression | trait-property-extension
+// trait-property-expression ->
+//scalar-logical-expression | scalar-integer-expression
+//
+// The parser for a logical expression will accept an integer expression,
+// and if it's not logical, it will flag an error later. The same thing
+// will happen if the scalar integer expression sees a logical expresion.
+// To avoid this, parse all expressions as scalar expressions.
+struct OmpTraitProperty {
+ CharBlock source;
+ UNION_CLASS_BOILERPLATE(OmpTraitProperty);
+ std::variant,
+ ScalarExpr, // trait-property-expresion
+ OmpTraitPropertyExtension>
+ u;
+};
+
+// trait-selector-name ->
+//KIND | DT // name-list (host, nohost, +/add-def-doc)
+//ISA | DT // name-list (isa_name, ... /impl-defined)
+//ARCH | DT // name-list (arch_name, ... /impl-defined)
+//directive-name |C// no properties
+//SIMD | C// clause-list (from declare_simd)
+// // (at least simdlen, inbranch/notinbranch)
+//DEVICE_NUM |T// device-number
+//UID | T// unique-string-id /impl-defined
+//VENDOR |I// name-list (vendor-id /add-def-doc)
+//EXTENSION | I// name-list (ext_name /impl-defined)
+//ATOMIC_DEFAULT_MEM_ORDER I | // value of admo
+//REQUIRES | I// clause-list (from requires)
+//CONDITION U// logical-expr
+//
+// Trait-set-selectors:
+//[D]evice, [T]arget_device, [C]onstruct, [I]mplementation, [U]ser.
+struct OmpTraitSelectorName {
+ CharBlock source;
+ UNION_CLASS_BOILERPLATE(OmpTraitSelectorName);
+ ENUM_CLASS(Value, Arch, Atomic_Default_Mem_Order, Condition, Device_Num,
kiranchandramohan wrote:
Are underscores generally used in this file?
https://github.com/llvm/llvm-project/pull/121815
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
@@ -3474,6 +3477,138 @@ WRAPPER_CLASS(OmpObjectList, std::list);
#define MODIFIERS() std::optional>
+inline namespace traits {
+// trait-property-name ->
+//identifier | string-literal
+struct OmpTraitPropertyName {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitPropertyName, std::string);
kiranchandramohan wrote:
Did we miss the identifier part, ie `Name`?
```
trait-property-name: identifier | string-literal
```
https://github.com/llvm/llvm-project/pull/121815
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
@@ -3474,6 +3477,138 @@ WRAPPER_CLASS(OmpObjectList, std::list);
#define MODIFIERS() std::optional>
+inline namespace traits {
+// trait-property-name ->
+//identifier | string-literal
+struct OmpTraitPropertyName {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitPropertyName, std::string);
+};
+
+// trait-score ->
+//SCORE(non-negative-const-integer-expression)
+struct OmpTraitScore {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitScore, ScalarIntExpr);
kiranchandramohan wrote:
Should this be `ScalarIntConstantExpr` ?
https://github.com/llvm/llvm-project/pull/121815
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
@@ -3474,6 +3477,138 @@ WRAPPER_CLASS(OmpObjectList, std::list);
#define MODIFIERS() std::optional>
+inline namespace traits {
+// trait-property-name ->
+//identifier | string-literal
+struct OmpTraitPropertyName {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitPropertyName, std::string);
+};
+
+// trait-score ->
+//SCORE(non-negative-const-integer-expression)
+struct OmpTraitScore {
+ CharBlock source;
+ WRAPPER_CLASS_BOILERPLATE(OmpTraitScore, ScalarIntExpr);
+};
+
+// trait-property-extension ->
+//trait-property-name (trait-property-value, ...)
+// trait-property-value ->
+//trait-property-name |
+//scalar-integer-expression |
+//trait-property-extension
+//
+// The grammar in OpenMP 5.2+ spec is ambiguous, the above is a different
+// version (but equivalent) that doesn't have ambiguities.
kiranchandramohan wrote:
Is this reported?
https://github.com/llvm/llvm-project/pull/121815
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
https://github.com/kiranchandramohan commented: Nice work. A few minor comments. https://github.com/llvm/llvm-project/pull/121815 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Parsing context selectors for METADIRECTIVE (PR #121815)
@@ -3453,6 +3453,17 @@ WRAPPER_CLASS(PauseStmt, std::optional);
// --- Common definitions
+struct OmpClause;
+struct OmpClauseList;
+
+struct OmpDirectiveSpecification {
+ TUPLE_CLASS_BOILERPLATE(OmpDirectiveSpecification);
+ std::tuple>>
+ t;
+ CharBlock source;
+};
kiranchandramohan wrote:
This needs a comment. Is this strictly part of context-selectors?
https://github.com/llvm/llvm-project/pull/121815
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [llvm] [flang][OpenMP] Parse cancel-directive-name as clause (PR #130146)
https://github.com/kiranchandramohan approved this pull request. LG. https://github.com/llvm/llvm-project/pull/130146 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
