[llvm-commits] [llvm] r77740 - in /llvm/trunk: include/llvm/CodeGen/AsmPrinter.h lib/CodeGen/AsmPrinter/AsmPrinter.cpp test/CodeGen/X86/2009-04-17-tls-fast.ll test/CodeGen/X86/tls1-pic.ll test/CodeGen/X86/tls2-pic.ll test/CodeGen/X86/tls3-pic.ll test/CodeGen/X86/tls4-pic.ll utils/TableGen/AsmWriterEmitter.cpp

[David Greene]

Chris Lattner wrote:

> I'm not seeing this kick in.  On X86, I'm seeing stuff like:
> 
> _get_results:
> LBB2_0: ## entry
>         pushl   %ebx
>         pushl   %edi
>         pushl   %esi
>         subl    $48, %esp
>         testl   %edx, %edx
>         movl    %ecx, %esi
>         je      LBB2_4  ## bb
> 
> 
> Shouldn't the esp/edx/esi be lined up?  What am I missing here?

Two things:

- There aren't any tabs in X86InstInfo.td (after the mnemonic, that is)

- FirstOperandColumn = 0 and MaxOperandLength = 0 in X86TargetAsmInfo
   which disables the padding.


>> +/// PadToColumn - This gets called every time a tab is emitted.  If
>> +/// column padding is turned on, we replace the tab with the
>> +/// appropriate amount of padding.  If not, we replace the tab with a
>> +/// space, except for the first operand so that initial operands are
>> +/// always lined up by tabs.
>> +void AsmPrinter::PadToColumn(unsigned Operand) const {
>> +  if (TAI->getOperandColumn(Operand) > 0) {
> 
> Is this just a complex way to test for "FirstOperandColumn != 0" ?

No, it's testing 0 for any operand, not just the first.

>> +    O.PadToColumn(TAI->getOperandColumn(Operand), 1);
>> +  }
>> +  else {
>> +    if (Operand == 1) {
> 
> How about "else if"?

Yep.

>> +      // Emit the tab after the mnemonic.
>> +      O << '\t';
> 
> This comment is borderline self evident.  Can you reword it something 
> like: "If this is the first operand after the mnemonic, we actually do 
> tab it out to make the operands line up.  For others, we just insert 
> space for simplicity."

Sure.

>> @@ -852,6 +733,8 @@
>>     << "  if (TAI->getOperandColumn(1) > 0) {\n"
>>     << "    // Don't emit trailing whitespace, let the column padding 
>> do it.  This\n"
>>     << "    // guarantees that a stray long opcode + tab won't upset 
>> the alignment.\n"
>> +    << "    // We need to handle this special case here because 
>> sometimes the initial\n"
>> +    << "    // mnemonic string includes a tab or space and sometimes 
>> it doesn't.\n"
>>     << "    unsigned OpLength = std::strlen(AsmStrs+(Bits & " << (1 << 
>> AsmStrBits)-1 << "));\n"
>>     << "    if (OpLength > 0 &&\n"
>>     << "        ((AsmStrs+(Bits & " << (1 << AsmStrBits)-1 << 
>> "))[OpLength-1] == ' ' ||\n"
> 
> Should this just be a considered a bug in .td files that don't do this 
> right?  What is the harm of just leaving this out?

Because AsmStrs gets generated by TableGen.  I'm not sure that .td
files actually have control over it since TableGen does the encoding
compaction itself.  Look at X86GenAsdmWriter[1].inc for example.

                              -Dave