| Message ID | f994f67d-e0de-ad28-d418-1eb5a70bc1b8@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: do away with HAVE_AS_NEGATIVE_TRUE | expand |
On 17/05/2023 3:22 pm, Jan Beulich wrote: > There's no real need for the associated probing - we can easily convert > to a uniform value without knowing the specific behavior (note also that > the respective comments weren't fully correct and have gone stale). All > we (need to) depend upon is unary ! producing 0 or 1 (and never -1). > > For all present purposes yielding a value with all bits set is more > useful. > > No difference in generated code. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > Unlike in C, there's also binary ! in assembly expressions, and even > binary !!. But those don't get in the way here. I had been wanting to do this for a while, but IMO a clearer expression is to take ((x) & 1) to discard the sign. It doesn't change any of the logic to use +1 (I don't think), and it's definitely the more common way for the programmer to think. ~Andrew
On 17.05.2023 19:05, Andrew Cooper wrote: > On 17/05/2023 3:22 pm, Jan Beulich wrote: >> There's no real need for the associated probing - we can easily convert >> to a uniform value without knowing the specific behavior (note also that >> the respective comments weren't fully correct and have gone stale). All >> we (need to) depend upon is unary ! producing 0 or 1 (and never -1). >> >> For all present purposes yielding a value with all bits set is more >> useful. >> >> No difference in generated code. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> Unlike in C, there's also binary ! in assembly expressions, and even >> binary !!. But those don't get in the way here. > > I had been wanting to do this for a while, but IMO a clearer expression > is to take ((x) & 1) to discard the sign. > > It doesn't change any of the logic to use +1 (I don't think), and it's > definitely the more common way for the programmer to think. Well, I can certainly switch. It simply seemed to me that with our many uses of !! elsewhere, using this here as well would only be consistent. (I did in fact consider the ... & 1 alternative.) Jan
On 19.05.2023 08:15, Jan Beulich wrote: > On 17.05.2023 19:05, Andrew Cooper wrote: >> On 17/05/2023 3:22 pm, Jan Beulich wrote: >>> There's no real need for the associated probing - we can easily convert >>> to a uniform value without knowing the specific behavior (note also that >>> the respective comments weren't fully correct and have gone stale). All >>> we (need to) depend upon is unary ! producing 0 or 1 (and never -1). >>> >>> For all present purposes yielding a value with all bits set is more >>> useful. >>> >>> No difference in generated code. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> --- >>> Unlike in C, there's also binary ! in assembly expressions, and even >>> binary !!. But those don't get in the way here. >> >> I had been wanting to do this for a while, but IMO a clearer expression >> is to take ((x) & 1) to discard the sign. >> >> It doesn't change any of the logic to use +1 (I don't think), and it's >> definitely the more common way for the programmer to think. > > Well, I can certainly switch. It simply seemed to me that with our many > uses of !! elsewhere, using this here as well would only be consistent. > (I did in fact consider the ... & 1 alternative.) Before even starting with this - you do realize that the C macro (AS_TRUE) expands to just a prefix for the expression to be dealt with? That would then become "1 & ", which I have to admit I find a little odd. The alternative of making this a more ordinary macro (with a parameter) would likely be more intrusive. Plus I assume you had a reason to do it the way it is right now (and I might end up figuring that reason the hard way when trying to change things). Jan
--- a/xen/arch/x86/arch.mk +++ b/xen/arch/x86/arch.mk @@ -26,10 +26,6 @@ $(call as-option-add,CFLAGS,CC,"invpcid $(call as-option-add,CFLAGS,CC,"movdiri %rax$(comma)(%rax)",-DHAVE_AS_MOVDIR) $(call as-option-add,CFLAGS,CC,"enqcmd (%rax)$(comma)%rax",-DHAVE_AS_ENQCMD) -# GAS's idea of true is -1. Clang's idea is 1 -$(call as-option-add,CFLAGS,CC,\ - ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) - # Check to see whether the assmbler supports the .nop directive. $(call as-option-add,CFLAGS,CC,\ ".L1: .L2: .nops (.L2 - .L1)$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE) --- a/xen/arch/x86/include/asm/alternative.h +++ b/xen/arch/x86/include/asm/alternative.h @@ -35,19 +35,19 @@ extern void alternative_branches(void); #define alt_repl_e(num) ".LXEN%=_repl_e"#num #define alt_repl_len(num) "(" alt_repl_e(num) " - " alt_repl_s(num) ")" -/* GAS's idea of true is -1, while Clang's idea is 1. */ -#ifdef HAVE_AS_NEGATIVE_TRUE -# define AS_TRUE "-" -#else -# define AS_TRUE "" -#endif +/* + * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea was + * consistently 1 up to 6.x (it matches GAS's now). Transform it to uniformly + * -1 (aka ~0). + */ +#define AS_TRUE "-!!" -#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE"(("a") < ("b")))))" +#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & "AS_TRUE"(("a") < ("b"))))" #define OLDINSTR(oldinstr, padding) \ ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t" \ ".LXEN%=_diff = " padding "\n\t" \ - "mknops ("AS_TRUE"(.LXEN%=_diff > 0) * .LXEN%=_diff)\n\t" \ + "mknops ("AS_TRUE"(.LXEN%=_diff > 0) & .LXEN%=_diff)\n\t" \ ".LXEN%=_orig_p:\n\t" #define OLDINSTR_1(oldinstr, n1) \ --- a/xen/arch/x86/include/asm/alternative-asm.h +++ b/xen/arch/x86/include/asm/alternative-asm.h @@ -29,17 +29,17 @@ #endif .endm -/* GAS's idea of true is -1, while Clang's idea is 1. */ -#ifdef HAVE_AS_NEGATIVE_TRUE -# define as_true(x) (-(x)) -#else -# define as_true(x) (x) -#endif +/* + * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea was + * consistently 1 up to 6.x (it matches GAS's now). Transform it to uniformly + * -1 (aka ~0). + */ +#define as_true(x) (-!!(x)) #define decl_orig(insn, padding) \ .L\@_orig_s: insn; .L\@_orig_e: \ .L\@_diff = padding; \ - mknops (as_true(.L\@_diff > 0) * .L\@_diff); \ + mknops (as_true(.L\@_diff > 0) & .L\@_diff); \ .L\@_orig_p: #define orig_len (.L\@_orig_e - .L\@_orig_s) @@ -49,7 +49,7 @@ #define decl_repl(insn, nr) .L\@_repl_s\()nr: insn; .L\@_repl_e\()nr: #define repl_len(nr) (.L\@_repl_e\()nr - .L\@_repl_s\()nr) -#define as_max(a, b) ((a) ^ (((a) ^ (b)) & -as_true((a) < (b)))) +#define as_max(a, b) ((a) ^ (((a) ^ (b)) & as_true((a) < (b)))) .macro ALTERNATIVE oldinstr, newinstr, feature decl_orig(\oldinstr, repl_len(1) - orig_len)
There's no real need for the associated probing - we can easily convert to a uniform value without knowing the specific behavior (note also that the respective comments weren't fully correct and have gone stale). All we (need to) depend upon is unary ! producing 0 or 1 (and never -1). For all present purposes yielding a value with all bits set is more useful. No difference in generated code. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Unlike in C, there's also binary ! in assembly expressions, and even binary !!. But those don't get in the way here.