[all-commits] [llvm/llvm-project] 896749: [amdgpu][openmp] Avoiding writing to packet header...

[Jon Chesterfield via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 896749aa0d420ae573255a64a349bc2a76cfed37
      https://github.com/llvm/llvm-project/commit/896749aa0d420ae573255a64a349bc2a76cfed37
  Author: Jon Chesterfield <JonChesterfield at users.noreply.github.com>
  Date:   2023-10-30 (Mon, 30 Oct 2023)

  Changed paths:
    M libc/utils/gpu/loader/amdgpu/Loader.cpp
    M openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp

  Log Message:
  -----------
  [amdgpu][openmp] Avoiding writing to packet header twice (#70695)

I think it follows from the HSA spec that a write to the first byte is
deemed significant to the GPU in which case writing to the second short
and reading back from it later would be safe. However, the examples for
this all involve an atomic write to the first 32 bits and it seems a
credible risk that the occasional CI errors abound invalid packets have
as their root cause that the firmware notices the early write to
packet->setup and treats that as a sign that the packet is ready to go.

That was overly-paranoid, however in passing noticed the code in libc is
genuinely invalid. The memset writes a zero to the header byte, changing
it from type_invalid (1) to type_vendor (0), at which point the GPU is
free to read the 64 byte packet and interpret it as a vendor packet,
which is probably why libc CI periodically errors about invalid packets.

Also a drive by change to do the atomic store on a uint32_t
consistently. I'm not sure offhand what __atomic_store_n on a uint16_t*
and an int resolves to, seems better to be unambiguous there.