On Mar 4, 2010, at 5:41 AM, Daniel Dunbar wrote:
> On Wed, Mar 3, 2010 at 2:30 AM, John McCall <[email protected]> wrote:
>> +static llvm::ConstantInt *getInt32(llvm::LLVMContext &Context, int32_t
>> Value) {
>> + return llvm::ConstantInt::get(llvm::Type::getInt32Ty(Context), Value);
>> +}
>
> I would prefer not to have a one off utility like this dangling in
> CGBuiltin. If we want such a thing, we should consistently use it.
>
> Would it make sense to make this a method on llvm::ConstantInt?
The trouble is that yes, people want these, but they want them at different
widths. I think we probably don't want all of getInt8(), getInt16(),
getInt32(), and get64(). Maybe we do.
I did consider pushing this to CodeGenFunction; I decided not to, because most
of codegen doesn't make constants of constant width. That said, there are
probably other places that would use it.
>> RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
>> unsigned BuiltinID, const CallExpr
>> *E) {
>> // See if we can constant fold this builtin. If so, don't emit it at all.
>> @@ -343,6 +347,20 @@
>>
>> llvm::ConstantInt::get(llvm::Type::getInt32Ty(VMContext), 1));
>> return RValue::get(Address);
>> }
>> + case Builtin::BI__builtin_dwarf_cfa: {
>> + // The offset in bytes from the first argument to the CFA.
>> + //
>> + // Why on earth is this in the frontend? Is there any reason at
>> + // all that the backend can't reasonably determine this while
>> + // lowering llvm.eh.dwarf.cfa()?
>> + //
>> + // TODO: If there's a satisfactory reason, add a target hook for
>> + // this instead of hard-coding 0, which is correct for most targets.
>
> Please use FIXME instead of TODO for easier grepability.
I don't consider these interchangeable. We've also got hundreds of TODOs
already — an order of magnitude fewer than we have FIXMEs, but still, they're
not rare.
> Is this wrong
> for targets we actually have support for? If so, it seems like we
> should go ahead and add the hook and fix it.
What is actual support? It's not wrong for the 3? 4? platforms I bothered to
track down DWARF documentation for. The only reason I haven't introduced the
hook already is because I'm not sure that's the right solution, as noted by the
comment above, which is a question I am actually going to ask people about when
I am around the office tomorrow. :)
John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
