x86emul: simplify SHLD/SHRD handling

Commit Message

Jan Beulich June 20, 2017, 6:20 a.m. UTC

First of all there's no point considering the "shift == width" case,
when immediately before that check we mask "shift" by "width - 1". And
then truncate_word() use can be reduced too: dst.val, as obtained by
generic operand fetching code, is already suitably truncated, and its
use can also be made symmetric in the main conditional expression (on
only left shift results). Finally masking the result of a right shift
is not necessary when the left hand operand doesn't have more than
"width" significant bits.

Signed-off-by: Jan Beulich <[e-mail]>
x86emul: simplify SHLD/SHRD handling

First of all there's no point considering the "shift == width" case,
when immediately before that check we mask "shift" by "width - 1". And
then truncate_word() use can be reduced too: dst.val, as obtained by
generic operand fetching code, is already suitably truncated, and its
use can also be made symmetric in the main conditional expression (on
only left shift results). Finally masking the result of a right shift
is not necessary when the left hand operand doesn't have more than
"width" significant bits.

Signed-off-by: Jan Beulich <[e-mail]>

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -6414,16 +6414,14 @@ x86_emulate(
         }
         if ( (shift &= width - 1) == 0 )
             break;
-        dst.orig_val = truncate_word(dst.val, dst.bytes);
-        dst.val = ((shift == width) ? src.val :
-                   (b & 8) ?
-                   /* shrd */
-                   ((dst.orig_val >> shift) |
-                    truncate_word(src.val << (width - shift), dst.bytes)) :
-                   /* shld */
-                   ((dst.orig_val << shift) |
-                    ((src.val >> (width - shift)) & ((1ull << shift) - 1))));
-        dst.val = truncate_word(dst.val, dst.bytes);
+        dst.orig_val = dst.val;
+        dst.val = (b & 8) ?
+                  /* shrd */
+                  ((dst.orig_val >> shift) |
+                   truncate_word(src.val << (width - shift), dst.bytes)) :
+                  /* shld */
+                  (truncate_word(dst.orig_val << shift, dst.bytes) |
+                   (src.val >> (width - shift)));
         _regs.eflags &= ~(X86_EFLAGS_OF | X86_EFLAGS_SF | X86_EFLAGS_ZF |
                           X86_EFLAGS_PF | X86_EFLAGS_CF);
         if ( (dst.val >> ((b & 8) ? (shift - 1) : (width - shift))) & 1 )

Comments

Andrew Cooper June 23, 2017, 1:19 p.m. UTC | #1

On 20/06/17 07:20, Jan Beulich wrote:
> First of all there's no point considering the "shift == width" case,
> when immediately before that check we mask "shift" by "width - 1". And
> then truncate_word() use can be reduced too: dst.val, as obtained by
> generic operand fetching code, is already suitably truncated, and its
> use can also be made symmetric in the main conditional expression (on
> only left shift results). Finally masking the result of a right shift
> is not necessary when the left hand operand doesn't have more than
> "width" significant bits.
>
> Signed-off-by: Jan Beulich <[e-mail]>

Reviewed-by: Andrew Cooper <[e-mail]>

Patch

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -6414,16 +6414,14 @@  x86_emulate(
         }
         if ( (shift &= width - 1) == 0 )
             break;
-        dst.orig_val = truncate_word(dst.val, dst.bytes);
-        dst.val = ((shift == width) ? src.val :
-                   (b & 8) ?
-                   /* shrd */
-                   ((dst.orig_val >> shift) |
-                    truncate_word(src.val << (width - shift), dst.bytes)) :
-                   /* shld */
-                   ((dst.orig_val << shift) |
-                    ((src.val >> (width - shift)) & ((1ull << shift) - 1))));
-        dst.val = truncate_word(dst.val, dst.bytes);
+        dst.orig_val = dst.val;
+        dst.val = (b & 8) ?
+                  /* shrd */
+                  ((dst.orig_val >> shift) |
+                   truncate_word(src.val << (width - shift), dst.bytes)) :
+                  /* shld */
+                  (truncate_word(dst.orig_val << shift, dst.bytes) |
+                   (src.val >> (width - shift)));
         _regs.eflags &= ~(X86_EFLAGS_OF | X86_EFLAGS_SF | X86_EFLAGS_ZF |
                           X86_EFLAGS_PF | X86_EFLAGS_CF);
         if ( (dst.val >> ((b & 8) ? (shift - 1) : (width - shift))) & 1 )