[llvm-commits] [llvm] r62286 - in /llvm/trunk: include/llvm/CodeGen/DwarfWriter.h lib/CodeGen/AsmPrinter/DwarfWriter.cpp lib/CodeGen/SelectionDAG/FastISel.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp test/DebugInfo/2009-01-15-dbg_declare.ll

Evan Cheng evan.cheng at apple.com
Fri Jan 16 11:22:41 PST 2009


On Jan 16, 2009, at 10:34 AM, Chris Lattner wrote:

>
> On Jan 16, 2009, at 9:55 AM, Evan Cheng wrote:
>
>> Hi Devang,
>>
>> Is this the right fix or just a work around? Should llvm-gcc be  
>> generating these invalid declare intrinsics in the first place? It  
>> seems like this will increase compile time. It could be noticeable  
>> when it's in fastisel mode, no?
>>
>> +    DIDescriptor DI(GV);
>> +    // Check current version. Allow Version6 for now.
>> +    unsigned Version = DI.getVersion();
>> +    if (Version != DIDescriptor::Version7 && Version !=  
>> DIDescriptor::Version6)
>> +      return false;
>>
>> I know DIDescriptor is light weight. But is it necessary to create  
>> the descriptor just to check the version number? Can you add a  
>> helper function to DebugInfo that examines GV and returns its  
>> corresponding version?
>
> This should be cheap.  DIDescriptor just holds the pointer to the  
> global.  This should inline into:
>
>    unsigned getVersion() const {
>       return getUnsignedField(0) & VersionMask / 
> *VersionMask=0xffff0000*/;
>     }
>
> ->
>
>     unsigned getUnsignedField(unsigned Elt) const {
>       return (unsigned)getUInt64Field(Elt);
>     }
>
> ->
>
>     uint64_t getUInt64Field(unsigned Elt) const;
>
> ->
>
> uint64_t DIDescriptor::getUInt64Field(unsigned Elt) const {
>   if (GV == 0) return 0;
>   Constant *C = GV->getInitializer();
>   if (C == 0 || Elt >= C->getNumOperands())
>     return 0;
>   if (ConstantInt *CI = dyn_cast<ConstantInt>(C->getOperand(Elt)))
>     return CI->getZExtValue();
>   return 0;
> }
>
> Which is pretty much what you have to do in any case.  DIDescriptor  
> shouldn't add over head here as long as it isn't "new"d.

Ok. But it's still seems somewhat awkward to me. Wouldn't a static  
method that does this check seem cleaner to you?

Evan

>
> -Chris
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090116/1be1c4e0/attachment.html>


More information about the llvm-commits mailing list