<div dir="ltr">I feel OK about this trade-off. The vast majority of C++ TUs have < 65k functions. Sanjoy?</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 21, 2015 at 2:25 PM, Vedant Kumar <span dir="ltr"><<a href="mailto:vsk@apple.com" target="_blank">vsk@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I've done some measurements on this.<br>
<br>
The test program I have just calls Function::Create(), F->setPersonalityFn(), and then F->eraseFromParent() in a loop 2^20 times.<br>
<br>
Results:<br>
<br>
pre-patch --- min: 1.10s max: 1.13s avg: 1.11s<br>
post-patch --- min: 1.26s max: 1.35s avg: 1.29s<br>
<br>
So we expect to lose 0.2 seconds per 1 million functions (with personality functions) in a project. I've only tested on my machine, and I haven't accounted for performance variations in other parts of the codebase where code was removed.<br>
<br>
I'm happy enough with the patch and think that this is a reasonable tradeoff. Should we go with this?<br>
<br>
<a href="http://reviews.llvm.org/D13829" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13829</a><br>
<span class="HOEnZb"><font color="#888888"><br>
vedant<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
> On Oct 16, 2015, at 5:56 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
><br>
><br>
>> On 2015-Oct-16, at 13:54, Vedant Kumar <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> wrote:<br>
>><br>
>> Here is a WIP patch as promised:<br>
>><br>
>> <a href="http://reviews.llvm.org/D13829" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13829</a><br>
>><br>
>> It uses a hungoff uselist to store optional data as needed.<br>
>><br>
>> Some early objections from Duncan:<br>
>><br>
>> - An extra one-time malloc() is required to set personality functions.<br>
>> - We get and set personality functions frequently. This patch introduces a level of indirection which slows the common case down.<br>
>><br>
>> Is this overhead acceptable?<br>
><br>
> If the overhead isn't bad, then this SGTM. (I haven't looked at the<br>
> patch.)<br>
><br>
> I guess I was thinking about large applications that use C++ exceptions<br>
> and build with LTO, but even there it's a per-function overhead. I<br>
> doubt it really matters.<br>
><br>
>> If not, maybe we could leave personality function handling untouched and add a "Constant **OptionalData" array to Function.<br>
><br>
> The `IndirectUser` approach you outlined initially might be better<br>
> than this?<br>
><br>
>> vedant<br>
>><br>
>><br>
>>> On Oct 14, 2015, at 3:12 PM, Vedant Kumar <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> wrote:<br>
>>><br>
>>> I like the idea of using hung off uses.<br>
>>><br>
>>> We can keep using SubclassData to indicate whether or not some optional data is present.<br>
>>><br>
>>> Benefits: zero space overhead until some optional data is set, we can get rid of the DenseMaps in LLVMContextImpl, and RAUW just works (so no clang changes are needed).<br>
>>><br>
>>> I'll have a patch up before the end of the week.<br>
>>><br>
>>> thanks<br>
>>> vedant<br>
>>><br>
>>><br>
>>>> On Oct 12, 2015, at 12:15 PM, Sanjoy Das via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br>
>>>><br>
>>>><br>
>>>><br>
>>>> David Majnemer wrote:<br>
>>>>><br>
>>>>><br>
>>>>> On Mon, Oct 12, 2015 at 11:00 AM, Duncan P. N. Exon Smith via llvm-dev<br>
>>>>> <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>>> wrote:<br>
>>>>><br>
>>>>><br>
>>>>>> On 2015-Oct-12, at 10:41, Sanjoy Das<br>
>>>>> <<a href="mailto:sanjoy@playingwithpointers.com">sanjoy@playingwithpointers.com</a><br>
>>>>> <mailto:<a href="mailto:sanjoy@playingwithpointers.com">sanjoy@playingwithpointers.com</a>>> wrote:<br>
>>>>>><br>
>>>>>><br>
>>>>>><br>
>>>>>> Vedant Kumar wrote:<br>
>>>>>>>>> That's a neat idea. To summarize: make Function have 3 optional<br>
>>>>> operands. (For context -- Function currently has 1 optional operand,<br>
>>>>> and my proposal is to move to 0.)<br>
>>>>>>>>><br>
>>>>>>>>> Could someone else chime in on what they'd like to see?<br>
>>>>>>>> Sanjoy's idea makes sense to me, but only if we never need to add<br>
>>>>>>>> prefix/prologue data after functions are created. Are there any<br>
>>>>> places<br>
>>>>>>>> where we need/want to add them after the fact?<br>
>>>>>>><br>
>>>>>>> I think so. I see:<br>
>>>>>>><br>
>>>>>>> LinkModules.cpp:<br>
>>>>> Dst.setPrefixData(MapValue(Src.getPrefixData(), ValueMap,<br>
>>>>>>> BitcodeReader.cpp:<br>
>>>>> FunctionPrologueWorklist.back().first->setPrologueData(C);<br>
>>>>>>> InlineFunction.cpp: Caller->setPersonalityFn(CalledPersonality);<br>
>>>>>>><br>
>>>>>>> Some of these sites could be refactored so that the Functions are<br>
>>>>> created with the prefix/prologue data they need. I don't think<br>
>>>>> that's possible for personality functions (see my third example).<br>
>>>>>>><br>
>>>>>>> Would we inhibit any future patches which add prefix/prologue<br>
>>>>> data to Functions on the fly by taking this approach?<br>
>>>>>><br>
>>>>>> You should always be able to create a new `llvm::Function`<br>
>>>>> instance (and RAUW it in) if you want to add prefix/prologue data to<br>
>>>>> functions after they've been created; just like you have to do today<br>
>>>>> for any other `llvm::User`s that do not have hung off uses.<br>
>>>>><br>
>>>>> It's possible, but a lot more involved with `Function`s. Besides<br>
>>>>> RAUW, you need to transfer over all the basic blocks.<br>
>>>>><br>
>>>>> This seems kind of wrong to me, if we expect it to happen.<br>
>>>>><br>
>>>>>> Which brings me to -- can you use hung off uses for this? These<br>
>>>>> use lists can be resized on the fly, so you should be able to add<br>
>>>>> and remove prologue data on the fly. If you're using hung off uses,<br>
>>>>> you'll probably still need a descriptor to remember whether / which<br>
>>>>> operands are prologue data etc.<br>
>>>>><br>
>>>>> Sure, this is another option. It might be simplest. I'd be<br>
>>>>> tempted to start with a 0/3 choice (if we allocate any hung-off<br>
>>>>> uses, allocate enough for all three operands) to simplify the<br>
>>>>> logic. Although I can't remember right now whether that's<br>
>>>>> legal (having nullptr operands followed by real ones)...<br>
>>>>><br>
>>>>>>>>>>> Personalities are stored as ``optional`` Function operands.<br>
>>>>> We actually always<br>
>>>>>>>>>>> allocate the space for this ``optional`` operand: there's a<br>
>>>>> FIXME in the<br>
>>>>>>>>>>> destructor for Function about this.<br>
>>>>><br>
>>>>> Makes me wonder, why didn't we use hung off uses to begin with?<br>
>>>>> Do functions "usually" have personality functions, for some<br>
>>>>> definition of?<br>
>>>>><br>
>>>>><br>
>>>>> Depends. In C++? It's pretty common to have objects which have<br>
>>>>> non-trivial destructors on the stack which means calling a function will<br>
>>>>> be an invoke which will require the function to have a personality. In<br>
>>>>> C? It's pretty rare. You'd need something like __attribute__((cleanup))<br>
>>>>> to do it, the most common source of this will be something<br>
>>>>> like pthread_cleanup_push. If I recall correctly, Julia sets the<br>
>>>>> personality on functions regardless of whether or not there are any<br>
>>>>> invokes, they need the AsmPrinter to scribble something down. I can't<br>
>>>>> say for other languages (Rust, etc.). From what I understand, Swift<br>
>>>>> doesn't use landingpad for EH so they wouldn't need the personality set.<br>
>>>><br>
>>>> Most functions we emit from our Java frontend have personalities.<br>
>>>><br>
>>>> -- Sanjoy<br>
>>>><br>
>>>>><br>
>>>>> _______________________________________________<br>
>>>>> LLVM Developers mailing list<br>
>>>>> <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a> <mailto:<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>><br>
>>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>>>>><br>
>>>>><br>
>>>> _______________________________________________<br>
>>>> LLVM Developers mailing list<br>
>>>> <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>>><br>
>><br>
><br>
<br>
</div></div></blockquote></div><br></div>