On 3 April 2011 10:41, Blue Swirl <blauwir...@gmail.com> wrote: > On Sat, Apr 2, 2011 at 1:33 AM, Peter Maydell <peter.mayd...@linaro.org> > wrote: >> Nobody seemed to have a coherent rule about when functions >> should be in op_helper.c and use the global 'env' variable >> and when they should be in some other file and have 'env' >> passed as a parameter
> In general, helpers for the translated code belong to op_helper.c. > They can and should access global env directly for speed. If a helper > is used for both translated code and outside of it, a wrapper should > be added to do global env shuffling (or for example a copy without > shuffling added). OK, we can do that, but at the moment "helper function not in op_helper.c" is hugely in the majority so there's a lot of code we'd be moving around: $ grep -c HELPER target-arm/*.c target-arm/helper.c:68 target-arm/iwmmxt_helper.c:59 target-arm/machine.c:0 target-arm/neon_helper.c:103 target-arm/op_helper.c:28 target-arm/translate.c:2 (ie just 10% or so of ARM helper functions are in op_helper.c) ...and this cleanup would basically amount to folding neon_helper.c, iwmmxt_helper.c and bits of helper.c into op_helper.c (and then removing the 'env' parameters, so a big patch to translate.c as well, which I don't suppose anybody maintaining an out-of-tree target-arm patchset will thank us for :-)). But I can submit a patch to do that if it's the right thing. -- PMM