[llvm-commits] [llvm] r92838 - in /llvm/trunk/lib/AsmParser: LLParser.cpp LLParser.h

Victor Hernandez vhernandez at apple.com
Fri Jan 8 10:10:28 PST 2010


You are correct, Chris.  I'll fix this.  This also avoids having to analyze and possible patch all the calls to MDNode::get() throughout the source.

Victor

On Jan 8, 2010, at 10:07 AM, Chris Lattner wrote:

> 
> On Jan 6, 2010, at 9:00 AM, Victor Hernandez wrote:
> 
>> Author: hernande
>> Date: Wed Jan  6 11:00:21 2010
>> New Revision: 92838
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=92838&view=rev
>> Log:
>> When parsing function-local metadata, create a function-local MDNode
> 
> Hi Victor,
> 
> I don't understand the purpose of this patch.  From what I can tell, you're basically propagating out 'isFunctionLocal' to the point where an MDNode is created.  However, I don't see why the MDNode ctor needs to know this.  Can't isFunctionLocal be computed in the MDNode ctor with something like this?
> 
> bool isFunctionLocal = false;
> for (unsigned i = 0; i != NumVals; ++i)
>  if (isa<Instruction>(Vals[i]) || isa<Argument>(Vals[i]) || isa<BasicBlock>(Vals[i]) ||
>      (isa<MDNode>(Vals[i]) && cast<MDNode>(Vals[i])->isFunctionLocal()))
>    isFunctionLocal = true;
> 
> ?
> 
> If so, this eliminates the need for this patch.
> 
>> /// resolved constant, metadata, or function-local value
>> bool LLParser::ConvertGlobalOrMetadataValIDToValue(const Type *Ty, ValID &ID,
>>                                                   Value *&V,
>> -                                                   PerFunctionState *PFS) {
>> +                                                   PerFunctionState *PFS,
>> +                                                   bool *isFunctionLocal) {
> 
> If this change is needed, please add the new argument to the doxygen documentation for this method (which explains that it is an out parameter and what it means) and make it a reference instead of a pointer.  Likewise for ParseMDNodeVector.
> 
> -Chris
> 
>> 
>> Modified:
>>   llvm/trunk/lib/AsmParser/LLParser.cpp
>>   llvm/trunk/lib/AsmParser/LLParser.h
>> 
>> Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=92838&r1=92837&r2=92838&view=diff
>> 
>> ==============================================================================
>> --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
>> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Wed Jan  6 11:00:21 2010
>> @@ -548,7 +548,7 @@
>>      ParseType(Ty, TyLoc) ||
>>      ParseToken(lltok::exclaim, "Expected '!' here") ||
>>      ParseToken(lltok::lbrace, "Expected '{' here") ||
>> -      ParseMDNodeVector(Elts, NULL) ||
>> +      ParseMDNodeVector(Elts, NULL, NULL) ||
>>      ParseToken(lltok::rbrace, "expected end of metadata node"))
>>    return true;
>> 
>> @@ -1911,11 +1911,13 @@
>> 
>>    if (EatIfPresent(lltok::lbrace)) {
>>      SmallVector<Value*, 16> Elts;
>> -      if (ParseMDNodeVector(Elts, PFS) ||
>> +      bool isFunctionLocal = false;
>> +      if (ParseMDNodeVector(Elts, PFS, &isFunctionLocal) ||
>>          ParseToken(lltok::rbrace, "expected end of metadata node"))
>>        return true;
>> 
>> -      ID.MDNodeVal = MDNode::get(Context, Elts.data(), Elts.size());
>> +      ID.MDNodeVal = MDNode::get(Context, Elts.data(), Elts.size(),
>> +                                 isFunctionLocal);
>>      ID.Kind = ValID::t_MDNode;
>>      return false;
>>    }
>> @@ -2447,7 +2449,8 @@
>> /// resolved constant, metadata, or function-local value
>> bool LLParser::ConvertGlobalOrMetadataValIDToValue(const Type *Ty, ValID &ID,
>>                                                   Value *&V,
>> -                                                   PerFunctionState *PFS) {
>> +                                                   PerFunctionState *PFS,
>> +                                                   bool *isFunctionLocal) {
>>  switch (ID.Kind) {
>>  case ValID::t_MDNode:
>>    if (!Ty->isMetadataTy())
>> @@ -2461,9 +2464,10 @@
>>    return false;
>>  case ValID::t_LocalID:
>>  case ValID::t_LocalName:
>> -    if (!PFS)
>> +    if (!PFS || !isFunctionLocal)
>>      return Error(ID.Loc, "invalid use of function-local name");
>>    if (ConvertValIDToValue(Ty, ID, V, *PFS)) return true;
>> +    *isFunctionLocal = true;
>>    return false;
>>  default:
>>    Constant *C;
>> @@ -2523,7 +2527,7 @@
>>    return false;
>>  }
>>  default:
>> -    return ConvertGlobalOrMetadataValIDToValue(Ty, ID, V, &PFS);
>> +    return ConvertGlobalOrMetadataValIDToValue(Ty, ID, V, &PFS, NULL);
>>  }
>> 
>>  return V == 0;
>> @@ -3850,7 +3854,7 @@
>> /// Element
>> ///   ::= 'null' | TypeAndValue
>> bool LLParser::ParseMDNodeVector(SmallVectorImpl<Value*> &Elts,
>> -                                 PerFunctionState *PFS) {
>> +                                 PerFunctionState *PFS, bool *isFunctionLocal) {
>>  do {
>>    // Null is a special case since it is typeless.
>>    if (EatIfPresent(lltok::kw_null)) {
>> @@ -3862,7 +3866,7 @@
>>    PATypeHolder Ty(Type::getVoidTy(Context));
>>    ValID ID;
>>    if (ParseType(Ty) || ParseValID(ID, PFS) ||
>> -        ConvertGlobalOrMetadataValIDToValue(Ty, ID, V, PFS))
>> +        ConvertGlobalOrMetadataValIDToValue(Ty, ID, V, PFS, isFunctionLocal))
>>      return true;
>> 
>>    Elts.push_back(V);
>> 
>> Modified: llvm/trunk/lib/AsmParser/LLParser.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.h?rev=92838&r1=92837&r2=92838&view=diff
>> 
>> ==============================================================================
>> --- llvm/trunk/lib/AsmParser/LLParser.h (original)
>> +++ llvm/trunk/lib/AsmParser/LLParser.h Wed Jan  6 11:00:21 2010
>> @@ -294,11 +294,13 @@
>>    bool ParseValID(ValID &ID, PerFunctionState *PFS = NULL);
>>    bool ConvertGlobalValIDToValue(const Type *Ty, ValID &ID, Constant *&V);
>>    bool ConvertGlobalOrMetadataValIDToValue(const Type *Ty, ValID &ID,
>> -                                             Value *&V, PerFunctionState *PFS);
>> +                                             Value *&V, PerFunctionState *PFS,
>> +                                             bool *isFunctionLocal);
>>    bool ParseGlobalValue(const Type *Ty, Constant *&V);
>>    bool ParseGlobalTypeAndValue(Constant *&V);
>>    bool ParseGlobalValueVector(SmallVectorImpl<Constant*> &Elts);
>> -    bool ParseMDNodeVector(SmallVectorImpl<Value*> &, PerFunctionState *PFS);
>> +    bool ParseMDNodeVector(SmallVectorImpl<Value*> &, PerFunctionState *PFS,
>> +                           bool *isFunctionLocal);
>> 
>>    // Function Parsing.
>>    struct ArgInfo {
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 





More information about the llvm-commits mailing list