[PATCH] D73051: [GlobalISel][AMDGPU] Legalize saturating add/subtract

Thu Jul 23 01:35:33 PDT 2020

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

LGTM, thanks.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:443
+
+    // Technically the saturating operations require clamp bit support, but this
+    // was introduced at the same time as 16-bit operations.
----------------
This comment seems to be in an odd place (not next to the saturating operations) and/or redundant with the assert just above?

================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-saddsat.mir:491-492
+    ; GFX9: [[BUILD_VECTOR_TRUNC1:%[0-9]+]]:_(<2 x s16>) = G_BUILD_VECTOR_TRUNC [[C1]](s32), [[C1]](s32)
+    ; GFX9: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
+    ; GFX9: [[BUILD_VECTOR_TRUNC2:%[0-9]+]]:_(<2 x s16>) = G_BUILD_VECTOR_TRUNC [[C2]](s32), [[C2]](s32)
+    ; GFX9: [[SMAX:%[0-9]+]]:_(<2 x s16>) = G_SMAX [[COPY]], [[BUILD_VECTOR_TRUNC2]]
----------------
Just curious: can't G_CONSTANT represent vector constants directly?

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

https://reviews.llvm.org/D73051