[PATCH] D147547: [AMDGPU] Add buffer intrinsics that take resources as pointers

[Piotr Sobczak via Phabricator via llvm-commits]

piotr added a comment.

Thanks for working on this. Just added a couple of nits.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:462
+/// Mutates IR (typicaly a load instruction) to use a <4 x s32> as the initial
+/// type of the operand `idx` and then to trunsform it to a `p8` via bitcasts
+/// and inttoptr. In addition, handle vectors of p8. Returns the new type.
----------------
Typo trunsform.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:1303
+    // The custom pointers (fat pointers, buffer resources) don't work with load
+    // and store at this level. Fat pointers should haev been lowered to
+    // intrinsics before the translation to MIR.
----------------
Typo haev.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.h:256-257
 
+  // Convert the i128 that a addrspace(8) pointer is natively represented as
+  // into the v4i32 that all the buffer intrinsics expent to receive. We can't
+  // add register classes for i128 on pain of the promotion logic going haywire,
----------------
a addrspace -> an addrspace ?
expent -> expect


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/fp64-atomics-gfx90a.ll:3
 ; RUN: llc -global-isel -march=amdgcn -mcpu=gfx90a -verify-machineinstrs < %s | FileCheck %s -check-prefix=GFX90A
+; RUN: llc < %s -march=amdgcn -mcpu=gfx940 -verify-machineinstrs | FileCheck %s -check-prefix=GFX940
 
----------------
This should be testing gfx90a, not gfx940, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147547/new/

https://reviews.llvm.org/D147547