In a number of cases we use ALTERNATIVE_2 with both replacement insns /
insn sequences being identical. Avoid emitting the same code twice, and
instead alias the necessary helper labels to the existing ones.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/include/asm/alternative-asm.h
+++ b/xen/arch/x86/include/asm/alternative-asm.h
@@ -51,6 +51,8 @@
 
 #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 clone_repl(new, old)    .equiv .L\@_repl_s\()new, .L\@_repl_s\()old; \
+                                .equiv .L\@_repl_e\()new, .L\@_repl_e\()old
 
 #define as_max(a, b)           ((a) ^ (((a) ^ (b)) & -as_true((a) < (b))))
 
@@ -100,7 +102,11 @@
     .section .altinstr_replacement, "ax", @progbits
 
     decl_repl(\newinstr1, 1)
+    .ifnes "\newinstr2", "\newinstr1"
     decl_repl(\newinstr2, 2)
+    .else
+    clone_repl(2, 1)
+    .endif
 
     .popsection
 .endm

On 30/04/2025 2:13 pm, Jan Beulich wrote:
> In a number of cases we use ALTERNATIVE_2 with both replacement insns /
> insn sequences being identical. Avoid emitting the same code twice, and
> instead alias the necessary helper labels to the existing ones.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

On a random build, the size of .altinstr_replacement drops from 0xe47 to
0xdf8, so not too bad.

While the patch is fine, if we're adjusting the assembly ALTERNATIVE_2,
we should make the same adjustment to the C version, even if there's
nothing to benefit from it immediately.

~Andrew

P.S. it would be even nicer if we would put these in mergeable sections,
but I haven't figured out way to set the mergable unit size, which needs
to be an absolute expression.

On 30.04.2025 18:00, Andrew Cooper wrote:
> On 30/04/2025 2:13 pm, Jan Beulich wrote:
>> In a number of cases we use ALTERNATIVE_2 with both replacement insns /
>> insn sequences being identical. Avoid emitting the same code twice, and
>> instead alias the necessary helper labels to the existing ones.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> On a random build, the size of .altinstr_replacement drops from 0xe47 to
> 0xdf8, so not too bad.
> 
> While the patch is fine, if we're adjusting the assembly ALTERNATIVE_2,
> we should make the same adjustment to the C version, even if there's
> nothing to benefit from it immediately.

Can do, but I expect that to end up more clumsy for, as you say, no real
gain.

> P.S. it would be even nicer if we would put these in mergeable sections,
> but I haven't figured out way to set the mergable unit size, which needs
> to be an absolute expression.

Well, have you missed
https://lists.xen.org/archives/html/xen-devel/2021-11/msg01009.html then?
Yet it might still make sense to use the approach here as well, as there
are limitations to what can really be merged (by the toolchain).

Jan

On 30/04/2025 5:05 pm, Jan Beulich wrote:
> On 30.04.2025 18:00, Andrew Cooper wrote:
>> On 30/04/2025 2:13 pm, Jan Beulich wrote:
>>> In a number of cases we use ALTERNATIVE_2 with both replacement insns /
>>> insn sequences being identical. Avoid emitting the same code twice, and
>>> instead alias the necessary helper labels to the existing ones.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> On a random build, the size of .altinstr_replacement drops from 0xe47 to
>> 0xdf8, so not too bad.
>>
>> While the patch is fine, if we're adjusting the assembly ALTERNATIVE_2,
>> we should make the same adjustment to the C version, even if there's
>> nothing to benefit from it immediately.
> Can do, but I expect that to end up more clumsy for, as you say, no real
> gain.

Fine.  Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
>> P.S. it would be even nicer if we would put these in mergeable sections,
>> but I haven't figured out way to set the mergable unit size, which needs
>> to be an absolute expression.
> Well, have you missed
> https://lists.xen.org/archives/html/xen-devel/2021-11/msg01009.html then?

Apparently so, yes.

~Andrew

> Yet it might still make sense to use the approach here as well, as there
> are limitations to what can really be merged (by the toolchain).
>
> Jan