[mips] [IAS] Implement the .asciiz directive.

[tomatabacu]

Summary

This directive is exactly the same as .asciz, except it's only used by MIPS.
It is used to store null terminated strings in object files.

[tomatabacu]

Rafael, I was wondering if you could help me with the implementation of this directive ?

The .asciiz assembler directive is the same as .asciz, except it's not accepted by all architectures (the only in-tree users are MIPS and XCore).
The problem with MIPS is that it should accept both .asciiz and .asciz (I don't know what XCore is supposed to do, since they haven't implemented an XCoreAsmParser).

I see three ways to implement .ascciiz:

1. Add .asciiz to the DirectiveKindMap in AsmParser (which will make it available for all architectures).
2. Make AsmParser::parseDirectiveAscii public and use it for parsing .asciiz in MipsAsmParser.cpp.
3. Make AsmParser::parseEscapedString public and use it in a reimplementation of AsmParser::parseDirectiveAscii in MipsAsmParser.cpp.

In this patch, I have chosen the 3rd approach, as it seemed the least intrusive.

[tomatabacu]

Ping.

[dsanders]

I'm of the opinion that solution #1 is preferable, primarily for the simplicity of the implementation. I believe there are a couple other targets that define .asciiz as well as .asciz. As you say, this will make it available for all targets but I don't think that's a big problem. I actually think it's nice to have the inter-target consistency.

[tomatabacu]

Ping.

[echristo]

Documentation somewhere for .asciiz? I looked in binutils and couldn't find anything.

-eric

[tomatabacu]

In D7530#135005, @echristo wrote:

Documentation somewhere for .asciiz? I looked in binutils and couldn't find anything.

-eric

Unfortunately, it's undocumented. I think it's a remnant from some old MIPS assemblers.

I'm actually starting to have doubts about supporting .asciiz in LLVM.

