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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
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)
----------------
foad wrote:
> Is there any good reason why this widens to s64 and then s128, insteading of doing it in one step?
I think this makes more sense for zext, where the high 64-bits can be done with a single s_mov_b64, and anyext just inherits from this logic


================
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.
+---
----------------
foad wrote:
> Where is the tbaa preserved in the GMIR for these tests? I don't see it.
It's not, this is for the follow on commit to fix 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]]
----------------
foad wrote:
> Ouch that's a big expansion! I hope most of it can be optimized away one day.
There are a lot of legalization messes to deal with


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

https://reviews.llvm.org/D85584