[llvm-commits] [llvm] r82062 - /llvm/trunk/include/llvm/Support/IRBuilder.h

[Chris Lattner]

On Sep 16, 2009, at 11:16 AM, Devang Patel wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=82062&view=rev
> Log:
> Add an interface to attach debugging information with an instruction.

Ok, a couple of quibbles:

> +++ llvm/trunk/include/llvm/Support/IRBuilder.h Wed Sep 16 13:16:11  
> 2009
> @@ -20,6 +20,7 @@
> #include "llvm/GlobalAlias.h"
> #include "llvm/GlobalVariable.h"
> #include "llvm/Function.h"
> +#include "llvm/Metadata.h"

This shouldn't be needed with changes below:

> #include "llvm/LLVMContext.h"
> #include "llvm/ADT/Twine.h"
> #include "llvm/Support/ConstantFolder.h"
> @@ -60,35 +61,38 @@
> class IRBuilder : public Inserter {
>   BasicBlock *BB;
>   BasicBlock::iterator InsertPt;
> +  MDKindID MDKind;
> +  MDNode *CurLocation;
>   LLVMContext &Context;
>   T Folder;

...

>
> +  /// SetCurrentLocation - This specifies the location information  
> used
> +  /// by debugging information.
> +  void SetCurrentLocation(MDNode *L) {
> +    if (MDKind == 0) {

MDKind should be an argument to SetCurrentLocation.  IRBuilder should  
not implicitly look it up.

Also, you renamed SetCurrentLocation -> SetCurrentDebugLocation.  Why  
not rename it to "SetDebugLocation"?

>   /// Insert - Insert and return the specified instruction.
>   template<typename InstTy>
>   InstTy *Insert(InstTy *I, const Twine &Name = "") const {
>     this->InsertHelper(I, Name, BB, InsertPt);
> +    if (CurLocation)
> +      Context.getMetadata().setMD(MDKind, CurLocation, I);

We should have an API on instruction that forwards to the MD api:
    Something like: I->AddMD(MDKind, CurLocation);

Most clients should only poke the "Metadata" class when they register  
their MDKind, and they should pass them around as opaque unsigned's.

-Chris