Re: [fpc-devel] Streamlining TVMTBuilder.generate_vmt after r41716 & r41884
Am 02.08.2019 um 21:27 schrieb bla...@blaise.ru: On 02.08.2019 21:36, bla...@blaise.ru wrote: embed a copy of the body of insert_struct_hidden_paras into TVMTBuilder.generate_vmt, then merge those two procdef-member traversals into one (hey, performance!) Would you guys oppose such a change? Then we could rename insert_struct_hidden_paras back to insert_record_hidden_paras :) Aside from performance, I would like it for closures (for their nameless methods, the insertion of hidden parameters cannot be deferred until the VMT generation). I raised this on core and it was rejected. Maintainability is more important than performance as it could easily be that something else is added to insert_struct_hidden_paras and then that is forgotten to be added in TVMTBuilder.generate_vmt. Also, handle_calling_convention would need to be changed not to indirectly rely on current_filepos, but I see that as a bonus: the trick of swapping current_filepos could be removed from its callers (namely, insert_record_hidden_paras). That proposal is accepted. However without an overload as I suggested, instead all callsites of handle_calling_convention will need to be adjusted. Feel free to draw up a patch for this. Regards, Sven ___ fpc-devel maillist - fpc-devel@lists.freepascal.org https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Re: [fpc-devel] Streamlining TVMTBuilder.generate_vmt after r41716 & r41884
On 03.08.2019 15:01, Sven Barth via fpc-devel wrote:
In principle one could do that, though more often than not inside the compiler
maintainability beats performance. I'd prefer an opinion of Florian and/or
Jonas on this though...
Leaving the issue of current_filepos for a moment, the change would be this:
---8<---
{ VMT entry }
if is_new_vmt_entry(tprocdef(def),overridesclasshelper) then
add_new_vmt_entry(tprocdef(def),overridesclasshelper);
+{ hidden params }
+
handle_calling_convention(tprocdef(def),[hcc_insert_hidden_paras]);
end;
end;
-insert_struct_hidden_paras(_class);
build_interface_mappings;
---8<---
I would say, this is quite maintainable: replacing one call for another.
After r41884, the insertion of hidden parameters is already tightly coupled
with VMT generation. In fact, it is no longer VMTBuilder, it is now
ObjectDefPostprocessor :)
What difference would it make for closures? In the end you'd still need to
ensure that handle_calling_convention isn't called twice.
1) I would rather ensure it on a per-method basis;
2) I would rather add a check inside the combined loop above, instead of
modifying insert_struct_hidden_paras and needlessly affecting RECORDs.
--
βþ
___
fpc-devel maillist - fpc-devel@lists.freepascal.org
https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Re: [fpc-devel] Streamlining TVMTBuilder.generate_vmt after r41716 & r41884
Am 02.08.2019 um 21:27 schrieb bla...@blaise.ru: On 02.08.2019 21:36, bla...@blaise.ru wrote: embed a copy of the body of insert_struct_hidden_paras into TVMTBuilder.generate_vmt, then merge those two procdef-member traversals into one (hey, performance!) Would you guys oppose such a change? Then we could rename insert_struct_hidden_paras back to insert_record_hidden_paras :) In principle one could do that, though more often than not inside the compiler maintainability beats performance. I'd prefer an opinion of Florian and/or Jonas on this though... Aside from performance, I would like it for closures (for their nameless methods, the insertion of hidden parameters cannot be deferred until the VMT generation). What difference would it make for closures? In the end you'd still need to ensure that handle_calling_convention isn't called twice. Also, handle_calling_convention would need to be changed not to indirectly rely on current_filepos, but I see that as a bonus: the trick of swapping current_filepos could be removed from its callers (namely, insert_record_hidden_paras). That would mean that the functions called inside handle_calling_convention (mainly those inside the if-clause for hcc_insert_hidden_paras) would have to be adjusted to handle that as well (especially lovely for those invoked by ForEachCall). If this is done there should also be an overload of handle_calling_convention that does not take a tfileposinfo argument and instead passes on current_filepos so that those code parts that just want to use the current position don't need to be changed. Regards, Sven ___ fpc-devel maillist - fpc-devel@lists.freepascal.org https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
