Translation to LLVM IR: use LogicalResult instead of bool

The translation code predates the introduction of LogicalResult and was relying
on the obsolete LLVM convention of returning false on success.  Change it to
use MLIR's LogicalResult abstraction instead. NFC.

PiperOrigin-RevId: 262589432

4 files changed, 35 insertions, 38 deletions

diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h b/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
index 063ce7d173b..04651b8cf6e 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
@@ -55,7 +55,7 @@ public:
     T translator(m);
     translator.llvmModule = std::move(llvmModule);
     translator.convertGlobals();
-    if (translator.convertFunctions())
+    if (failed(translator.convertFunctions()))
       return nullptr;
 
     return std::move(translator.llvmModule);
@@ -68,15 +68,16 @@ protected:
   explicit ModuleTranslation(ModuleOp module) : mlirModule(module) {}
   virtual ~ModuleTranslation() {}
 
-  virtual bool convertOperation(Operation &op, llvm::IRBuilder<> &builder);
+  virtual LogicalResult convertOperation(Operation &op,
+                                         llvm::IRBuilder<> &builder);
   static std::unique_ptr<llvm::Module> prepareLLVMModule(ModuleOp m);
 
 private:
-  bool convertFunctions();
+  LogicalResult convertFunctions();
   void convertGlobals();
-  bool convertOneFunction(FuncOp func);
+  LogicalResult convertOneFunction(FuncOp func);
   void connectPHINodes(FuncOp func);
-  bool convertBlock(Block &bb, bool ignoreArguments);
+  LogicalResult convertBlock(Block &bb, bool ignoreArguments);
 
   template <typename Range>
   SmallVector<llvm::Value *, 8> lookupValues(Range &&values);
diff --git a/mlir/lib/Target/LLVMIR/ConvertToNVVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertToNVVMIR.cpp
index c670cbf8337..a1e09fda84d 100644
--- a/mlir/lib/Target/LLVMIR/ConvertToNVVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertToNVVMIR.cpp
@@ -52,8 +52,8 @@ public:
   ~ModuleTranslation() override {}
 
 protected:
