On 09.10.2025 09:45, Jan Beulich wrote:
> On 08.10.2025 19:19, Andrew Cooper wrote:
>> On 25/09/2025 11:48 am, Jan Beulich wrote:
>>> --- a/xen/arch/x86/include/asm/pdx.h
>>> +++ b/xen/arch/x86/include/asm/pdx.h
>>> @@ -13,9 +13,9 @@
>>>      asm_inline goto (                               \
>>>          ALTERNATIVE(                                \
>>>              "",                                     \
>>> -            "jmp %l0",                              \
>>> -            ALT_NOT(X86_FEATURE_PDX_COMPRESSION))   \
>>> -        : : : : label )
>>> +            "jmp %l1",                              \
>>> +            [feat])                                 \
>>> +        : : [feat] "i" (ALT_NOT(X86_FEATURE_PDX_COMPRESSION)) : : label )
>>
>> Not a bug in this change, but the pre-existing use of positional labels
>> is something I was expecting not to introduce at all seeing as we
>> started cleanly with named labels.
>>
>> The jmp wants to be:
>>
>>   "jmp %l" #label
>>
>> to cope with the fact it's a macro parameter too.
> 
> Unrelated change? I can of course do the adjustment in a separate prereq
> patch, but then it would have been nice if you had commented along these
> lines before that code actually had gone in.
> 
> That said, isn't it at least bad practice to not expose the label use to
> the compiler? To avoid using positional operands, shouldn't we rather
> name the operand, and then use "jmp %l[whatever_the_name]"? That's a
> change I could see as being justified to do right here, rather than in a
> separate patch.

Hmm, no, labels can't be named; they are their own names. I.e. what I think
we want here is "jmp %l[" #label "]".

Jan

Reply via email to