[COFF, ARM64] Use '//' as comment character in assembly files in GNU environments

[mstorsjo]

Summary

This allows using semicolons for bundling up more than one statement per line. This is used within the mingw-w64 project in some assembly files that contain code for multiple architectures.

[mstorsjo]

An example of where this is used is e.g. https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-crt/stdio/scanf2-template.S (with aarch64 support is added in a patch at https://sourceforge.net/p/mingw-w64/mailman/message/35983275/).

[compnerd]

Please create a special GNU specialization for this.

[mstorsjo]

Limited the change to GNU environments.

[martell]

lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp

72 ↗

(On Diff #109913)

Can we make this consistent with the other Targets?
In ARMMCTargetDesc.cpp we have

else if (TheTriple.isWindowsMSVCEnvironment())
  MAI = new ARMCOFFMCAsmInfoMicrosoft();
else if (TheTriple.isOSWindows())
  MAI = new ARMCOFFMCAsmInfoGNU();

So flip this so it checks if MSVCEnvironment first and if not and still windows do the GNU style.
It also seems we should have
AArch64COFFMCAsmInfo and not AArch64MCAsmInfoCOFF but that is outside the scope of this patch

[mstorsjo]

lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp

72 ↗

(On Diff #109913)

Sure. Then I'll update the existing test in test/MC/AArch64/coff-relocations.s from aarch64-windows to aarch64-windows-msvc, since that test uses ; for comments. Then it might also make sense to add a AArch64COFFMCAsmInfoMicrosoft frontend classs if msvc is the exception, not the default case, right?

[martell]

lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp

72 ↗

(On Diff #109913)

I never clarified why... cygwin, midipix etc. :)

else if (TheTriple.isWindowsMSVCEnvironment())
  MAI = new AArch64MCAsmInfoCOFF();
else if (TheTriple.isOSWindows())
  MAI = new AArch64MCAsmInfoCOFFGNU();

should suffice, though AArch64MCAsmInfoCOFF should be AArch64COFFMCAsmInfoMicrosoft

[martell]

lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp

72 ↗

(On Diff #109913)

I wrote that before seeing your reply.
Yeah that probably makes the most sense here.

[mstorsjo]

Made the GNU case the default. Apparently I didn't have to update the existing test either, because aarch64-windows still triggers isWindowsMSVCEnvironment (but not isKnownWindowsMSVCEnvironment).

[mstorsjo]

Ping @compnerd, any further comments on this?

[martell]

@mstorsjo Shouldn't AArch64MCAsmInfoMicrosoft have the word COFF in it somewhere?

[mstorsjo]

In D36366#838985, @martell wrote:

@mstorsjo Shouldn't AArch64MCAsmInfoMicrosoft have the word COFF in it somewhere?

I guess - I can amend it that way later.

[mstorsjo]

Renamed AArch64MCAsmInfoMicrosoft to AArch64MCAsmInfoMicrosoftCOFF.

llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h

 struct AArch64MCAsmInfoELF : public MCAsmInfoELF {
   explicit AArch64MCAsmInfoELF(const Triple &T);
 };
 
 struct AArch64MCAsmInfoCOFF : public MCAsmInfoCOFF {
   explicit AArch64MCAsmInfoCOFF();
 };
 
+struct AArch64MCAsmInfoMicrosoftCOFF : public AArch64MCAsmInfoCOFF {
+  explicit AArch64MCAsmInfoMicrosoftCOFF();
+};
+
+struct AArch64MCAsmInfoGNUCOFF : public AArch64MCAsmInfoCOFF {
+  explicit AArch64MCAsmInfoGNUCOFF();
+};
+
 } // namespace llvm
 
 #endif

llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp

AArch64MCAsmInfoELF::AArch64MCAsmInfoELF(const Triple &T) {
   ExceptionsType = ExceptionHandling::DwarfCFI;
 
   UseIntegratedAssembler = true;
 
   HasIdentDirective = true;
 }
 
 AArch64MCAsmInfoCOFF::AArch64MCAsmInfoCOFF() {
-  CommentString = ";";
   PrivateGlobalPrefix = ".L";
   PrivateLabelPrefix = ".L";
   AlignmentIsInBytes = false;
   SupportsDebugInformation = true;
   ExceptionsType = ExceptionHandling::WinEH;
 }
+
+AArch64MCAsmInfoMicrosoftCOFF::AArch64MCAsmInfoMicrosoftCOFF() {
+  CommentString = ";";
+}
+
+AArch64MCAsmInfoGNUCOFF::AArch64MCAsmInfoGNUCOFF() {
+  CommentString = "//";
+}

llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp

static MCRegisterInfo *createAArch64MCRegisterInfo(const Triple &Triple) {
   return X;
 }
 
 static MCAsmInfo *createAArch64MCAsmInfo(const MCRegisterInfo &MRI,
                                          const Triple &TheTriple) {
   MCAsmInfo *MAI;
   if (TheTriple.isOSBinFormatMachO())
     MAI = new AArch64MCAsmInfoDarwin();
+  else if (TheTriple.isWindowsMSVCEnvironment())
+    MAI = new AArch64MCAsmInfoMicrosoftCOFF();
   else if (TheTriple.isOSBinFormatCOFF())
-    MAI = new AArch64MCAsmInfoCOFF();
+    MAI = new AArch64MCAsmInfoGNUCOFF();
   else {
     assert(TheTriple.isOSBinFormatELF() && "Invalid target");
     MAI = new AArch64MCAsmInfoELF(TheTriple);
   }
 
   // Initial state of the frame pointer is SP.
   unsigned Reg = MRI.getDwarfRegNum(AArch64::SP, true);
   MCCFIInstruction Inst = MCCFIInstruction::createDefCfa(nullptr, Reg, 0);

llvm/trunk/test/MC/AArch64/coff-gnu.s

+// RUN: llvm-mc -triple aarch64-windows-gnu -filetype obj -o %t.obj %s
+// RUN: llvm-objdump -d %t.obj | FileCheck %s
+
+func:
+// Check that the nop instruction after the semicolon also is handled
+nop; nop
+add x0, x0, #42
+
+// CHECK:  0:       1f 20 03 d5     nop
+// CHECK:  4:       1f 20 03 d5     nop
+// CHECK:  8:       00 a8 00 91     add x0, x0, #42