[PATCH] D35545: [AArch64, COFF] Interpret .align as power of two for COFF as well

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp:108
   PrivateLabelPrefix = ".L";
+  AlignmentIsInBytes = false;
 }
----------------
mstorsjo wrote:
> efriedma wrote:
> > Is there any particular reason to choose one way or the other here?
> In my case, it is to make it possible to assemble existing handwritten aarch64 gas assembly (that currently targets linux and darwin) for windows as well. For armv7, we already have the same situation where linux/darwin gas assembly also can be built for windows, but only while using the armv7-windows-gnu target, not the armv7-windows-msvc target.
> 
> If you'd prefer that, I could try to make that distinction here as well, and only match the gas behaviour in the -gnu variant.
Should be fine as-is, I think; ARMCOFFMCAsmInfoMicrosoft also sets AlignmentIsInBytes to false.


================
Comment at: test/MC/AArch64/jump-table.s:2
 // RUN: llvm-mc < %s -triple=aarch64-none-linux-gnu -filetype=obj | llvm-readobj -r | FileCheck %s
+// RUN: llvm-mc < %s -triple=aarch64-windows-gnu
 
----------------
mstorsjo wrote:
> efriedma wrote:
> > This is just checking whether llvm-mc crashes; is that intentional?
> It's intentional in the sense that I wanted to test that `.align 3` is interpreted as 8 byte alignment; prior to the code change this fails to assemble since 3 bytes isn't a valid alignment.
I'd prefer a test that actually verifies that the .align directive produces the correct object file.


https://reviews.llvm.org/D35545