convert yielded result

Author: ergawy

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

@@ -530,7 +530,6 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
     fir::ExtendedValue symExV = converter.getSymbolExtendedValue(*sym);
 
     symTable->pushScope();
-    mlir::Type yieldedType;
 
     // Populate the `alloc` region.
     {
@@ -550,10 +549,17 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
       symTable->addSymbol(*sym, localExV);
       symTable->pushScope();
       cloneSymbol(sym);
+      mlir::Value cloneAddr = symTable->shallowLookupSymbol(*sym).getAddr();
+      mlir::Type cloneType = cloneAddr.getType();
+
+      mlir::Value yieldedValue =
+          (symType == cloneType) ? cloneAddr
+                                 : firOpBuilder.createConvert(
+                                       cloneAddr.getLoc(), symType, cloneAddr);
+
       auto yieldOp = firOpBuilder.create<mlir::omp::YieldOp>(
-          hsb.getAddr().getLoc(),
-          symTable->shallowLookupSymbol(*sym).getAddr());
-      yieldedType = yieldOp.getOperand(0).getType();
+          hsb.getAddr().getLoc(), yieldedValue);
+      yieldOp.getOperand(0).getType();
       symTable->popScope();
     }
 
@@ -562,9 +568,8 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
       mlir::Region &copyRegion = result.getCopyRegion();
       // First block argument corresponding to the original/host value while
       // second block argument corresponding to the privatized value.
-      mlir::Block *copyEntryBlock =
-          firOpBuilder.createBlock(&copyRegion, /*insertPt=*/{},
-                                   {symType, yieldedType}, {symLoc, symLoc});
+      mlir::Block *copyEntryBlock = firOpBuilder.createBlock(
+          &copyRegion, /*insertPt=*/{}, {symType, symType}, {symLoc, symLoc});
       firOpBuilder.setInsertionPointToEnd(copyEntryBlock);
 
       auto addSymbol = [&](unsigned argIdx, bool force = false) {

flang/test/Lower/OpenMP/DelayedPrivatization/equivalence.f90

@@ -17,12 +17,13 @@ subroutine private_common
 ! CHECK:  ^bb0(%{{.*}}: ![[X_TYPE]]):
 ! CHECK:    %[[PRIV_ALLOC:.*]] = fir.alloca f32 {bindc_name = "x", {{.*}}}
 ! CHECK:    %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]] {{{.*}}} : (![[PRIV_TYPE:fir.ref<f32>]]) -> ({{.*}})
-! CHECK:    omp.yield(%[[PRIV_DECL]]#0 : ![[PRIV_TYPE]])
+! CHECK:    %[[PRIV_CONV:.*]] = fir.convert %[[PRIV_DECL]]#0 : (![[PRIV_TYPE]]) -> ![[X_TYPE]]
+! CHECK:    omp.yield(%[[PRIV_CONV]] : ![[X_TYPE]])
 ! CHECK:  } copy {
-! CHECK:  ^bb0(%[[ORIG_PTR:.*]]: ![[X_TYPE]], %[[PRIV_REF:.*]]: ![[PRIV_TYPE]]):
+! CHECK:  ^bb0(%[[ORIG_PTR:.*]]: ![[X_TYPE]], %[[PRIV_REF:.*]]: ![[X_TYPE]]):
 ! CHECK:    %[[ORIG_VAL:.*]] = fir.load %[[ORIG_PTR]] : !fir.ptr<f32>
-! CHECK:    hlfir.assign %[[ORIG_VAL]] to %[[PRIV_REF]] temporary_lhs : f32, ![[PRIV_TYPE]]
-! CHECK:    omp.yield(%[[PRIV_REF]] : ![[PRIV_TYPE]])
+! CHECK:    hlfir.assign %[[ORIG_VAL]] to %[[PRIV_REF]] temporary_lhs : f32, ![[X_TYPE]]
+! CHECK:    omp.yield(%[[PRIV_REF]] : ![[X_TYPE]])
 ! CHECK:  }
 
 ! CHECK:  func.func @_QPprivate_common() {

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

@@ -2493,11 +2493,7 @@ LogicalResult PrivateClauseOp::verify() {
              << "Did not expect any values to be yielded.";
     }
 
-    // TODO How can we check that 2 types are compatible without leaking
-    // dialect-specific (e.g. FIR) information? The problem is that in some
-    // cases, we would get the input type of the symbol as, e.g. `fir.ptr<i32>`
-    // while the allocated private memory is of type `fir.ref<i32>`.
-    if (yieldedTypes.size() == 1 /*&& yieldedTypes.front() == symType*/)
+    if (yieldedTypes.size() == 1 && yieldedTypes.front() == symType)
       return success();
 
     auto error = mlir::emitError(yieldOp.getLoc())

mlir/test/Dialect/OpenMP/invalid.mlir

@@ -2207,14 +2207,13 @@ func.func @omp_distribute_unconstrained_order() -> () {
   }
   return
 }
-
-// Disabled for now. See relevant TODO in PrivateClauseOp::verify().
-//omp.private {type = private} @x.privatizer : i32 alloc {
-//^bb0(%arg0: i32):
-//  %0 = arith.constant 0.0 : f32
-//  // _expected-error_ @below {{Invalid yielded value. Expected type: 'i32', got: 'f32'}}
-//  omp.yield(%0 : f32)
-//}
+// -----
+omp.private {type = private} @x.privatizer : i32 alloc {
+^bb0(%arg0: i32):
+  %0 = arith.constant 0.0 : f32
+  // expected-error @below {{Invalid yielded value. Expected type: 'i32', got: 'f32'}}
+  omp.yield(%0 : f32)
+}
 
 // -----