[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 3/4] x86/hvm: Rework hpet_write() for improved code generation



In the HPET_STATUS handling, the use of __clear_bit(i, &new_val) is the only
thing causing it to be spilled to the stack.  Furthemore we only care about
the bottom 3 bits, so rewrite it to be a plain for loop.

For the {start,stop}_timer variables, these are spilled to the stack despite
the __{set,clear}_bit() calls.  Again we only care about the bottom 3 bits, so
shrink the variables from long to int.  Use for_each_set_bit() rather than
opencoding it at the end which amongst other things means the loop predicate
is no longer forced to the stack by the loop body.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

All in all, it's modest according to bloat-o-meter:

  add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-29 (-29)
  Function                                     old     new   delta
  hpet_write                                  2225    2196     -29

but we have shrunk the stack frame by 8 bytes; 0x28 as opposed to 0x30 before.
---
 xen/arch/x86/hvm/hpet.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 87642575f9cd..e3981d5e467c 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -349,8 +349,7 @@ static int cf_check hpet_write(
     unsigned int tn, i;
 
     /* Acculumate a bit mask of timers whos state is changed by this write. */
-    unsigned long start_timers = 0;
-    unsigned long stop_timers  = 0;
+    unsigned int start_timers = 0, stop_timers = 0;
 #define set_stop_timer(n)    (__set_bit((n), &stop_timers))
 #define set_start_timer(n)   (__set_bit((n), &start_timers))
 #define set_restart_timer(n) (set_stop_timer(n),set_start_timer(n))
@@ -405,16 +404,12 @@ static int cf_check hpet_write(
 
     case HPET_STATUS:
         /* write 1 to clear. */
-        while ( new_val )
+        for ( i = 0; i < HPET_TIMER_NUM; i++ )
         {
-            bool active;
-
-            i = ffsl(new_val) - 1;
-            if ( i >= HPET_TIMER_NUM )
-                break;
-            __clear_bit(i, &new_val);
-            active = __test_and_clear_bit(i, &h->hpet.isr);
-            if ( active )
+            if ( !(new_val & (1U << i)) )
+                continue;
+
+            if ( __test_and_clear_bit(i, &h->hpet.isr) )
             {
                 hvm_ioapic_deassert(v->domain, timer_int_route(h, i));
                 if ( hpet_enabled(h) && timer_enabled(h, i) &&
@@ -533,19 +528,11 @@ static int cf_check hpet_write(
     }
 
     /* stop/start timers whos state was changed by this write. */
-    while (stop_timers)
-    {
-        i = ffsl(stop_timers) - 1;
-        __clear_bit(i, &stop_timers);
+    for_each_set_bit ( i, stop_timers )
         hpet_stop_timer(h, i, guest_time);
-    }
 
-    while (start_timers)
-    {
-        i = ffsl(start_timers) - 1;
-        __clear_bit(i, &start_timers);
+    for_each_set_bit ( i, start_timers )
         hpet_set_timer(h, i, guest_time);
-    }
 
 #undef set_stop_timer
 #undef set_start_timer
-- 
2.39.2




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.