Repository navigation
refactor(warpforthc): deslop the compiler driver - #135
Merged
Merged
Conversation
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.
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.
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.
openInputFile and openOutputFile already handle stdin/stdout and format the error with the path, replacing two hand-written open-and-report blocks.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cleans up the
warpforthcdriver and the PTX extractor it calls, and fixes a parser bug found along the way. Each change is its own commit.User-visible changes
unknown worderror after a parameter or shared-memory name used inside a word, and afterI/J/Kused outside enough loops.emitOperationreturnednullptrboth for unknown words and for words it had already diagnosed. Now it emits the unknown-word error itself. The three existing error tests gainCHECK-NOT: unknown word, and they fail without the fix.warpforthcno longer printsfailed to parse Forth sourceorcompilation pipeline failedafter the located diagnostic that already explains the failure. Every pass failure path ends inapplyPartialConversionor an upstream pass, and both emit their own errors.extractPTXFromModulenow usesmodule.emitErrorinstead ofllvm::errs(), and drops therun warpforth-opthint, which was wrong underwarpforthc. New test:test/Translation/MLIRToPTX/no-binary-error.mlir.cannot open input file '<path>': ...(from MLIR'sopenInputFile/openOutputFile). They used to saycould not open.Internal changes
parseForthSourcealready loads, and drops the four headers kept only for that.mlir-to-ptxregistration uses the typedTranslateFromMLIRRegistrationoverload in place of a hand-writtenModuleOpcast.Verification
check-warpforth: 135/135 pass.warpforthcoutput on everytest/Pipeline/*.forthfile is byte-identical to the output before this branch.check-clang-tidy(Homebrew clang-tidy 21) flagsllvm-prefer-static-over-anonymous-namespaceinForthToMemRef.cppandForthToGPU.cpp. This branch doesn't touch either file. IWYU isn't installed locally, so CI will be the first IWYU run on the include changes.