[PATCH] D48826: [AMDGPU] Add support for TFE/LWE in image intrinsics

[David Stuttard via Phabricator via llvm-commits]

dstuttard marked 7 inline comments as done.
dstuttard added a comment.

Minor changes made.

I'll look at implementing the extra tests you highlight - I think at least one of those might already be covered, but I'll verify that during the implementation of the new ones.
I think we might also have an issue with non-TFE/LWE variants that have a return type that is larger than the dmask (the last of your bullets) - so fixing this for the TFE/LWE case might also fix that - I'll check.

Regarding the suggested re-factor to remove the spooky effect at a distance code :) - agreed that it is a bit of a muddle as it stands. I think your suggestion is a good one so I'll look at re-factoring as suggested.



================
Comment at: lib/Target/AMDGPU/SIAddIMGInit.cpp:86-87
+        // variants.
+        if (!TFE && !LWE)
+          continue;
+
----------------
nhaehnle wrote:
> I'd prefer this to be an assertion. It's easy enough to change if we ever do get MIMG instructions without TFE/LWE. In the meantime, assertions allow us to be more explicit about the space of possibilities we actually need to think about.
Agreed, that makes sense


Repository:
  rL LLVM

https://reviews.llvm.org/D48826