[PATCH] D85584: AMDGPU/GlobalISel: Legalize odd sized loads with widening

[Jay Foad via Phabricator via llvm-commits]

foad accepted this revision.
foad added a comment.
This revision is now accepted and ready to land.

Looks good, just a couple of questions inline.



================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-load-constant.mir:12387-12391
+    ; CI: [[LOAD:%[0-9]+]]:_(s32) = G_LOAD [[COPY]](p4) :: (load 4, addrspace 4)
+    ; CI: [[DEF:%[0-9]+]]:_(s32) = G_IMPLICIT_DEF
+    ; CI: [[MV:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[LOAD]](s32), [[DEF]](s32)
+    ; CI: [[DEF1:%[0-9]+]]:_(s64) = G_IMPLICIT_DEF
+    ; CI: [[MV1:%[0-9]+]]:_(s128) = G_MERGE_VALUES [[MV]](s64), [[DEF1]](s64)
----------------
Is there any good reason why this widens to s64 and then s128, insteading of doing it in one step?


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-load-memory-metadata.mir:35-36
+
+# Make sure range metadata is not preserved when widening loads, but
+# tbaa is.
+---
----------------
Where is the tbaa preserved in the GMIR for these tests? I don't see it.


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-store.mir:643-666
+    ; SI: [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[UV]](s32)
+    ; SI: [[AND:%[0-9]+]]:_(s16) = G_AND [[TRUNC]], [[C]]
+    ; SI: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 8
+    ; SI: [[COPY2:%[0-9]+]]:_(s32) = COPY [[C1]](s32)
+    ; SI: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
+    ; SI: [[COPY3:%[0-9]+]]:_(s32) = COPY [[UV1]](s32)
+    ; SI: [[AND1:%[0-9]+]]:_(s32) = G_AND [[COPY3]], [[C2]]
----------------
Ouch that's a big expansion! I hope most of it can be optimized away one day.


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

https://reviews.llvm.org/D85584