[PATCH] D24125: [AMDGPU] Promote uniform i16 ops to i32 ops

[Matt Arsenault via llvm-commits]

arsenm added a comment.

Needs a dedicated test which just runs the pass with opt for all of the operations


================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:179
@@ -151,1 +178,3 @@
 
+bool AMDGPUCodeGenPrepare::visitBinaryOperator(BinaryOperator &I) {
+  bool Changed = false;
----------------
You can't use zext to promote any operation, you must use sext for the signed operations.

================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:179
@@ -151,1 +178,3 @@
 
+bool AMDGPUCodeGenPrepare::visitBinaryOperator(BinaryOperator &I) {
+  bool Changed = false;
----------------
arsenm wrote:
> You can't use zext to promote any operation, you must use sext for the signed operations.
This also won't get selects for the min/max pattern

================
Comment at: lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp:183
@@ +182,3 @@
+  // Promote uniform 16 bit operation to equivalent 32 bit operation.
+  if (DA->isUniform(&I) && I.getType()->isIntegerTy(16))
+    Changed |= promoteUniformI16OpToI32Op(I);
----------------
This should check the type first since it is cheaper. We also should investigate whether this should be done for smaller types that will be legalized to i16

================
Comment at: test/CodeGen/AMDGPU/mul_uint24.ll:28
@@ -28,2 +27,3 @@
+; SI: s_sext_i32_i16
 define void @test_umul24_i16_sext(i32 addrspace(1)* %out, i16 %a, i16 %b) {
 entry:
----------------
I'm surprised this test broke from this. These changed cases should then be duplicated into a version with VGPRs to keep checking the pattern this was meant to


https://reviews.llvm.org/D24125