From 4f28d89676ff83b0a8c296a3f06ec88952156290 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Sat, 24 Oct 2026 22:13:17 +1100 Subject: [PATCH 1/6] fix(translate): report each failed word once emitOperation returned nullptr both for unknown words and for words it had already diagnosed (parameter or shared names inside a word, I/J/K outside enough loops). The caller treated every nullptr as unknown, so those cases printed a second, misleading "unknown word" error. emitOperation now emits the unknown-word diagnostic itself, so nullptr always means already reported. --- lib/Translation/ForthToMLIR/ForthToMLIR.cpp | 4 ++-- lib/Translation/ForthToMLIR/ForthToMLIR.h | 2 +- test/Translation/Forth/loop-index-depth-error.forth | 1 + test/Translation/Forth/param-ref-in-word-error.forth | 1 + test/Translation/Forth/shared-ref-in-word-error.forth | 1 + 5 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/Translation/ForthToMLIR/ForthToMLIR.cpp b/lib/Translation/ForthToMLIR/ForthToMLIR.cpp index 664dd13..aa9f726 100644 --- a/lib/Translation/ForthToMLIR/ForthToMLIR.cpp +++ b/lib/Translation/ForthToMLIR/ForthToMLIR.cpp @@ -905,7 +905,7 @@ Value ForthParser::emitOperation(StringRef word, Value inputStack, .getOutputStack(); } - // Unknown word + (void)emitError("unknown word: " + word); return nullptr; } @@ -1281,7 +1281,7 @@ LogicalResult ForthParser::parseBody(Value &stack) { } else { Value newStack = emitOperation(currentToken.text, stack, loc); if (!newStack) - return emitError("unknown word: " + currentToken.text); + return failure(); stack = newStack; consume(); } diff --git a/lib/Translation/ForthToMLIR/ForthToMLIR.h b/lib/Translation/ForthToMLIR/ForthToMLIR.h index 42502fc..04317f6 100644 --- a/lib/Translation/ForthToMLIR/ForthToMLIR.h +++ b/lib/Translation/ForthToMLIR/ForthToMLIR.h @@ -144,7 +144,7 @@ class ForthParser { Location getLoc(); /// Emit a Forth operation based on the current token. - /// Returns the updated stack value or nullptr on error. + /// Returns the updated stack value, or nullptr after emitting a diagnostic. Value emitOperation(StringRef word, Value inputStack, Location loc); /// Create a new block with a single !forth.stack argument, appended to diff --git a/test/Translation/Forth/loop-index-depth-error.forth b/test/Translation/Forth/loop-index-depth-error.forth index fcb1cf9..93dfc58 100644 --- a/test/Translation/Forth/loop-index-depth-error.forth +++ b/test/Translation/Forth/loop-index-depth-error.forth @@ -1,4 +1,5 @@ \ RUN: %not %warpforth-translate --forth-to-mlir %s 2>&1 | %FileCheck %s \ CHECK: 'J' requires 2 nested DO/LOOP(s) +\ CHECK-NOT: unknown word \! kernel main 10 0 DO J LOOP diff --git a/test/Translation/Forth/param-ref-in-word-error.forth b/test/Translation/Forth/param-ref-in-word-error.forth index 283e234..a83cae5 100644 --- a/test/Translation/Forth/param-ref-in-word-error.forth +++ b/test/Translation/Forth/param-ref-in-word-error.forth @@ -1,5 +1,6 @@ \ RUN: %not %warpforth-translate --forth-to-mlir %s 2>&1 | %FileCheck %s \ CHECK: parameter 'DATA' cannot be referenced inside a word definition +\ CHECK-NOT: unknown word \! kernel main \! param DATA i64[256] : BAD-WORD DATA @ ; diff --git a/test/Translation/Forth/shared-ref-in-word-error.forth b/test/Translation/Forth/shared-ref-in-word-error.forth index 5f7b3f0..db3ab34 100644 --- a/test/Translation/Forth/shared-ref-in-word-error.forth +++ b/test/Translation/Forth/shared-ref-in-word-error.forth @@ -1,5 +1,6 @@ \ RUN: %not %warpforth-translate --forth-to-mlir %s 2>&1 | %FileCheck %s \ CHECK: shared memory 'SCRATCH' cannot be referenced inside a word definition +\ CHECK-NOT: unknown word \! kernel main \! shared SCRATCH i64[256] : BAD-WORD SCRATCH @ ; From cbe9035dcc27340f72159877dbb2cf7638b0a97f Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Sat, 24 Oct 2026 22:14:11 +1100 Subject: [PATCH 2/6] refactor(warpforthc): drop redundant failure messages The parser and the pipeline passes already report located diagnostics through the SourceMgr handler, so the trailing "failed to parse Forth source" and "compilation pipeline failed" lines only repeated them. --- tools/warpforthc/warpforthc.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tools/warpforthc/warpforthc.cpp b/tools/warpforthc/warpforthc.cpp index bd13536..1f2758f 100644 --- a/tools/warpforthc/warpforthc.cpp +++ b/tools/warpforthc/warpforthc.cpp @@ -88,10 +88,8 @@ int main(int argc, char **argv) { SourceMgrDiagnosticHandler diagHandler(sourceMgr, &context); auto module = forth::parseForthSource(sourceMgr, &context); - if (!module) { - llvm::errs() << "error: failed to parse Forth source\n"; + if (!module) return 1; - } // Run the compilation pipeline PassManager pm(&context); @@ -100,10 +98,8 @@ int main(int argc, char **argv) { warpforth::WarpForthPipelineOptions pipelineOptions; pipelineOptions.chip = targetArchitecture.getValue(); warpforth::buildWarpForthPipeline(pm, pipelineOptions); - if (failed(pm.run(*module))) { - llvm::errs() << "error: compilation pipeline failed\n"; + if (failed(pm.run(*module))) return 1; - } // Extract PTX and write to output std::error_code ec; From 86118ab8401b4c6ee21c295551ee6862b0de5efb Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Sat, 24 Oct 2026 22:14:39 +1100 Subject: [PATCH 3/6] refactor(warpforthc): stop registering the parser's dialects parseForthSource already loads the Forth, arith, cf, func and memref dialects it emits. The driver's own registry.insert repeated that list and kept four headers alive only for it. --- tools/warpforthc/warpforthc.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tools/warpforthc/warpforthc.cpp b/tools/warpforthc/warpforthc.cpp index 1f2758f..ed4a960 100644 --- a/tools/warpforthc/warpforthc.cpp +++ b/tools/warpforthc/warpforthc.cpp @@ -8,10 +8,7 @@ #include "mlir/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.h" #include "mlir/Conversion/FuncToLLVM/ConvertFuncToLLVM.h" #include "mlir/Conversion/MemRefToLLVM/MemRefToLLVM.h" -#include "mlir/Dialect/ControlFlow/IR/ControlFlow.h" // IWYU pragma: keep #include "mlir/Dialect/Func/Extensions/InlinerExtension.h" -#include "mlir/Dialect/Func/IR/FuncOps.h" // IWYU pragma: keep -#include "mlir/Dialect/MemRef/IR/MemRef.h" // IWYU pragma: keep #include "mlir/IR/AsmState.h" #include "mlir/IR/BuiltinOps.h" #include "mlir/IR/Diagnostics.h" @@ -23,7 +20,6 @@ #include "mlir/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.h" #include "mlir/Target/LLVMIR/Dialect/NVVM/NVVMToLLVMIRTranslation.h" #include "warpforth/Conversion/Passes.h" -#include "warpforth/Dialect/Forth/ForthDialect.h" // IWYU pragma: keep #include "warpforth/Translation/ForthToMLIR/ForthToMLIR.h" #include "warpforth/Translation/MLIRToPTX/MLIRToPTX.h" #include "llvm/Support/CommandLine.h" @@ -68,10 +64,6 @@ int main(int argc, char **argv) { // Parse Forth source to MLIR DialectRegistry registry; - registry - .insert(); - arith::registerConvertArithToLLVMInterface(registry); cf::registerConvertControlFlowToLLVMInterface(registry); registerConvertFuncToLLVMInterface(registry); From 0fa34b72737463a6f76bf7cb1c609fc98de7c9ce Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Sat, 24 Oct 2026 22:15:07 +1100 Subject: [PATCH 4/6] refactor(warpforthc): open files with MLIR's FileUtilities helpers openInputFile and openOutputFile already handle stdin/stdout and format the error with the path, replacing two hand-written open-and-report blocks. --- tools/warpforthc/warpforthc.cpp | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/tools/warpforthc/warpforthc.cpp b/tools/warpforthc/warpforthc.cpp index ed4a960..e8e77fb 100644 --- a/tools/warpforthc/warpforthc.cpp +++ b/tools/warpforthc/warpforthc.cpp @@ -14,6 +14,7 @@ #include "mlir/IR/Diagnostics.h" #include "mlir/IR/MLIRContext.h" #include "mlir/Pass/PassManager.h" +#include "mlir/Support/FileUtilities.h" #include "mlir/Target/LLVM/NVVM/Target.h" #include "mlir/Target/LLVMIR/Dialect/Builtin/BuiltinToLLVMIRTranslation.h" #include "mlir/Target/LLVMIR/Dialect/GPU/GPUToLLVMIRTranslation.h" @@ -23,7 +24,6 @@ #include "warpforth/Translation/ForthToMLIR/ForthToMLIR.h" #include "warpforth/Translation/MLIRToPTX/MLIRToPTX.h" #include "llvm/Support/CommandLine.h" -#include "llvm/Support/FileSystem.h" #include "llvm/Support/InitLLVM.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/ToolOutputFile.h" @@ -52,15 +52,15 @@ int main(int argc, char **argv) { "WarpForth compiler: Forth to PTX\n"); // Set up source manager with input file - auto inputFile = llvm::MemoryBuffer::getFileOrSTDIN(inputFilename); - if (auto ec = inputFile.getError()) { - llvm::errs() << "error: could not open input file '" << inputFilename - << "': " << ec.message() << "\n"; + std::string errorMessage; + auto inputFile = openInputFile(inputFilename, &errorMessage); + if (!inputFile) { + llvm::errs() << "error: " << errorMessage << "\n"; return 1; } llvm::SourceMgr sourceMgr; - sourceMgr.AddNewSourceBuffer(std::move(*inputFile), llvm::SMLoc()); + sourceMgr.AddNewSourceBuffer(std::move(inputFile), llvm::SMLoc()); // Parse Forth source to MLIR DialectRegistry registry; @@ -94,18 +94,16 @@ int main(int argc, char **argv) { return 1; // Extract PTX and write to output - std::error_code ec; - llvm::ToolOutputFile outputFile(outputFilename, ec, llvm::sys::fs::OF_Text); - if (ec) { - llvm::errs() << "error: could not open output file '" << outputFilename - << "': " << ec.message() << "\n"; + auto outputFile = openOutputFile(outputFilename, &errorMessage); + if (!outputFile) { + llvm::errs() << "error: " << errorMessage << "\n"; return 1; } - if (failed(warpforth::extractPTXFromModule(*module, outputFile.os()))) { + if (failed(warpforth::extractPTXFromModule(*module, outputFile->os()))) { return 1; } - outputFile.keep(); + outputFile->keep(); return 0; } From da446e73f233744179faa17f19109f86e34d44ae Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Sat, 24 Oct 2026 22:15:17 +1100 Subject: [PATCH 5/6] style(warpforthc): drop narrating comments and stray braces The four step comments restated the next line, and one sat above the registry setup rather than the parse. The last single-statement if kept braces that no other if in the file uses. --- tools/warpforthc/warpforthc.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tools/warpforthc/warpforthc.cpp b/tools/warpforthc/warpforthc.cpp index e8e77fb..413934d 100644 --- a/tools/warpforthc/warpforthc.cpp +++ b/tools/warpforthc/warpforthc.cpp @@ -51,7 +51,6 @@ int main(int argc, char **argv) { llvm::cl::ParseCommandLineOptions(argc, argv, "WarpForth compiler: Forth to PTX\n"); - // Set up source manager with input file std::string errorMessage; auto inputFile = openInputFile(inputFilename, &errorMessage); if (!inputFile) { @@ -62,7 +61,6 @@ int main(int argc, char **argv) { llvm::SourceMgr sourceMgr; sourceMgr.AddNewSourceBuffer(std::move(inputFile), llvm::SMLoc()); - // Parse Forth source to MLIR DialectRegistry registry; arith::registerConvertArithToLLVMInterface(registry); cf::registerConvertControlFlowToLLVMInterface(registry); @@ -83,7 +81,6 @@ int main(int argc, char **argv) { if (!module) return 1; - // Run the compilation pipeline PassManager pm(&context); if (failed(applyPassManagerCLOptions(pm))) return 1; @@ -93,16 +90,14 @@ int main(int argc, char **argv) { if (failed(pm.run(*module))) return 1; - // Extract PTX and write to output auto outputFile = openOutputFile(outputFilename, &errorMessage); if (!outputFile) { llvm::errs() << "error: " << errorMessage << "\n"; return 1; } - if (failed(warpforth::extractPTXFromModule(*module, outputFile->os()))) { + if (failed(warpforth::extractPTXFromModule(*module, outputFile->os()))) return 1; - } outputFile->keep(); return 0; From 540d04f72fa3435f8fe70661b2b1af5b3fe122d6 Mon Sep 17 00:00:00 2001 From: Alex Cameron Date: Sat, 24 Oct 2026 22:16:07 +1100 Subject: [PATCH 6/6] refactor(translate): report PTX extraction errors as diagnostics extractPTXFromModule wrote straight to llvm::errs(), bypassing the diagnostic handler, and its hint to run warpforth-opt was wrong when warpforthc was the caller. It now uses module.emitError. The mlir-to-ptx registration passes the function to the typed TranslateFromMLIRRegistration overload, which does the ModuleOp cast and error itself. --- lib/Translation/MLIRToPTX/MLIRToPTX.cpp | 17 +++-------------- test/Translation/MLIRToPTX/no-binary-error.mlir | 6 ++++++ 2 files changed, 9 insertions(+), 14 deletions(-) create mode 100644 test/Translation/MLIRToPTX/no-binary-error.mlir diff --git a/lib/Translation/MLIRToPTX/MLIRToPTX.cpp b/lib/Translation/MLIRToPTX/MLIRToPTX.cpp index 0b1a148..8c6c680 100644 --- a/lib/Translation/MLIRToPTX/MLIRToPTX.cpp +++ b/lib/Translation/MLIRToPTX/MLIRToPTX.cpp @@ -31,11 +31,8 @@ LogicalResult mlir::warpforth::extractPTXFromModule(ModuleOp module, } }); - if (!foundBinary) { - llvm::errs() << "error: no gpu.binary operations found in module\n"; - llvm::errs() << "hint: run 'warpforth-opt --warpforth-pipeline' first\n"; - return failure(); - } + if (!foundBinary) + return module.emitError("no gpu.binary operations found in module"); return success(); } @@ -43,15 +40,7 @@ LogicalResult mlir::warpforth::extractPTXFromModule(ModuleOp module, void mlir::warpforth::registerMLIRToPTXTranslation() { TranslateFromMLIRRegistration registration( "mlir-to-ptx", "Extract PTX assembly from gpu.binary operations", - [](Operation *op, llvm::raw_ostream &output) { - auto module = dyn_cast(op); - if (!module) { - llvm::errs() << "error: expected ModuleOp at top level\n"; - return failure(); - } - return warpforth::extractPTXFromModule(module, output); - }, - [](DialectRegistry ®istry) { + warpforth::extractPTXFromModule, [](DialectRegistry ®istry) { registry.insert(); }); } diff --git a/test/Translation/MLIRToPTX/no-binary-error.mlir b/test/Translation/MLIRToPTX/no-binary-error.mlir new file mode 100644 index 0000000..ba70bd6 --- /dev/null +++ b/test/Translation/MLIRToPTX/no-binary-error.mlir @@ -0,0 +1,6 @@ +// RUN: %not %warpforth-translate --mlir-to-ptx %s 2>&1 | %FileCheck %s + +// CHECK: no-binary-error.mlir:[[@LINE+2]]:1: error: no gpu.binary operations found in module +// CHECK-NOT: hint +module { +}