On 18/03/2019 14:58, Jan Beulich wrote: >>>> On 18.03.19 at 15:11, <andrew.coop...@citrix.com> wrote: >> On 18/03/2019 13:20, Jan Beulich wrote: >>> >>> On 18.03.19 at 12:27, <andrew.coop...@citrix.com> wrote: >>>> * Some of the single-byte versions specify "=q" as the output. This is a >>>> remnent of the 32bit build and can be relaxed to "=r" in 64bit builds. >>> I have to admit that I don't understand the "relaxed" part of this: >>> "q" and "r" represent the exact same set of registers on 64-bit. >>> Unless the conversion allows further code folding, I think it wouldn't >>> be a bad idea to retain the distinction, just for cases like code >>> eventually getting shared via something like lib/x86/. >> The change from =q to =r is specifically to allow the folding to +r > Hmm, I still don't understand. "+q" is as valid as "+r". And "q" does > not make any size implications, i.e. registers in that group aren't > implicitly byte ones (albeit this aspect doesn't even matter here).
I'm at a complete loss to understand what you are objecting to here. "q" and "r" mean different things on 32bit, and "+q" would be wrong to use there. "q" and "r" are synonymous on 64bit, but "r" is still the one to use because that is the more common constraint, and therefore the more familiar for people reading the code. > >>>> @@ -40,28 +37,24 @@ static always_inline unsigned long __xchg( >>>> switch ( size ) >>>> { >>>> case 1: >>>> - asm volatile ( "xchgb %b0,%1" >>>> - : "=q" (x) >>>> - : "m" (*__xg(ptr)), "0" (x) >>>> - : "memory" ); >>>> + asm volatile ( "xchg %b[x], %[ptr]" >>>> + : [x] "+r" (x), [ptr] "+m" (*(uint8_t *)ptr) >>>> + :: "memory" ); >>>> break; >>>> case 2: >>>> - asm volatile ( "xchgw %w0,%1" >>>> - : "=r" (x) >>>> - : "m" (*__xg(ptr)), "0" (x) >>>> - : "memory" ); >>>> + asm volatile ( "xchg %w[x], %[ptr]" >>>> + : [x] "+r" (x), [ptr] "+m" (*(uint16_t *)ptr) >>>> + :: "memory" ); >>>> break; >>>> case 4: >>>> - asm volatile ( "xchgl %k0,%1" >>>> - : "=r" (x) >>>> - : "m" (*__xg(ptr)), "0" (x) >>>> - : "memory" ); >>>> + asm volatile ( "xchg %k[x], %[ptr]" >>>> + : [x] "+r" (x), [ptr] "+m" (*(uint32_t *)ptr) >>>> + :: "memory" ); >>>> break; >>>> case 8: >>>> - asm volatile ( "xchgq %0,%1" >>>> - : "=r" (x) >>>> - : "m" (*__xg(ptr)), "0" (x) >>>> - : "memory" ); >>>> + asm volatile ( "xchg %q[x], %[ptr]" >>>> + : [x] "+r" (x), [ptr] "+m" (*(uint64_t *)ptr) >>>> + :: "memory" ); >>>> break; >>> Is the q modifier really useful to have here (and elsewhere below)? >> Yes - it is strictly necessary, because otherwise it gets derived from >> the type of (x) which is unsigned long even in the smaller size constructs. > What you say explains why you need the b, w, and k modifiers. It > doesn't explain the q one, since sizeof(unsigned long) is 8 and > hence exactly what would result without the modifier. Oh, in which case, "for consistency". ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel