[all-commits] [llvm/llvm-project] 88b10f: [MC][AArch64] Segregate constant pool caches by si...

[Simon Tatham via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 88b10f3e3aa93232f1f530cf8dfe1227f5f74ae9
      https://github.com/llvm/llvm-project/commit/88b10f3e3aa93232f1f530cf8dfe1227f5f74ae9
  Author: Simon Tatham <simon.tatham at arm.com>
  Date:   2024-03-28 (Thu, 28 Mar 2024)

  Changed paths:
    M llvm/include/llvm/MC/ConstantPools.h
    M llvm/lib/MC/ConstantPools.cpp
    A llvm/test/MC/AArch64/constant-pool-sizes.s

  Log Message:
  -----------
  [MC][AArch64] Segregate constant pool caches by size. (#86832)

If you write a 32- and a 64-bit LDR instruction that both refer to the
same constant or symbol using the = syntax:

```
  ldr w0, =something
  ldr x1, =something
```

then the first call to `ConstantPool::addEntry` will insert the constant
into its cache of existing entries, and the second one will find the
cache entry and reuse it. This results in a 64-bit load from a 32-bit
constant, reading nonsense into the other half of the target register.

In this patch I've done the simplest fix: include the size of the
constant pool entry as part of the key used to index the cache. So now
32- and 64-bit constant loads will never share a constant pool entry.

There's scope for doing this better, in principle: you could imagine
merging the two slots with appropriate overlap, so that the 32-bit load
loads the LSW of the 64-bit value. But that's much more complicated: you
have to take endianness into account, and maybe also adjust the size of
an existing entry. This is the simplest fix that restores correctness.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications