[llvm-commits] [llvm] r103757 - in /llvm/trunk: include/llvm/MC/MCParser/AsmParser.h include/llvm/MC/MCStreamer.h lib/MC/MCAsmStreamer.cpp lib/MC/MCMachOStreamer.cpp lib/MC/MCNullStreamer.cpp lib/MC/MCParser/AsmParser.cpp test/MC/AsmParser/directive_tbss.s

[Eric Christopher]

> 
> Hi Eric, thanks for working on this:
> 

Fun!

>> +void MCAsmStreamer::EmitTBSSSymbol(MCSymbol *Symbol, uint64_t Size,
>> +                                   unsigned ByteAlignment) {
>> +  assert(Symbol != NULL && "Symbol shouldn't be NULL!");
>> +  OS << ".tbss ";
>> +  
>> +  // This is a mach-o specific directive and the name requires some mangling.
>> +  OS << *Symbol << "$tlv$init, " << Size;
> 
> The $tlv$init suffix should be added by the caller, and be part of the MCSymbol.
> 
>> +  
>> +  // Demangle the name output.  The trailing characters are guaranteed to be
>> +  // $tlv$init so just strip that off.
>> +  StringRef DemName = Name.substr(0, Name.size() - strlen("$tlv$init"));
> 
> That allows elimination of this logic from the parser, which isn't safe if the suffix doesn't exist.
> 

Yeah, I was pretty unhappy about it and figured I'd be changing it.  The idea is that something like:

__thread a = 0;

will turn into:

.tbss _a+somemangling

.tlv
_a:
   .quad xxxx
   .quad _a+somemangling
   .quad 0

for any thread local symbol - but there's definitely no reason to have a specific mangling.  Something else I'd considered was:

void EmitTBSSSymbol(StringRef Name, uint64_t Size, unsigned ByteAlignment) ...

I just didn't have enough there that i'd be happy about it.

Another was:

void EmitTBSSSymbol(MCSymbol Sym, ...., StringRef Mangle)

but then I'm stuck trying to figure out which symbol I'm mangling.

>> +  // FIXME: Diagnose overflow.
>> +  if (Pow2Alignment < 0)
>> +    return Error(Pow2AlignmentLoc, "invalid '.tbss' alignment, can't be less"
>> +                 "than zero");
> 
> How about "negative" instead of "less than zero" :)

Sure! I'll change the wording in a couple of other places too :)

> 
>> 
>> +++ llvm/trunk/test/MC/AsmParser/directive_tbss.s Thu May 13 20:50:28 2010
>> @@ -0,0 +1,7 @@
>> +# RUN: llvm-mc -triple i386-unknown-unknown %s | FileCheck %s
> 
> This is a darwin-specific directive, you should force a darwin triple.

OK!

-eric