[PATCH] D152085: [llvm][MC] Add .pushsection/.popsection support to COFFAsmParser

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Please include more diff context. See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Thanks. This extension looks reasonable to me.

> These directives aren't directly useful, however for frontends that have inline asm support this is really useful.

Agree. If you know which frontend is going to use this, consider mentioning it, otherwise I'll just assume some Clang users will adopt the feature.



================
Comment at: llvm/lib/MC/MCParser/COFFAsmParser.cpp:71
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveCGProfile>(".cg_profile");
+    addDirectiveHandler<&COFFAsmParser::ParseDirectivePushSection>(
+        ".pushsection");
----------------
Add this after `addDirectiveHandler<&COFFAsmParser::ParseDirectiveSection>(".section"); `


================
Comment at: llvm/lib/MC/MCParser/COFFAsmParser.cpp:371
 // Subsections are not supported.
-bool COFFAsmParser::ParseDirectiveSection(StringRef, SMLoc) {
+bool COFFAsmParser::ParseSectionArguments(StringRef, SMLoc) {
   StringRef SectionName;
----------------
While changing signature, consider fixing the function case to `camelCase`.


================
Comment at: llvm/test/MC/COFF/section.s:216
+.popsection
+
+// Back to section data1
----------------
Add some tests with arguments (including section flags)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152085/new/

https://reviews.llvm.org/D152085