[PATCH 1/2] x86: refine special casing of insns with MSR as immediate

Jan Beulich jbeulich@suse.com
Thu Apr 9 07:19:57 GMT 2026


On 09.04.2026 09:13, H.J. Lu wrote:
> On Thu, Apr 9, 2026 at 2:47 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> PR gas/34028
>>
>> i.op[].imms is legitimate to de-reference only when the operand actually
>> is an immediate. The pointer happens to be non-NULL for most other operand
>> kinds (i.e. a deref is UB, but would likely not fault), just not for
>> memory operands without displacement.
>>
>> Leverage that the to-be-special-cased insns all only have just a single
>> immediate, and leverage further that after the immediately preceding
>> swapping of operands valid immediates will come first. Hence the
>> conditional can be adjusted to satisfy the criteria above, and no loop is
>> needed at all.
>>
>> With the conditional changed, leverage the property also in optimize_imm()
>> itself, reducing the number of loop iterations.
>>
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -7303,7 +7303,9 @@ i386_assemble (char *line)
>>           && i.imm_operands && i.operands > i.imm_operands))
>>        swap_2_operands (0, 1);
>>
>> -  if (i.imm_operands)
>> +  /* All legitimate immediates are placed first now.  Others, if any, will be
>> +     rejected by match_template() anyway.  */
>> +  if (operand_type_check (i.types[0], imm))
>>      {
>>        /* For USER_MSR and MSR_IMM instructions, imm32 stands for the name of a
>>          model specific register (MSR). That's an unsigned quantity, whereas all
>> @@ -7313,10 +7315,7 @@ i386_assemble (char *line)
>>        if (is_cpu(current_templates.start, CpuUSER_MSR)
>>           || t->mnem_off == MN_rdmsr
>>           || t->mnem_off == MN_wrmsrns)
>> -       {
>> -         for (j = 0; j < i.imm_operands; j++)
>> -           i.types[j] = smallest_imm_type (i.op[j].imms->X_add_number);
>> -       }
>> +       i.types[0] = smallest_imm_type (i.op[0].imms->X_add_number);
>>        else
>>         optimize_imm ();
>>      }
>> @@ -8520,7 +8519,7 @@ optimize_imm (void)
>>                && current_templates.start->mnem_off != MN_jmpabs))
>>      guess_suffix = LONG_MNEM_SUFFIX;
>>
>> -  for (op = i.operands; --op >= 0;)
>> +  for (op = i.imm_operands; --op >= 0;)
>>      if (operand_type_check (i.types[op], imm))
>>        {
>>         switch (i.op[op].imms->X_op)
>> --- a/gas/testsuite/gas/i386/x86-64-msr_imm-inval.l
>> +++ b/gas/testsuite/gas/i386/x86-64-msr_imm-inval.l
>> @@ -1,5 +1,7 @@
>>  .* Assembler messages:
>>  .*:5: Error: operand type mismatch for `rdmsr'
>>  .*:6: Error: operand type mismatch for `rdmsr'
>> -.*:7: Error: operand type mismatch for `wrmsrns'
>> +.*:7: Error: operand type mismatch for `rdmsr'
>>  .*:8: Error: operand type mismatch for `wrmsrns'
>> +.*:9: Error: operand type mismatch for `wrmsrns'
>> +.*:10: Error: operand type mismatch for `wrmsrns'
>> --- a/gas/testsuite/gas/i386/x86-64-msr_imm-inval.s
>> +++ b/gas/testsuite/gas/i386/x86-64-msr_imm-inval.s
>> @@ -4,5 +4,7 @@
>>  _start:
>>         rdmsr  $5151515151515151, %r12
>>         rdmsr  $-515151, %r12
>> +       rdmsr  (%rax), $0
>>         wrmsrns  %r12, $5151515151515151
>>         wrmsrns  %r12, $-515151
>> +       wrmsrns  $0, (%rax)
> 
> PR gas/34028 test crashed in 32-bit mode.   Please add a 32-bit test
> to verify that it no longer happens.

As indicated before, imo that's excessive. The code path is the same, so
testing where the insn forms can sensibly be used is pretty much sufficient.

Jan


More information about the Binutils mailing list