[llvm-commits] [llvm] r126538 - in /llvm/trunk/utils/TableGen: AsmWriterEmitter.cpp AsmWriterEmitter.h

[Chris Lattner]

On May 22, 2011, at 1:12 AM, Bill Wendling wrote:
>> .. and do a linear search?  Instructions don't have many operands, so your linear search is very likely to be faster than a single std::map allocation anyway.
>> 
> Okay. That shouldn't be too bad.

Thanks.

>> You're generating some aliases like this, which aren't any simpler or better:
>> 
>> case X86::SUB_FST0r:
>> ...
>>   if (MI->getNumOperands() == 1 &&
>>       MI->getOperand(0).getReg() == X86::ST0) {
>>     // (SUB_FST0r ST0)
>>     AsmString = "fsub %st(0), %st(0)";
>>     break;
>>   }
>> 
>> Is there a way to filter these out to avoid bloating the generated alias printer?
>> 
> Yes. I added that after this patch.

To be clear, all the examples are from mainline.

> Dan pointed out that some of the aliases are not as nice as what they're aliasing. We can disable them on a case-by-case basis by adding '0' to the end of the InstAlias. (Same for the MOVSD you mentioned below.)

That makes sense for the movsx vs movswl, but the point of this is that tblgen should be able to automatically prune these things out as well.  This is good because it reduces the code size of the generated alias stuff.

>> case X86::MOVSD:
>>   if (MI->getNumOperands() == 0) {
>>     // (MOVSD)
>>     AsmString = "movsd";
>>     break;
>>   }
>>   return false;
>> 
>> Why are you checking the # operands here, this can never have a non-zero number of operands?
> 
> It's probably a bug in how this code is generated that it doesn't recognize a zero number of operands as being impossible.

What do you mean impossible?  My point is this instruction *always* has zero instructions, so this check is always true and shouldn't be generated.

> 
>> Also, this is another example of an alias that isn't worth it.  This is giving us "movsd" instead of "movsl" which isn't cheaper or better in terms of understandability.  This is bloating out the alias table.  I think you need a cost model or something to decide if an alias is better than the aliasee.
>> 
>> case X86::IDIV32r:
>>   if (MI->getNumOperands() == 1 &&
>>       MI->getOperand(0).isReg() &&
>>       regIsInRegisterClass(RC_GR32, MI->getOperand(0).getReg())) {
>>     // (IDIV32r GR32:$src)
>>     AsmString = "idivl $src, %eax";
>>     OpMap["src"] = 0;
>>     break;
>>   }
>>   return false;
>> 
>> This is another example of a case where the alias is longer than the aliased instruction.  A cost model should trim these out.
>> 
> In my opinion, it's not necessarily the length that's a factor, but which form of the instruction gives the best information. I'm hoping that the "opt-out" works sufficiently. If it doesn't, then I can try to come up with a heuristic that can.

We need some heuristic that is better than what is in there now.  I agree that manually opting out on specific cases makes sense (because movsx is shorter and worse than movswl), but tblgen should default to not doing silly things.

>> I really love this functionality, but there still seems to be these two big issues to work out (cost model for aliases + layering violation).
>> 
> Thanks. I'll work to resolve these issues. :-)

Thanks!

-Chris