Mon Feb 23 23:49:20 PST 2009

Also what about tblgen changes?

The code is still in X86GenAsmWriter.inc:

   if (TAI->doesSupportDebugInformation() &&
       DW->ShouldEmitDwarfDebug()) {
     const MachineFunction *MF = MI->getParent()->getParent();
     DebugLoc CurDL = MI->getDebugLoc();

     if (!CurDL.isUnknown()) {
       static DebugLocTuple PrevDLT(~0U, ~0U, ~0U);
       DebugLocTuple CurDLT = MF->getDebugLocTuple(CurDL);

       if (PrevDLT.Src != ~0U && PrevDLT != CurDLT)
	printLabel(DW->RecordSourceLine(CurDLT.Line, CurDLT.Col,
                                         CurDLT.Src));

       PrevDLT = CurDLT;
     }
   }

This should not be run when "fast", right?

While we are at it, it looks like this code should be simplified. If  
TAI->doesSupportDebugInformation()  is false, DW- 
 >ShouldEmitDwarfDebug() should not return true. That means only the  
later check is needed?

Also, please make MF AsmPrinter class ivar so we can eliminate this:
const MachineFunction *MF = MI->getParent()->getParent();

Evan

On Feb 23, 2009, at 11:19 PM, Evan Cheng wrote:

>
> On Feb 23, 2009, at 11:11 PM, Bill Wendling wrote:
>
>> On Feb 23, 2009, at 10:20 PM, Evan Cheng wrote:
>>
>>> On Feb 23, 2009, at 6:35 PM, Bill Wendling wrote:
>>>
>>>> -
>>>> +
>>>> +  /// FastCodeGen - True if generating code via the "fast" isel.
>>>> +  ///
>>>> +  bool FastCodeGen;
>>>> public:
>>>> static char ID; // Pass identification, replacement for typeid
>>>>
>>>> @@ -104,6 +107,9 @@
>>>> /// ShouldEmitDwarfDebug - Returns true if Dwarf debugging
>>>> declarations should
>>>> /// be emitted.
>>>> bool ShouldEmitDwarfDebug() const;
>>>> +
>>>> +  bool getFastCodeGen() const { return FastCodeGen; }
>>>> +  void setFastCodeGen(bool Fast) { FastCodeGen = Fast; }
>>>> };
>>>
>>> Bill, is this necessary? See LLVMTargetMachine.cpp:
>>>
>>> case TargetMachine::AssemblyFile:
>>>   if (addAssemblyEmitter(PM, Fast, Out))
>>>     return FileModel::Error;
>>>
>>> It's passing in "Fast".
>>>
>> Possibly? Though the code is weird. Here's the X86 version:
>>
>> bool X86TargetMachine::addAssemblyEmitter(PassManagerBase &PM, bool
>> Fast,
>>                                          raw_ostream &Out) {
>>  assert(AsmPrinterCtor && "AsmPrinter was not linked in");
>>  if (AsmPrinterCtor)
>>    PM.add(AsmPrinterCtor(Out, *this));
>>  return false;
>> }
>>
>> Note that the "if" conditional is redundant because of the assert.
>> Also, it doesn't use the "Fast" boolean. But I'll look into it.
>
> Please do. I'd rather "fast" be set at pass initialization time (it's
> ugly enough as it is).
>
>>
>>
>>>> @@ -1289,9 +1292,8 @@
>>>>
>>>>      unsigned Line = DSP->getLine();
>>>>      unsigned Col = DSP->getColumn();
>>>> -        const Function *F =  
>>>> DAG.getMachineFunction().getFunction();
>>>>
>>>> -        if (!F->hasFnAttr(Attribute::OptimizeForSize)) {
>>>> +        if (Fast) {
>>>>        // A bit self-referential to have DebugLoc on Debug_Loc
>>>> nodes, but it
>>>>        // won't hurt anything.
>>>>        if (useDEBUG_LOC) {
>>>
>>> Ewww. Legalizer shouldn't have to do this.
>>
>> Yeah. The whole debugging thing is gross and does things it  
>> shouldn't.
>>
>>> Is this code even
>>> exercised? I don't see ISD::DBG_STOPPOINT ever being generated. If
>>> not, we should just make the default expansion to its chain.
>>>
>> I'm pretty sure I had to do something like this. It's easy to  
>> check, I
>> suppose.
>
> If for some reason legalizer still has to do this, this is as good a
> time as any to fix it.
>
> Thanks,
>
> Evan
>
>>
>>
>> -bw
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits