[PATCH] D154101: [MC] Add three-state parseDirective as a replacement for ParseDirective

Sergei Barannikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 29 08:57:26 PDT 2023


barannikov88 created this revision.
Herald added subscribers: sstefan1, pmatos, asb, Jim, hiraditya, jgravelle-google, sbc100, dylanmckay, dschuff.
Herald added a project: All.
barannikov88 updated this revision to Diff 535834.
barannikov88 added a comment.
barannikov88 added reviewers: MaskRay, benshi001.
barannikov88 published this revision for review.
barannikov88 added a subscriber: benshi001.
Herald added subscribers: llvm-commits, aheejin.
Herald added a project: LLVM.

Use the new method in WebAssembly target



================
Comment at: llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp:700
                                         AVRMCExpr::VK_AVR_None);
-    return false;
+    return ParseStatus::NoMatch;
   }
----------------
@benshi001
There is something odd here. It handles cases like: `.word foo - bar`, emits some data, but then lets the generic parser handle the directive, which results in emitting more data. It also looks like this code path is not covered by object-emission tests. This was introduced in D38029.



Conventionally, parsing methods return false on success and true on
error. However, directive parsing methods need a third state: the
directive is not target specific. This case was detected on "best
effort" strategy: if the target parser did not consume any tokens,
the directive is assumed to be not target-specific.
Some targets fail to follow the convention: they return success after
emitting an error or do not consume the entire line on success, which
resulted in adding more workarounds to parseStatement.

This patch tries to improve the situation by introducing parseDirective
method that returns ParseStatus -- three-state class. The new method
should eventually replace the old one returning bool.

ParseStatus is intentionally implicitly constructible from bool to allow
uses like `return Error(Loc, "message")`. It also has a potential to
replace OperandMatchResulTy as it is more convenient to use due to the
implicit construction from bool and more type safe.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154101

Files:
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/MCTargetAsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp
  llvm/lib/Target/BPF/AsmParser/BPFAsmParser.cpp
  llvm/lib/Target/Lanai/AsmParser/LanaiAsmParser.cpp
  llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp
  llvm/lib/Target/M68k/AsmParser/M68kAsmParser.cpp
  llvm/lib/Target/MSP430/AsmParser/MSP430AsmParser.cpp
  llvm/lib/Target/SystemZ/AsmParser/SystemZAsmParser.cpp
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D154101.535834.patch
Type: text/x-patch
Size: 25479 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230629/6188ab93/attachment.bin>


More information about the llvm-commits mailing list