DebugFactory

So, one feature of the late, lamented DebugInfoBuilder that I am missing quite badly, and which is not available in the current DIFactory, is the ability to specify structure offsets abstractly. The DebugFactory requires that you pass in structure offset information as ints, whereas DebugInfoBuilder had "offsetOf" and "alignOf" methods, similar to the "sizeOf" trick, that would create the constants for you in a way that didn't require your front end to know the sizes of things. So far I've been able to avoid having any references to target machines in my frontend, but now that I am trying to flesh out my debug info more fully I'm finding it hard to use DIFactory as-is.

What I'd like to see is a set of alternative factory methods in DIFactory, where you pass in an LLVM type, and possibly a GEP index, plus all of the parameters that it can't figure out from looking at the type. So you can eliminate the offset, size, and alignment parameters since those can be figured out from the type.

-- Talin

Is it true that DIFactory can figure this out with Target info in all cases ?

oops... I meant to say "... without Target info..."

// Calculate the size of the specified LLVM type.
Constant * DebugInfoBuilder::getSize(const Type * type) {
    Constant * one = ConstantInt::get(Type::Int32Ty, 1);
    return ConstantExpr::getPtrToInt(
        ConstantExpr::getGetElementPtr(
            ConstantPointerNull::get(PointerType::getUnqual(type)),
            &one, 1), Type::Int32Ty);
}
    
Constant * DebugInfoBuilder::getAlignment(const Type * type) {
    // Calculates the alignment of T using "sizeof({i8, T}) - sizeof(T)"
    return ConstantExpr::getSub(
        getSize(StructType::get(Type::Int8Ty, type, NULL)),
        getSize(type));
}

This was code that was checked in before DebugInfoBuilder was replaced by DIFactory.

This adds code to find the info that FE knows anyway. That is
undesirable IMO. If there is a need, you can do this trick in your FE.

Eh? My point is that the FE doesn’t know this information. A platform-agnostic FE doesn’t know how many bits are in a pointer, for example. By using this code, the knowledge of how big fields are can be deferred until the target is selected.

In other words - with the old code, I could generate valid DWARF debugging information without having a target selected. With the new code, I’m required to select a target before I can generate debug info.

Are ConstantExpr::getAlignOf, getSizeOf, and getOffsetOf what
you're looking for?

Dan

That will work. Now all I need is for the methods of DIFactory to take a Constant* instead of uint64.

ok. Feel free to add
    /// CreateBasicType - Create a basic type like int, float, etc.
    DIBasicType CreateBasicType(DIDescriptor Context, const std::string &Name,
                                DICompileUnit CompileUnit, unsigned LineNumber,
                                uint64_t SizeInBits, uint64_t AlignInBits,
                                uint64_t OffsetInBits, unsigned Flags,
                                unsigned Encoding);

variants that take Constant*, for example,

    /// CreateBasicType - Create a basic type like int, float, etc.
    DIBasicType CreateBasicType(DIDescriptor Context, const std::string &Name,
                                DICompileUnit CompileUnit, unsigned LineNumber,
                                Constant *SizeInBits, Constant *AlignInBits,
                                Constant *OffsetInBits, unsigned Flags,
                                unsigned Encoding);

and so on.

Here is a patch that does just that.

debug_constants.patch (7.53 KB)

This does not work. I'm getting

llvm/tools/clang/lib/CodeGen/CGDebugInfo.cpp: In member function
‘llvm::DIType clang::CodeGen::CGDebugInfo::CreateQualifiedType(clang::QualType,
llvm::DICompileUnit)’:
/Users/yash/clean/llvm/tools/clang/lib/CodeGen/CGDebugInfo.cpp:225:
error: call of overloaded ‘CreateDerivedType(unsigned int&,
llvm::DICompileUnit&, const char [1], llvm::DICompileUnit, int, int,
int, int, int, llvm::DIType&)’ is ambiguous
llvm/include/llvm/Analysis/DebugInfo.h:530: note: candidates are:
llvm::DIDerivedType llvm::DIFactory::CreateDerivedType(unsigned int,
llvm::DIDescriptor, llvm::StringRef, llvm::DICompileUnit, unsigned
int, uint64_t, uint64_t, uint64_t, unsigned int, llvm::DIType)
llvm/include/llvm/Analysis/DebugInfo.h:540: note:
llvm::DIDerivedType llvm::DIFactory::CreateDerivedType(unsigned int,
llvm::DIDescriptor, const std::string&, llvm::DICompileUnit, unsigned
int, llvm::Constant*, llvm::Constant*, llvm::Constant*, unsigned int,
llvm::DIType)

OK so the problem is that the compiler sees ‘0’ and can’t decide whether its an integer or a null pointer of type Constant *. I guess the new functions will have to have slightly different names.

The old patch works, it's just that when you pass "0, 0, 0" for size, align and offset the compiler can't decide which method to call since "0" can be either a pointer or an integer.

I can produce a new patch if you like, but I'm having trouble thinking of good names for the new methods. Alternatively, I suppose we could re-arrange the order of the parameters slightly to make the overloaded methods unambiguous.

I'd like to hear what you think before proceeding any further. I do want to get this in, though, as it has been very useful to my work.

Talin wrote:

How about you pass reference of Constant instead of pointer to Constant ?