-  bool convertOperation(Operation &opInst,
-                        llvm::IRBuilder<> &builder) override {
+  LogicalResult convertOperation(Operation &opInst,
+                                 llvm::IRBuilder<> &builder) override {
 
 #include "mlir/LLVMIR/NVVMConversions.inc"
 
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 3cbf543bdfc..5e1109bbdd0 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -194,8 +194,8 @@ SmallVector<llvm::Value *, 8> ModuleTranslation::lookupValues(Range &&values) {
 // using the `builder`.  LLVM IR Builder does not have a generic interface so
 // this has to be a long chain of `if`s calling different functions with a
 // different number of arguments.
-bool ModuleTranslation::convertOperation(Operation &opInst,
-                                         llvm::IRBuilder<> &builder) {
+LogicalResult ModuleTranslation::convertOperation(Operation &opInst,
+                                                  llvm::IRBuilder<> &builder) {
   auto extractPosition = [](ArrayAttr attr) {
     SmallVector<unsigned, 4> position;
     position.reserve(attr.size());
@@ -228,33 +228,33 @@ bool ModuleTranslation::convertOperation(Operation &opInst,
     llvm::Value *result = convertCall(opInst);
     if (opInst.getNumResults() != 0) {
       valueMapping[opInst.getResult(0)] = result;
-      return false;
+      return success();
     }
     // Check that LLVM call returns void for 0-result functions.
-    return !result->getType()->isVoidTy();
+    return success(result->getType()->isVoidTy());
   }
 
   // Emit branches.  We need to look up the remapped blocks and ignore the block
   // arguments that were transformed into PHI nodes.
   if (auto brOp = dyn_cast<LLVM::BrOp>(opInst)) {
     builder.CreateBr(blockMapping[brOp.getSuccessor(0)]);
-    return false;
+    return success();
   }
   if (auto condbrOp = dyn_cast<LLVM::CondBrOp>(opInst)) {
     builder.CreateCondBr(valueMapping.lookup(condbrOp.getOperand(0)),
                          blockMapping[condbrOp.getSuccessor(0)],
                          blockMapping[condbrOp.getSuccessor(1)]);
-    return false;
+    return success();
   }
 
-  opInst.emitError("unsupported or non-LLVM operation: ") << opInst.getName();
-  return true;
+  return opInst.emitError("unsupported or non-LLVM operation: ")
+         << opInst.getName();
 }
 
 // Convert block to LLVM IR.  Unless `ignoreArguments` is set, emit PHI nodes
 // to define values corresponding to the MLIR block arguments.  These nodes
 // are not connected to the source basic blocks, which may not exist yet.
-bool ModuleTranslation::convertBlock(Block &bb, bool ignoreArguments) {
+LogicalResult ModuleTranslation::convertBlock(Block &bb, bool ignoreArguments) {
   llvm::IRBuilder<> builder(blockMapping[&bb]);
 
   // Before traversing operations, make block arguments available through
@@ -269,11 +269,9 @@ bool ModuleTranslation::convertBlock(Block &bb, bool ignoreArguments) {
         std::distance(predecessors.begin(), predecessors.end());
     for (auto *arg : bb.getArguments()) {
       auto wrappedType = arg->getType().dyn_cast<LLVM::LLVMType>();
-      if (!wrappedType) {
-        emitError(bb.front().getLoc(),
-                  "block argument does not have an LLVM type");
-        return true;
-      }
+      if (!wrappedType)
+        return emitError(bb.front().getLoc(),
+                         "block argument does not have an LLVM type");
       llvm::Type *type = wrappedType.getUnderlyingType();
       llvm::PHINode *phi = builder.CreatePHI(type, numPredecessors);
       valueMapping[arg] = phi;
@@ -282,11 +280,11 @@ bool ModuleTranslation::convertBlock(Block &bb, bool ignoreArguments) {
 
   // Traverse operations.
   for (auto &op : bb) {
-    if (convertOperation(op, builder))
-      return true;
+    if (failed(convertOperation(op, builder)))
+      return failure();
   }
 
-  return false;
+  return success();
 }
 
 // Create named global variables that correspond to llvm.global definitions.
@@ -379,7 +377,7 @@ static llvm::SetVector<Block *> topologicalSort(FuncOp f) {
   return blocks;
 }
 
-bool ModuleTranslation::convertOneFunction(FuncOp func) {
+LogicalResult ModuleTranslation::convertOneFunction(FuncOp func) {
   // Clear the block and value mappings, they are only relevant within one
   // function.
   blockMapping.clear();
@@ -396,11 +394,9 @@ bool ModuleTranslation::convertOneFunction(FuncOp func) {
       // NB: Attribute already verified to be boolean, so check if we can indeed
       // attach the attribute to this argument, based on its type.
       auto argTy = mlirArg->getType().dyn_cast<LLVM::LLVMType>();
-      if (!argTy.getUnderlyingType()->isPointerTy()) {
-        func.emitError(
+      if (!argTy.getUnderlyingType()->isPointerTy())
+        return func.emitError(
             "llvm.noalias attribute attached to LLVM non-pointer argument");
-        return true;
-      }
       if (attr.getValue())
         llvmArg.addAttr(llvm::Attribute::AttrKind::NoAlias);
     }
@@ -421,17 +417,17 @@ bool ModuleTranslation::convertOneFunction(FuncOp func) {
   auto blocks = topologicalSort(func);
   for (auto indexedBB : llvm::enumerate(blocks)) {
     auto *bb = indexedBB.value();
-    if (convertBlock(*bb, /*ignoreArguments=*/indexedBB.index() == 0))
-      return true;
+    if (failed(convertBlock(*bb, /*ignoreArguments=*/indexedBB.index() == 0)))
+      return failure();
   }
 
   // Finally, after all blocks have been traversed and values mapped, connect
   // the PHI nodes to the results of preceding blocks.
   connectPHINodes(func);
-  return false;
+  return success();
 }
 
-bool ModuleTranslation::convertFunctions() {
+LogicalResult ModuleTranslation::convertFunctions() {
   // Declare all functions first because there may be function calls that form a
   // call graph with cycles.
   for (FuncOp function : mlirModule.getOps<FuncOp>()) {
@@ -442,7 +438,7 @@ bool ModuleTranslation::convertFunctions() {
         convertFunctionType(llvmModule->getContext(), function.getType(),
                             function.getLoc(), isVarArgs);
     if (!functionType)
-      return true;
+      return failure();
     llvm::FunctionCallee llvmFuncCst =
         llvmModule->getOrInsertFunction(function.getName(), functionType);
     assert(isa<llvm::Function>(llvmFuncCst.getCallee()));
@@ -456,11 +452,11 @@ bool ModuleTranslation::convertFunctions() {
     if (function.isExternal())
       continue;
 
-    if (convertOneFunction(function))
-      return true;
+    if (failed(convertOneFunction(function)))
+      return failure();
   }
 
-  return false;
+  return success();
 }
 
 std::unique_ptr<llvm::Module> ModuleTranslation::prepareLLVMModule(ModuleOp m) {
diff --git a/mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp b/mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
index 5c34ed160b2..150fb7cfb97 100644
--- a/mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
+++ b/mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
@@ -164,7 +164,7 @@ static bool emitOneBuilder(const Record &record, raw_ostream &os) {
   os << "if (auto op = dyn_cast<" << op.getQualCppClassName()
      << ">(opInst)) {\n";
   os << bs.str() << builderStrRef << "\n";
-  os << "  return false;\n";
+  os << "  return success();\n";
   os << "}\n";
 
   return true;