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

Chris Lattner clattner at apple.com
Fri Jan 15 15:02:28 PST 2010


On Jan 15, 2010, at 2:59 PM, Dale Johannesen wrote:
>>> +  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.

Ok, if it grows much or we add another target, we'll need to commonize  
it.  I'm ok with leaving it here for now.  The target hook will be  
needed when we actually start emitting dwarf, but we can design it  
lazily when we know more.

>>> +    // 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).

Sigh, you're right, the DI* constructors should take a const pointer.   
Please file a bugzilla about this, thanks Dale.

-Chris



More information about the llvm-commits mailing list