[llvm-commits] [llvm] r93565 - in /llvm/trunk: include/llvm/Target/Target.td lib/Target/X86/AsmPrinter/X86MCInstLower.cpp

[Dale Johannesen]

On Jan 15, 2010, at 2:46 PMPST, Chris Lattner wrote:

> On Jan 15, 2010, at 2:22 PM, Dale Johannesen wrote:
>> Author: johannes
>> Date: Fri Jan 15 16:22:35 2010
>> New Revision: 93565
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=93565&view=rev
>> Log:
>> DEBUG_VALUE is now variable sized, as it has a
>> target-dependent memory address representation in it.
>> Restore X86 printing of DEBUG_VALUE; lowering is
>> done in X86RegisterInfo using the normal algorithm.
>
> Ok.
>
>> +  case TargetInstrInfo::DEBUG_VALUE: {
>> +    if (!VerboseAsm)
>> +      return;
>> +    O << '\t' << MAI->getCommentString() << "DEBUG_VALUE: ";
>> +    unsigned NOps = MI->getNumOperands();
>
> Do you plan to move this into the target-independent AsmPrinter code?

I can move part of it if you feel strongly; part of it is target  
dependent.  The whole printing-as-comment thing is temporary anyway.   
My expectation is that that will go away before we move on to another  
target, but that could be wrong.

>> +    // cast away const; DIetc do not take const operands for some  
>> reason
>> +    DIVariable V((MDNode*)(MI->getOperand(NOps-1).getMetadata()));
>
> This shouldn't need a cast.

Well, I could "fix" it by changing getMetadata not to return a const*,  
but I'd rather not; metadata is read-only at this point, so it  
*should* return a const*.   The DI* constructors do not take const  
MDNode*.  IMO they ought to, but that's a big change (doing it  
consistently throughout and patching all the uses, I mean).

>> +    O << V.getName();
>> +    O << " <- ";
>> +    if (NOps==3) {
>> +      // Variable is in register
>> +      assert(MI- 
>> >getOperand(0).getType()==MachineOperand::MO_Register);
>> +      printOperand(MI, 0);
>> +    } else {
>> +      // Frame address.  Currently handles ESP or ESP + offset only
>> +      assert(MI- 
>> >getOperand(0).getType()==MachineOperand::MO_Register);
>> +      assert(MI- 
>> >getOperand(3).getType()==MachineOperand::MO_Immediate);
>
> What about EBP?

I'll fix the comment.