Add some comments and clean up some codoe

Author: bhandarkar-pranav

flang/lib/Optimizer/CodeGen/CodeGen.cpp

@@ -52,7 +52,6 @@
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
 #include "mlir/Dialect/LLVMIR/NVVMDialect.h"
 #include "mlir/Dialect/LLVMIR/Transforms/AddComdats.h"
-#include "mlir/Dialect/LLVMIR/Transforms/OpenMPOffloadPrivatizationPrepare.h"
 #include "mlir/Dialect/OpenACC/OpenACC.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/BuiltinTypes.h"

mlir/lib/Dialect/LLVMIR/IR/FunctionCallUtils.cpp

@@ -68,7 +68,6 @@ mlir::LLVM::lookupOrCreateFn(OpBuilder &b, Operation *moduleOp, StringRef name,
                              SymbolTableCollection *symbolTables) {
   assert(moduleOp->hasTrait<OpTrait::SymbolTable>() &&
          "expected SymbolTable operation");
-  llvm::errs() << "Looking up " << name << "\n";
   auto func = lookupFuncOp(name, moduleOp, symbolTables);
   auto funcT = LLVMFunctionType::get(resultType, paramTypes, isVarArg);
   // Assert the signature of the found function is same as expected
@@ -86,7 +85,7 @@ mlir::LLVM::lookupOrCreateFn(OpBuilder &b, Operation *moduleOp, StringRef name,
     }
     return func;
   }
-  llvm::errs() << "Did not find " << name << ".. creating it \n";
+
   OpBuilder::InsertionGuard g(b);
   assert(!moduleOp->getRegion(0).empty() && "expected non-empty region");
   b.setInsertionPointToStart(&moduleOp->getRegion(0).front());

mlir/lib/Dialect/LLVMIR/Transforms/OpenMPOffloadPrivatizationPrepare.cpp

@@ -95,25 +95,21 @@ class OMPTargetPrepareDelayedPrivatizationPattern
         newPrivVars.push_back(privVar);
         continue;
       }
-      // Treat the privVar as varPtr. TODO: For boxchars this likely wont be a
-      // pointer. Allocate heap memory that corresponds to the type of memory
+
+      // Allocate heap memory that corresponds to the type of memory
       // pointed to by varPtr
+      // TODO: For boxchars this likely wont be a pointer.
       mlir::Value varPtr = privVar;
       mlir::Value heapMem = allocateHeapMem(targetOp, privVar, mod, rewriter);
-      if (!heapMem) {
-        newPrivVars.push_back(privVar);
+      if (!heapMem)
         return failure();
-      }
-      newPrivVars.push_back(heapMem);
-
-      // Find the earliest insertion point for the copy.
-
-      // Now, fix up the omp::MapInfoOp instances that use varPtr to refer
-      // to heapMem instead.
-      using ReplacementEntry = std::pair<Operation *, Operation *>;
-      llvm::SmallVector<ReplacementEntry> replRecord;
 
+      newPrivVars.push_back(heapMem);
 
+      // Find the earliest insertion point for the copy. This will be before
+      // the first in the list of omp::MapInfoOp instances that use varPtr.
+      // After the copy these omp::MapInfoOp instances will refer to heapMem
+      // instead.
       Operation *varPtrDefiningOp = varPtr.getDefiningOp();
       std::set<Operation *> users;
       users.insert(varPtrDefiningOp->user_begin(),
@@ -122,11 +118,9 @@ class OMPTargetPrepareDelayedPrivatizationPattern
       auto usesVarPtr = [&users](Operation *op) -> bool {
         return users.count(op);
       };
-
       SmallVector<Operation *> chainOfOps;
       chainOfOps.push_back(mapInfoOperation);
       if (!mapInfoOp.getMembers().empty()) {
-
         for (auto member : mapInfoOp.getMembers()) {
           if (usesVarPtr(member.getDefiningOp()))
             chainOfOps.push_back(member.getDefiningOp());
@@ -144,13 +138,15 @@ class OMPTargetPrepareDelayedPrivatizationPattern
       });
 
       rewriter.setInsertionPoint(chainOfOps.front());
+      // Copy the value of the local variable into the heap-allocated location.
       mlir::Location loc = chainOfOps.front()->getLoc();
       mlir::Type varType = getElemType(varPtr);
-      // Copy the value of the local variable into the heap-allocated location.
       auto loadVal = rewriter.create<LLVM::LoadOp>(loc, varType, varPtr);
       LLVM_ATTRIBUTE_UNUSED auto storeInst =
           rewriter.create<LLVM::StoreOp>(loc, loadVal.getResult(), heapMem);
 
+      using ReplacementEntry = std::pair<Operation *, Operation *>;
+      llvm::SmallVector<ReplacementEntry> replRecord;
       auto cloneAndMarkForDeletion = [&](Operation *origOp) -> Operation * {
         Operation *clonedOp = rewriter.clone(*origOp);
         rewriter.replaceAllOpUsesWith(origOp, clonedOp);

mlir/lib/Tools/mlir-opt/MlirOptMain.cpp

@@ -624,7 +624,7 @@ LogicalResult mlir::MlirOptMain(llvm::raw_ostream &outputStream,
   // We use the thread-pool this context is creating, and avoid
   // creating any thread when disabled.
   MLIRContext threadPoolCtx;
-  llvm::errs() << "threadPoolCtx.isMultithreadingEnabled() = " << threadPoolCtx.isMultithreadingEnabled() << "\n";
+
   if (threadPoolCtx.isMultithreadingEnabled())
     threadPool = &threadPoolCtx.getThreadPool();