I've grepped the Linux kernel and I've found only 2 uses of .asciiz, both in MIPS assembly,
while .asciz is used a lot more, in many different architectures, including MIPS (only once, but that means it's not a compatibility issue).
FreeBSD has a few more uses of .asciiz (about 8, all of them in MIPS assembly), but .asciz still outnumbers .asciiz.

It might be worth it to try to push Linux and FreeBSD to switch to .asciz for MIPS assembly, but, on the other hand, .asciiz is not difficult to support, just a bit ugly.

[dsanders]

I haven't seen any documentation for it either. Just existing usage.

If it's really used as little as it seems then removing support for it from binutils and correcting Linux/FreeBSD/etc. makes sense to me. We'd need binutils to agree to drop support for it though. Otherwise, any patch to switch .asciiz to .asciz will find it difficult to get merged.

[tomatabacu]

In D7530#136593, @dsanders wrote:

We'd need binutils to agree to drop support for it though. Otherwise, any patch to switch .asciiz to .asciz will find it difficult to get merged.

I don't think we *need* GAS to agree to drop support for .asciiz.

To me, this .asciiz situation feels like the VLAIS situation in Clang.
It's true that if GAS stopped supporting .asciiz, it would force everyone to switch, but I think we need to be convincing the developers to actively use and
support the IAS (and the whole Clang/LLVM toolchain), instead of convincing GAS to conform to our implementation.

In this regard, Linux is more problematic, as it seems to me like Clang/LLVM is used by a small subset of their dev community at the moment.

Also, I don't see why people would oppose switching to .asciz. GAS also supports .asciz for MIPS assembly, so there's no friction in switching.

[dsanders]

In D7530#136916, @tomatabacu wrote:

In D7530#136593, @dsanders wrote:

We'd need binutils to agree to drop support for it though. Otherwise, any patch to switch .asciiz to .asciz will find it difficult to get merged.

I don't think we *need* GAS to agree to drop support for .asciiz.

To me, this .asciiz situation feels like the VLAIS situation in Clang.

The difference between the two is that lack of support for VLAIS is easy to justify on the ground that it requires a significant rework to the way all structure members are accessed. On the other hand, it's easy to look at the .asciiz directive and think "surely that's a trivial change, why don't you fix your assembler?".

In short, the battle is worth fighting for VLAIS but it's not worth it for .asciiz.

It's true that if GAS stopped supporting .asciiz, it would force everyone to switch, but I think we need to be convincing the developers to actively use and
support the IAS (and the whole Clang/LLVM toolchain), instead of convincing GAS to conform to our implementation.

In this regard, Linux is more problematic, as it seems to me like Clang/LLVM is used by a small subset of their dev community at the moment.

You won't be able to get everyone to agree to support both assemblers. It's hard enough to convince the developers within a single company to support multiple tools for the same purpose. You'll always have a subset that cares about building with GAS, as subset that cares about IAS, and maybe a subset that cares about both. Together, the three groups will move in the general direction of compatibility but it will break every so often.

Also, I don't see why people would oppose switching to .asciz. GAS also supports .asciz for MIPS assembly, so there's no friction in switching.

Me neither, but we've both witnessed first hand how difficult it is to get the other clang compatibility fixes merged into Linux. It will be a lot easier to support .asciiz in IAS. Not supporting .asciiz is an option, but the battle will be hard (and neverending) while GAS continues to support both directives.

[tomatabacu]

Added a way to create aliases to directives in the target-independent AsmParser.
Add .asciiz as an alias of .asciz in MipsAsmParser.

By doing this, we can reuse the existing implementation of .asciz without enabling .asciiz for all the other supported architectures.

[tomatabacu]

Ping.

[dsanders]

LGTM

[echristo]

Fine with me. Thanks!

-eric

include/llvm/MC/MCParser/MCAsmParser.h

protected: // Can only create subclasses.
   MCAsmParser();
 
 public:
   virtual ~MCAsmParser();
 
   virtual void addDirectiveHandler(StringRef Directive,
                                    ExtensionDirectiveHandler Handler) = 0;
 
+  virtual void addAliasForDirective(StringRef Directive, StringRef Alias) = 0;
+
   virtual SourceMgr &getSourceManager() = 0;
 
   virtual MCAsmLexer &getLexer() = 0;
   const MCAsmLexer &getLexer() const {
     return const_cast<MCAsmParser*>(this)->getLexer();
   }
 
   virtual MCContext &getContext() = 0;

lib/MC/MCParser/AsmParser.cpp

public:
 
   bool Run(bool NoInitialTextSection, bool NoFinalize = false) override;
 
   void addDirectiveHandler(StringRef Directive,
                            ExtensionDirectiveHandler Handler) override {
     ExtensionDirectiveMap[Directive] = Handler;
   }
 
+  void addAliasForDirective(StringRef Directive, StringRef Alias) override {
+    DirectiveKindMap[Directive] = DirectiveKindMap[Alias];
+  }
+
 public:
   /// @name MCAsmParser Interface
   /// {
 
   SourceMgr &getSourceManager() override { return SrcMgr; }
   MCAsmLexer &getLexer() override { return Lexer; }
   MCContext &getContext() override { return Ctx; }
   MCStreamer &getStreamer() override { return Out; }

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

#undef GET_OPERAND_DIAGNOSTIC_TYPES
 
   MipsAsmParser(MCSubtargetInfo &sti, MCAsmParser &parser,
                 const MCInstrInfo &MII, const MCTargetOptions &Options)
       : MCTargetAsmParser(), STI(sti),
         ABI(MipsABIInfo::computeTargetABI(Triple(sti.getTargetTriple()),
                                           sti.getCPU(), Options)) {
     MCAsmParserExtension::Initialize(parser);
 
+    parser.addAliasForDirective(".asciiz", ".asciz");
+
     // Initialize the set of available features.
     setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
     
     // Remember the initial assembler options. The user can not modify these.
     AssemblerOptions.push_back(
                      make_unique<MipsAssemblerOptions>(getAvailableFeatures()));
     
     // Create an assembler options environment for the user to modify.

test/MC/Mips/asciiz-directive-bad.s

+# RUN: not llvm-mc -triple mips-unknown-linux %s 2>&1 | FileCheck %s
+
+  .asciiz 12
+# CHECK: :[[@LINE-1]]:11: error: expected string in '.asciiz' directive
+  .asciiz "a"3
+# CHECK: :[[@LINE-1]]:14: error: unexpected token in '.asciiz' directive
+  .asciiz "a",
+# CHECK: :[[@LINE-1]]:15: error: expected string in '.asciiz' directive

test/MC/Mips/asciiz-directive.s

+# RUN: llvm-mc -triple mips-unknown-linux %s | FileCheck %s
+# .asciiz is exactly the same as .asciz, except it's MIPS-specific.
+
+t1:
+  .asciiz
+# CHECK-LABEL: t1
+
+t2:
+  .asciiz "a"
+# CHECK-LABEL: t2
+# CHECK: .byte 97
+# CHECK: .byte 0
+
+t3:
+  .asciiz "a", "b", "c"
+# CHECK-LABEL: t3
+# CHECK: .byte 97
+# CHECK: .byte 0
+# CHECK: .byte 98
+# CHECK: .byte 0
+# CHECK: .byte 99
+# CHECK: .byte 0
+
+t4:
+  .asciiz "abcdefghijklmnop"
+# CHECK-LABEL: t4
+# CHECK: .ascii "abcdefghijklmnop"
+# CHECK: .byte 0