[llvm-commits] [llvm] r168704 - in /llvm/trunk: lib/CodeGen/AsmPrinter/AsmPrinter.cpp test/CodeGen/ARM/2010-12-15-elf-lcomm.ll test/CodeGen/ARM/elf-lcomm-align.ll

[Ulrich Weigand]

Rafael Espíndola <rafael.espindola at gmail.com> wrote on 29.11.2012 02:27:31:

> Any reason why the integrated assembler should not implement lcomm the
> same way as the external assembler?

What do you mean by "implement lcomm"?  There's two separate issues:

1. How should the AsmParser interpret a ".lcomm" directive in an
   assembler source file?

2. How should EmitLocalCommonSymbol be implemented, either in the
   MCAsmStreamer or in the MCELFStreamer?

As to 1., if we want to achieve full compatibility with gas, we'd have
to implement the precise rules (platform-specific) default alignment
rules gas uses.  This would be done in AsmParser::ParseDirectiveComm,
I assume.  [ But this doesn't matter either way for the case I was
fixing, which is generating code for a compiler-generated global
variable via AsmPrinter::EmitGlobalVariable. ]

As to 2., since EmitLocalCommonSymbol always gets a ByteAlignment
argument, it is arguably correct to precisely honor that argument
in the MCELFStreamer case.  The MCAsmStreamer case is the difficult
one on platforms where the native assembler doesn't support an
alignment argument with its .lcomm directive.  In this case, it is
impossible to implement EmitLocalCommonSymbol precisely, except
for very specific values of ByteAlignment.  This means that in
almost all cases, it would still be incorrect for EmitGlobalVariable
to call EmitLocalCommonSymbol, and since there is no particular
advantage to doing so in the rare cases where it would be correct,
the patch I checked in still seems the best solution ...

Bye,
Ulrich