[llvm-commits] [llvm] r52800 - in /llvm/trunk: include/llvm/CodeGen/MachineDebugInfoDesc.h include/llvm/CodeGen/MachineModuleInfo.h lib/CodeGen/DwarfWriter.cpp lib/CodeGen/MachineDebugInfoDesc.cpp lib/CodeGen/MachineModuleInfo.cpp lib/CodeGen/Sel

Bill Wendling isanbard at gmail.com
Fri Jun 27 16:41:46 PDT 2008


On Fri, Jun 27, 2008 at 4:16 PM, Chris Lattner <clattner at apple.com> wrote:
> On Jun 26, 2008, at 5:09 PM, Bill Wendling wrote:
>> Log:
>> Refactor the DebugInfoDesc stuff out of the MachineModuleInfo file.
>> Clean up
>> some uses of std::vector, where it's return std::vector by value.
>> Yuck!
>
> Nice, much better.  One request: when making a big reorganization
> change like this, it is vastly preferred to move the code without
> changing it at all.  Because you committed the move with the vector
> change, it is impossible to review the vector change.
>
Sorry...

> Can you see if there are files #including MachineModuleInfo.h that can
> change to #include MachineDebugInfo.h instead?
>
>> @@ -0,0 +1,706 @@
>> +//===-- llvm/CodeGen/MachineDebugInfoDesc.h ---------------------*-
>> C++ -*-===//
>
> How about just MachineDebugInfo.h?
>
I'm separating out other stuff into that file.

>> +//
>> =
>> =
>> =
>> ----------------------------------------------------------------------=
>> ==//
>> +//
>> +//
>> +//
>> +//
>> =
>> =
>> =
>> ----------------------------------------------------------------------=
>> ==//
>
> What does this do?
>
Nothing! ;-) Um, I'll come up with a comment...

>> +#ifndef LLVM_CODEGEN_MACHINEDEBUGINFODESC_H
>> +#define LLVM_CODEGEN_MACHINEDEBUGINFODESC_H
>> +
>> +#include "llvm/GlobalValue.h"
>
> Is it possible to just extern the classes that are used?
>
It's using the GlobalValue::LinkageTypes enum. I suppose I could make
it an unsigned int instead?

>> --- llvm/trunk/include/llvm/CodeGen/MachineModuleInfo.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/MachineModuleInfo.h Thu Jun 26
>> 19:09:40 2008
>> @@ -31,18 +31,20 @@
>> #ifndef LLVM_CODEGEN_MACHINEMODULEINFO_H
>> #define LLVM_CODEGEN_MACHINEMODULEINFO_H
>>
>> #include "llvm/GlobalValue.h"
>> #include "llvm/Pass.h"
>> +#include "llvm/ADT/SmallPtrSet.h"
>> +#include "llvm/ADT/SmallVector.h"
>> +#include "llvm/ADT/UniqueVector.h"
>> +#include "llvm/Support/DataTypes.h"
>> +#include "llvm/Support/Dwarf.h"
>
> Does this header depend on MachineDebugInfo.h?  If so, it should
> #include it.
>
No.

>> --- llvm/trunk/lib/CodeGen/DwarfWriter.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/DwarfWriter.cpp Thu Jun 26 19:09:40 2008
>> @@ -20,6 +20,7 @@
>> #include "llvm/Module.h"
>> #include "llvm/Type.h"
>> #include "llvm/CodeGen/AsmPrinter.h"
>> +#include "llvm/CodeGen/MachineDebugInfoDesc.h"
>> #include "llvm/CodeGen/MachineModuleInfo.h"
>
> Are these headers really independent of each other?
>
Yes.

-bw



More information about the llvm-commits mailing list