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

[Martell Malone via Phabricator via llvm-commits]

martell added inline comments.


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp:72
     MAI = new AArch64MCAsmInfoDarwin();
+  else if (TheTriple.isOSBinFormatCOFF() && TheTriple.isGNUEnvironment())
+    MAI = new AArch64MCAsmInfoCOFFGNU();
----------------
martell wrote:
> mstorsjo wrote:
> > martell wrote:
> > > 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
> > > 
> > 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?
> 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`
> 
> 
I wrote that before seeing your reply.
Yeah that probably makes the most sense here.


https://reviews.llvm.org/D36366