[PATCH] D126009: [AMDGPU] Enforce alignment of image vaddr on gfx90a

[Brendon Cahoon via Phabricator via llvm-commits]

bcahoon added a comment.

This looks good. Just a couple of minor comments.



================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:8445
+
+bool SIInstrInfo::enforceOperandRCAlignment(MachineInstr &MI,
+                                            unsigned OpName) const {
----------------
This method returns bool, but none of the callers check the result.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.h:1148
+  // This is used if an operand is a 32 bit register but needs to be aligned
+  // regadless.
+  bool enforceOperandRCAlignment(MachineInstr &MI, unsigned OpName) const;
----------------
spelling, regardless


================
Comment at: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.image.sample.dim.gfx90a.ll:73
+; GFX90A-LABEL: {{^}}sample_1d_addr_align:
+; GFX90A: v_mov_b32_e32 [[VADDR:v[02468][0-9]*]], v1
+; SDAG:   image_sample v{{[0-9]+}}, [[VADDR]], s[{{[0-9:]+}}], s[{{[0-9:]+}}] dmask:0x1
----------------
Is this pattern correct? I believe it would allow v21 for example.


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

https://reviews.llvm.org/D126009