[llvm-commits] [llvm] r62107 - in /llvm/trunk: lib/Transforms/IPO/Inliner.cpp lib/Transforms/Utils/InlineCost.cpp test/Transforms/Inline/2009-01-12-RecursiveInline.ll
dalej at apple.com
Tue Jan 13 13:46:21 PST 2009
On Jan 12, 2009, at 11:57 PMPST, Duncan Sands wrote:
> Hi Dale,
>> @@ -180,14 +180,12 @@
>> Function *Callee = CS.getCalledFunction();
>> Function *Caller = TheCall->getParent()->getParent();
>> - // Don't inline a directly recursive call.
>> - if (Caller == Callee ||
>> // Don't inline functions which can be redefined at link-time
>> to mean
>> // something else.
>> // FIXME: We allow link-once linkage since in practice all
>> versions of
>> // the function have the same body (C++ ODR) - but the LLVM
>> // of LinkOnceLinkage doesn't require this.
>> - (Callee->mayBeOverridden() && !Callee->hasLinkOnceLinkage())
>> + if ((Callee->mayBeOverridden() && !Callee-
>> >hasLinkOnceLinkage()) ||
> the comment (containing the FIXME) is now indented funny.
I don't agree, I think it was indented funny before. Let's get the
// Don't inline functions which can be redefined at link-time
// something else.
// FIXME: We allow link-once linkage since in practice all
// the function have the same body (C++ ODR) - but the LLVM
// of LinkOnceLinkage doesn't require this.
if ((Callee->mayBeOverridden() && !Callee->hasLinkOnceLinkage()) ||
// Don't inline functions marked noinline.
I think it's better not to put comments into the middle of if
given that somebody thinks that's a good idea, it makes sense to line
up the comments
with the part of the if statement they're describing, rather than the
entire if statement.
This is consistent with what's done with comments after the first.
> Also, it is
> probably OK to inline a function with weak linkage (mayBeOverridden)
Maybe so. What happens if different compilations make different
inlining decisions? Is the linker allowed to require that all copies
of a weak function are the same? I think not.
More information about the llvm-commits