[AMDGPU] Propagate constants for llvm.amdgcn.wave.reduce.umin/umax

[pravinjagtap]

Formatting

[sameerds]

Should the change description say "propagate constants for llvm.amdgcn.wave.reduce.umin/umax"? That seems more informative.

[foad]

llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp

1139

This should go in lib/Analysis/ConstantFolding.cpp instead.

llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll

6197

Tests can go in test/Transforms/InstSimplify/ConstProp/AMDGPU/.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp

1139

that's true of many folds we do here, but I thought we were supposed to avoid putting target intrinsics there. We do lose out of the benefits of being a pure analysis here though

1144

isa<Constant> covers isa<Poison>

[pravinjagtap]

Moved const fold impl from /lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp to
lib/Analysis/ConstantFolding.cpp

[pravinjagtap]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.reduce.umax.ll

138 ↗

(On Diff #543881)

Hello @foad, I am not sure why const fold optimization is not being applied here in llc.

[foad]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.reduce.umax.ll

138 ↗

(On Diff #543881)

llc does not run IR optimization passes. Use opt for that.

[pravinjagtap]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.reduce.umax.ll

138 ↗

(On Diff #543881)

If you notice, poison_value test point is being modified after early-cse pass during llc invocation in this test because of the current change in ConstantFolding.cpp. I was expecting same for const_value test point here.

[foad]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.reduce.umax.ll

138 ↗

(On Diff #543881)

Why do you think "const fold optimization is not being applied here"? Looking at the ISA, it just stores a constant value. What did you expect to see?

[pravinjagtap]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.reduce.umax.ll

138 ↗

(On Diff #543881)

Agree. Sorry, I overlooked. The optimization is applied in this case also.

[arsenm]

llvm/lib/Analysis/ConstantFolding.cpp

2851 ↗

(On Diff #543881)

It doesn't need to be a literal int isa<Constant> should cover every case

llvm/test/Transforms/InstSimplify/ConstProp/AMDGPU/wave.reduce.ll

33 ↗

(On Diff #543881)

Also try something exotic, like ptrtoint of a global declaration

[pravinjagtap]

Addressed review comments

[pravinjagtap]

llvm/test/Transforms/InstSimplify/ConstProp/AMDGPU/wave.reduce.ll

64 ↗

(On Diff #544173)

This usecase is not being optimized (const-fold).

[arsenm]

LGTM with the spurious include dropped

llvm/lib/Analysis/ConstantFolding.cpp

63 ↗

(On Diff #544173)

Don't add new include

llvm/test/Transforms/InstSimplify/ConstProp/AMDGPU/wave.reduce.ll

64 ↗

(On Diff #544173)

Don't see why that would be but it's unimportant

[pravinjagtap]

Addressed review comments

llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp

case Intrinsic::amdgcn_image_store_mip_cube: {
     int DMaskIdx = getAMDGPUImageDMaskIntrinsic(II.getIntrinsicID()) ? 1 : -1;
     if (simplifyAMDGCNMemoryIntrinsicDemanded(IC, II, DemandedElts, DMaskIdx,
                                               false)) {
       return IC.eraseInstFromFunction(II);
     }
 
     break;
   }
+  case Intrinsic::amdgcn_wave_reduce_umin:
+  case Intrinsic::amdgcn_wave_reduce_umax: {
+    Value *Src = II.getArgOperand(0);
+
+    // Propagate poison and constant value
+    if (isa<PoisonValue>(Src) || isa<Constant>(Src))
+      return IC.replaceInstUsesWith(II, Src);
+
+    break;
+  }
   }
   if (const AMDGPU::ImageDimIntrinsicInfo *ImageDimIntr =
             AMDGPU::getImageDimIntrinsicInfo(II.getIntrinsicID())) {
     return simplifyAMDGCNImageIntrinsic(ST, ImageDimIntr, II, IC);
   }
   return std::nullopt;
 }
 

llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll

 define half @test_constant_fold_exp2_f16_neg_denorm() {
 ; CHECK-LABEL: @test_constant_fold_exp2_f16_neg_denorm(
 ; CHECK-NEXT:    [[VAL:%.*]] = call half @llvm.amdgcn.exp2.f16(half 0xH83FF)
 ; CHECK-NEXT:    ret half [[VAL]]
 ;
   %val = call half @llvm.amdgcn.exp2.f16(half 0xH83ff)
   ret half %val
 }
+
+; --------------------------------------------------------------------
+; llvm.amdgcn.wave.reduce.umin.i32
+; --------------------------------------------------------------------
+
+declare i32 @llvm.amdgcn.wave.reduce.umin.i32(i32, i32 immarg)
+declare i32 @llvm.amdgcn.wave.reduce.umax.i32(i32, i32 immarg)
+
+define amdgpu_kernel void @test_constant_fold_wave_reduce_umin_poison(ptr addrspace(1) %out, i32 %in) {
+; CHECK-LABEL: @test_constant_fold_wave_reduce_umin_poison(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret void
+;
+entry:
+  %result = call i32 @llvm.amdgcn.wave.reduce.umin.i32(i32 poison, i32 1)
+  store i32 %result, ptr addrspace(1) %out
+  ret void
+}
+
+define amdgpu_kernel void @test_constant_fold_wave_reduce_umin_const(ptr addrspace(1) %out) {
+; CHECK-LABEL: @test_constant_fold_wave_reduce_umin_const(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    store i32 123, ptr addrspace(1) [[OUT:%.*]], align 4
+; CHECK-NEXT:    ret void
+;
+entry:
+  %result = call i32 @llvm.amdgcn.wave.reduce.umin.i32(i32 123, i32 1)
+  store i32 %result, ptr addrspace(1) %out
+  ret void
+}
+
+define amdgpu_kernel void @test_constant_fold_wave_reduce_umax_poison(ptr addrspace(1) %out, i32 %in) {
+; CHECK-LABEL: @test_constant_fold_wave_reduce_umax_poison(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret void
+;
+entry:
+  %result = call i32 @llvm.amdgcn.wave.reduce.umax.i32(i32 poison, i32 1)
+  store i32 %result, ptr addrspace(1) %out
+  ret void
+}
+
+define amdgpu_kernel void @test_constant_fold_wave_reduce_umax_const(ptr addrspace(1) %out) {
+; CHECK-LABEL: @test_constant_fold_wave_reduce_umax_const(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    store i32 123, ptr addrspace(1) [[OUT:%.*]], align 4
+; CHECK-NEXT:    ret void
+;
+entry:
+  %result = call i32 @llvm.amdgcn.wave.reduce.umax.i32(i32 123, i32 1)
+  store i32 %result, ptr addrspace(1) %out
+  ret void
+}