aarch64_multibinary.h: Fix -Wasm-operand-widths

[bsbernd]

Compilation with clang gave warnings as per below. Arm64 is has a width of 64 bit and these warnings came up.

In file included from igzip/aarch64/igzip_multibinary_aarch64_dispatcher.c:29: ./include/aarch64_multibinary.h:338:35: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths] asm("mrs %0, MIDR_EL1 " : "=r" (id)); ^ ./include/aarch64_multibinary.h:338:12: note: use constraint modifier "w" asm("mrs %0, MIDR_EL1 " : "=r" (id)); ^~ %w0 1 warning generated. In file included from mem/aarch64/mem_aarch64_dispatcher.c:29: ./include/aarch64_multibinary.h:338:35: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths] asm("mrs %0, MIDR_EL1 " : "=r" (id)); ^ ./include/aarch64_multibinary.h:338:12: note: use constraint modifier "w" asm("mrs %0, MIDR_EL1 " : "=r" (id)); ^~ %w0 1 warning generated.

[pablodelara]

Hi @bsbernd. Why compiler are you using? And could you sign off the commit? Thanks!

[bsbernd]

@pablodelara just signed it. And yeah, I had forgotten to write that it happens with clang - also updated the commit message.

[bsbernd]

Hmm, interesting that it is a actually a 32bit register https://developer.arm.com/documentation/ddi0601/2024-03/External-Registers/MIDR-EL1--Main-ID-Register

[bsbernd]

I tried to grep through linux kernel code a bit, I think it resolves to

/*
 * For registers without architectural names, or simply unsupported by
 * GAS.
 *
 * __check_r forces warnings to be generated by the compiler when
 * evaluating r which wouldn't normally happen due to being passed to
 * the assembler via __stringify(r). 
 */ 
#define read_sysreg_s(r) ({                     \
    u64 __val;                          \
    u32 __maybe_unused __check_r = (u32)(r);            \
    asm volatile(__mrs_s("%0", r) : "=r" (__val));          \
    __val;                              \
})

So also 64bit

[bsbernd]

In glibc

static inline void
init_cpu_features (struct cpu_features *cpu_features)
{
  register uint64_t midr = UINT64_MAX;

  /* Get the tunable override.  */
  const char *mcpu = TUNABLE_GET (glibc, cpu, name, const char *, NULL);
  if (mcpu != NULL)
    midr = get_midr_from_mcpu (mcpu);

  /* If there was no useful tunable override, query the MIDR if the kernel
     allows it.  */
  if (midr == UINT64_MAX)
    {
      if (GLRO (dl_hwcap) & HWCAP_CPUID)
    asm volatile ("mrs %0, midr_el1" : "=r"(midr));
      else
    midr = 0;
    }

  cpu_features->midr_el1 = midr;

So I think my patch is correct (I'm typically not working on assembler level, though).

[pablodelara]

@liuqinfei could you review this PR? Thanks!

[liuqinfei]

Hmm, interesting that it is a actually a 32bit register https://developer.arm.com/documentation/ddi0601/2024-03/External-Registers/MIDR-EL1--Main-ID-Register

A different version of the register description was found. And only the lower 32 bits of the register are valid. SO, for some platform the register is thought to be 64 bit. So, this may be the reason what the warning comes from. https://developer.arm.com/documentation/ddi0601/2020-12/AArch64-Registers/MIDR-EL1--Main-ID-Register

And according to the latest version, as you point out from the link, the register has been changed to a 32-bit version. Therefore, there is no truncation risk. So, @bsbernd what is your platform?

[bsbernd]

bschubert@bschubert-bld-arm-2 ~>lscpu 
Architecture:           aarch64
  CPU op-mode(s):       32-bit, 64-bit
  Byte Order:           Little Endian
  CPU(s):                 40
  On-line CPU(s) list:  0-39
Vendor ID:              ARM
  Model name:           Neoverse-N1
    Model:              1
    Thread(s) per core: 1
    Core(s) per socket: 1
    Socket(s):          40
    Stepping:           r3p1
    BogoMIPS:           50.00
    Flags:              fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm lrcpc dcpop asimddp ssbs

@liuqinfei Yeah I think there is no risk with a 32 bit variable, but as glibc and linux kernel use a 64 bit variable there also should be no harm.

[liuqinfei]

bschubert@bschubert-bld-arm-2 ~>lscpu 
Architecture:           aarch64
  CPU op-mode(s):       32-bit, 64-bit
  Byte Order:           Little Endian
  CPU(s):                 40
  On-line CPU(s) list:  0-39
Vendor ID:              ARM
  Model name:           Neoverse-N1
    Model:              1
    Thread(s) per core: 1
    Core(s) per socket: 1
    Socket(s):          40
    Stepping:           r3p1
    BogoMIPS:           50.00
    Flags:              fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm lrcpc dcpop asimddp ssbs

@liuqinfei Yeah I think there is no risk with a 32 bit variable, but as glibc and linux kernel use a 64 bit variable there also should be no harm.

Then the PR is fine. LGTM

[pablodelara]

Apologies for the delay. This is merged now.

[bsbernd]

No issue at all. Thank you @pablodelara.