On 29/03/16 16:26, Paolo Bonzini wrote:
>
> On 29/03/2016 15:19, Sergey Fedorov wrote:
>> On 22/03/16 17:59, Alex Bennée wrote:
>>> sergey.fedo...@linaro.org writes:
>>>
>>>> From: Paolo Bonzini <pbonz...@redhat.com>
>>>>
>>>> Use a continue statement.
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>>>> [Sergey Fedorov: Fix moving to list head in case of no TB]
>>>> Signed-off-by: Sergey Fedorov <sergey.fedo...@linaro.org>
>>>> ---
>>>> cpu-exec.c | 50 +++++++++++++++++++++++++-------------------------
>>>> 1 file changed, 25 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/cpu-exec.c b/cpu-exec.c
>>>> index fd92452f16f6..f90482eff778 100644
>>>> --- a/cpu-exec.c
>>>> +++ b/cpu-exec.c
>>>> @@ -225,37 +225,37 @@ static TranslationBlock *tb_find_physical(CPUState
>>>> *cpu,
>>>> phys_pc = get_page_addr_code(env, pc);
>>>> phys_page1 = phys_pc & TARGET_PAGE_MASK;
>>>> h = tb_phys_hash_func(phys_pc);
>>>> - ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h];
>>>> - for(;;) {
>>>> - tb = *ptb1;
>>>> - if (!tb) {
>>>> - return NULL;
>>>> + for (ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h];
>>>> + (tb = *ptb1) != NULL;
>>>> + ptb1 = &tb->phys_hash_next) {
>>> I'm not sure I'm keen on the assignment in the for condition clause. I
>>> appreciate the cleansing of the if !tb return exit though. Could we be
>>> cleaner maybe? Here is my attempt:
>>>
>>> static TranslationBlock *tb_find_physical(CPUState *cpu,
>>> target_ulong pc,
>>> target_ulong cs_base,
>>> uint64_t flags)
>>> {
>>> CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>>> TranslationBlock *tb, **tb_hash_head, **ptb1;
>>> unsigned int h;
>>> tb_page_addr_t phys_pc, phys_page1;
>>>
>>> /* find translated block using physical mappings */
>>> phys_pc = get_page_addr_code(env, pc);
>>> phys_page1 = phys_pc & TARGET_PAGE_MASK;
>>> h = tb_phys_hash_func(phys_pc);
>>>
>>> /* Start at head of the hash entry */
>>> ptb1 = tb_hash_head = &tcg_ctx.tb_ctx.tb_phys_hash[h];
>>> tb = *ptb1;
>>>
>>> while (tb) {
>>>
>>> if (tb->pc == pc &&
>>> tb->page_addr[0] == phys_page1 &&
>>> tb->cs_base == cs_base &&
>>> tb->flags == flags) {
>>>
>>> if (tb->page_addr[1] == -1) {
>>> /* done, we have a match */
>>> break;
>>> } else {
>>> /* check next page if needed */
>>> target_ulong virt_page2 = (pc & TARGET_PAGE_MASK)
>>> + TARGET_PAGE_SIZE;
>>> tb_page_addr_t phys_page2 = get_page_addr_code(env,
>>> virt_page2);
>>>
>>> if (tb->page_addr[1] == phys_page2) {
>>> break;
>>> }
>>> }
>>> }
>>>
>>> ptb1 = &tb->phys_hash_next;
>>> tb = *ptb1;
>>> }
>>>
>>> if (tb) {
>>> /* Move the TB to the head of the list */
>>> *ptb1 = tb->phys_hash_next;
>>> tb->phys_hash_next = *tb_hash_head;
>>> *tb_hash_head = tb;
>>> }
>>> return tb;
>>> }
>>>
>>> FWIW the compiled code is 9 bytes shorter on my machine.
>> Looks like another attempt to rewrite it. I am wondering whom to
>> attribute as an author, then? :)
> I don't really care. :)
Alex, could you give your s-o-b for your variant of code? Or would you
like to make a patch by yourself?
Kind regards,
Sergey
