It would be nice if ConstantExpr’s GEP-building methods used the same naming convention and had the same convenience methods as IRBuilder. (In fact, the naming convention between IRBuilder and the various Constants.h classes desperately needs to be reconciled, a point which I am sure everyone is painfully aware of.)
Another thing I’d like to see is for ConstantArray::get(), ConstantStruct::get() and ConstantVector::get() to have an overload that takes an iterator pair like IRBuilder’s CreateGEP and CreateCall methods. Also useful for creating small structs would be overloads for ConstantStruct::get that took 1-4 arguments, instead of having to create a vector every time. Even better would be an “end with null” version, like StructType::get() does. These should be added to FunctionType::get() as well.
It would be nice if ConstantExpr’s GEP-building methods used the same naming convention and had the same convenience methods as IRBuilder. (In fact, the naming convention between IRBuilder and the various Constants.h classes desperately needs to be reconciled, a point which I am sure everyone is painfully aware of.)
Another thing I’d like to see is for ConstantArray::get(), ConstantStruct::get() and ConstantVector::get() to have an overload that takes an iterator pair like IRBuilder’s CreateGEP and CreateCall methods. Also useful for creating small structs would be overloads for ConstantStruct::get that took 1-4 arguments, instead of having to create a vector every time. Even better would be an “end with null” version, like StructType::get() does. These should be added to FunctionType::get() as well.
By the way, the reason I’m asking this is because I thought I might spend some time adding the convenience methods myself - but if there is a plan underway to fix up the APIs or resolve the incompatibilities with the naming conventions I don’t want to get in the way of that.
All of which are very useful. However, ConstExpression only has:
getGetElementPtr
getInBoundsGetElementPtr
It would be nice if ConstantExpr's GEP-building methods used the
same naming convention and had the same convenience methods as
IRBuilder. (In fact, the naming convention between IRBuilder and the
various Constants.h classes desperately needs to be reconciled, a
point which I am sure everyone is painfully aware of.)
It's a matter of layering. IRBuilder will call into ConstantExpr as needed; the ConstantExpr API is at the same level as the llvm::Instruction hierarchy itself.
Another thing I'd like to see is for ConstantArray::get(),
ConstantStruct::get() and ConstantVector::get() to have an overload
that takes an iterator pair like IRBuilder's CreateGEP and
CreateCall methods. Also useful for creating small structs would be
overloads for ConstantStruct::get that took 1-4 arguments, instead
of having to create a vector every time. Even better would be an
"end with null" version, like StructType::get() does. These should
be added to FunctionType::get() as well.
It would be nice if ConstantExpr’s GEP-building methods used the
same naming convention and had the same convenience methods as
IRBuilder. (In fact, the naming convention between IRBuilder and the
various Constants.h classes desperately needs to be reconciled, a
point which I am sure everyone is painfully aware of.)
It’s a matter of layering. IRBuilder will call into ConstantExpr as needed; the ConstantExpr API is at the same level as the llvm::Instruction hierarchy itself.
I find that I call the static methods in ConstantExpr a lot without going through IRBuilder - hundreds of times in my frontend. My language generates a lot of static data - reflection information, trace tables for the garbage collector, dispatch tables, runtime annotations, static object instances, and so on. (In fact there’s a special helper class “StructBuilder” that automates a lot of the work of building constant structs, such as adding the standard object header and so on.)
I guess what I’m asking is, would there be any resistance to adding additional static members to ConstantExpr to make it more convenient to do constant GEPs?
I find that I call the static methods in ConstantExpr a *lot* without going
through IRBuilder - hundreds of times in my frontend. My language generates a
lot of static data - reflection information, trace tables for the garbage
collector, dispatch tables, runtime annotations, static object instances, and so
on. (In fact there's a special helper class "StructBuilder" that automates a lot
of the work of building constant structs, such as adding the standard object
header and so on.)
I think it would be better to create a ConstBuilder class rather than add stuff
to ConstantExpr.
I find that I call the static methods in ConstantExpr a lot without going
through IRBuilder - hundreds of times in my frontend. My language generates a
lot of static data - reflection information, trace tables for the garbage
collector, dispatch tables, runtime annotations, static object instances, and so
on. (In fact there’s a special helper class “StructBuilder” that automates a lot
of the work of building constant structs, such as adding the standard object
header and so on.)
I think it would be better to create a ConstBuilder class rather than add stuff
to ConstantExpr.
OK I’ll do that. A few questions:
Capitalize method names or not? I see both styles - the style guide says lower case, but it appears that the newer code is using capitalized method names.
Should methods be named “CreateXXX” (like IRBuilder) or “GetXXX” (like ConstantExpr).
Should method comments use @brief or repeat the method name - I see both styles in the code.
I find that I call the static methods in ConstantExpr a lot without going
through IRBuilder - hundreds of times in my frontend. My language generates a
lot of static data - reflection information, trace tables for the garbage
collector, dispatch tables, runtime annotations, static object instances, and so
on. (In fact there’s a special helper class “StructBuilder” that automates a lot
of the work of building constant structs, such as adding the standard object
header and so on.)
I think it would be better to create a ConstBuilder class rather than add stuff
to ConstantExpr.
Well, it doesn’t seem that there’s much interest in a ConstBuilder class, based on the number of replies to my proposal So I’m back to the question as to whether it would be acceptable to add a few extra helper methods to ConstantExpr, ConstStruct, and so on.