[llvm-commits] [llvm-gcc-4.2] r97676 - in /llvm-gcc-4.2/trunk/gcc: llvm-backend.cpp llvm-internal.h llvm-types.cpp tree.c tree.h

Stuart Hastings stuart at apple.com
Thu Mar 4 13:43:53 PST 2010


On Mar 4, 2010, at 12:54 AM, Duncan Sands wrote:

> Hi Stuart,
> 
>> +  // Random initialization of TypeRefinementDatabase if we're using a
>> +  // PCH.  Won't work until the GCC PCH has been read in and digested.
> 
> what do you mean by "random initialization"?

This function has a name patterned after two similar functions that restore LLVM data structures from a PCH.  However, where those functions extract data from a bitcode file embedded in the PCH, this function extracts data saved in the GCC section.  Alas, it seems this function can't be invoked with its cousins, because the GCC part of the PCH hasn't yet been retrieved from disk when they're invoked.  Locating the call at this spot clearly solves the problem (late enough that the PCH has been read, but before LLVM uses the data stored in TypeRefinementDatabase::TypeUsers[]), but it still seems "wrong" to me.  In fact, it looked to me like a very "random" spot to call this function, thus the comment.  Perhaps I should give the function C linkage and invoke it from the GCC PCH logic?

Regardless of the above, I agree that the word "Random" is confusing here, so I've revised the comment.

>> +// We've just read in a PCH; retrieve the set of GCC types that were
>> +// known to TypeUsers[], and re-populate it.  Intended to be called
>> +// once, but harmless if called multiple times, or if no PCH is
>> +// present.
>> +void readLLVMTypeUsers() {
> 
> This comment does not follow LLVM's doxygen style.

Revised.

>> +// Record the set of GCC types currently known to TypeUsers[] inside
>> +// GCC so they will be preserved in a PCH.  Intended to be called
>> +// once, just before the PCH is written.
>> +void writeLLVMTypeUsers() {
> 
> Likewise.

Revised.

>> +  std::map<const Type*, std::vector<tree>  >::iterator
>> +    I = TypeDB.TypeUsers.begin(),
>> +    E = TypeDB.TypeUsers.end();
>> +  for (; I != E; I++)
> 
> Normal LLVM style is to use ++I

Done.

>> +    for (unsigned i = 0, e = I->second.size(); i != e; ++i)
>> +      llvm_push_TypeUsers(I->second[i]);
>> +}
>>  //===----------------------------------------------------------------------===//
> 
> Missing blank line between the end of the function and the following comment.

Sorry, fixed.

stuart



More information about the llvm-commits mailing list