[PATCH] D148957: [AMDGPU] Add intrinsic for converting global pointers to resources

[Krzysztof Drewniak via Phabricator via llvm-commits]

krzysz00 marked an inline comment as done.
krzysz00 added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:5217-5218
+  // an abundance of caution.
+  case Intrinsic::amdgcn_flat_as_buffer_rsrc:
+  case Intrinsic::amdgcn_global_as_buffer_rsrc:
     return true;
----------------
krzysz00 wrote:
> arsenm wrote:
> > This handling needs a test (I'm assuming that was the intent of ptr-buffer-alias-scheduling.ll, but I think we also need a pure IR one that doesn't depend on codegen)
> The test over in LICM is handling this, though there might be a more straightforward way to do it.
Unless you can think of a "purer" way to check this


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:4395
+bool AMDGPULegalizerInfo::legalizePointerAsRsrcIntrin(
+    MachineInstr &MI, MachineRegisterInfo &MRI, MachineIRBuilder &B) const {
+  Register Result = MI.getOperand(0).getReg();
----------------
arsenm wrote:
> krzysz00 wrote:
> > gandhi21299 wrote:
> > > Might be helpful to have an assertion on the number of operands of `MI`. Is it possible that any of the operands is not a register?
> > From what I can tell of all the surrounding code ... no?
> Only if any operands are immarg, which they aren't (IIRC this was a MachineVerifier check which is missing)
Ok so this is basically fine as is, sounds like


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148957