[PATCH] D124232: [AMDGPU] Use d16 flag for image.sample instructions

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

Could do with some tests for other instructions like image_gather4, image_load, image_msaa_load.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:115
 /// the modified arguments.
 static Optional<Instruction *> modifyIntrinsicCall(
+    IntrinsicInst &OldIntr, Instruction &InstToReplace, unsigned NewIntr,
----------------
Please update the comment to describe the new arguments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:232
+      // intrinsic with D16 flag.
+      if (II.hasOneUse()) {
+        Instruction *user = II.user_back();
----------------
As a further improvement you could handle multiple uses if all of them are fptruncs - but perhaps that will never happen in practice because of CSE.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:233
+      if (II.hasOneUse()) {
+        Instruction *user = II.user_back();
+
----------------
LLVM uses CamelCase for variable names, like `User`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124232