[AMDGPU][GISel] Missing FMAX3 use

[qcolombet]

GISel fails to use the max3 (and probably min3) instruction on AMDGPU. Instead it uses a sequence of 2 max instructions.
SDISel gets this right.

I believe the AMDGPU port miss a port of the SITargetLowering::performMinMaxCombine optimization.

To Reproduce

Download the attached IR or copy/paste it from below.
Then run:

llc -O3 -march=amdgcn -mcpu=gfx942  -mtriple amdgcn-amd-hmcsa -global-isel=<0|1> repro.ll -o -

repro.ll.txt

Result

GISel:

	s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
	v_max_f32_e32 v0, v0, v0
	v_max_f32_e32 v0, 0, v0
	v_mov_b32_e32 v4, v1
	v_mov_b32_e32 v5, v2
	v_max_f32_e32 v0, 0, v0
	flat_store_dword v[4:5], v0
	s_waitcnt vmcnt(0) lgkmcnt(0)
	s_setpc_b64 s[30:31]

SDISel:

	s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
	v_mov_b32_e32 v3, v2
	v_mov_b32_e32 v2, v1
	v_max3_f32 v0, v0, 0, 0
	flat_store_dword v[2:3], v0
	s_waitcnt vmcnt(0) lgkmcnt(0)
	s_setpc_b64 s[30:31]

GISel uses 3 max instructions where SDISel manages to do the same thing with just one max3 instruction.

Note: The test case was automatically reduced hence the input values constants are not representative of the real workload.

Note

Input IR:

target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
target triple = "amdgcn-amd-amdhsa"

; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
declare <1 x float> @llvm.maxnum.v1f32(<1 x float>, <1 x float>) #0

; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
declare float @llvm.maxnum.f32(float, float) #0

define void @foo.bb374(<1 x float> %i466, ptr %out) {
newFuncRoot:
  %i497 = tail call <1 x float> @llvm.maxnum.v1f32(<1 x float> %i466, <1 x float> zeroinitializer)
  %i503 = extractelement <1 x float> %i497, i64 0
  %i507 = tail call float @llvm.maxnum.f32(float %i503, float 0.000000e+00)
  store float %i507, ptr %out
  ret void
}

attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-backend-amdgpu

Author: Quentin Colombet (qcolombet)

GISel fails to use the max3 (and probably min3) instruction on AMDGPU. Instead it uses a sequence of 2 max instructions. SDISel gets this right.

I believe the AMDGPU port miss a port of the SITargetLowering::performMinMaxCombine optimization.

To Reproduce

Download the attached IR or copy/paste it from below.
Then run:

llc -O3 -march=amdgcn -mcpu=gfx942  -mtriple amdgcn-amd-hmcsa -global-isel=<0|1> repro.ll -o -

repro.ll.txt

Result

GISel:

	s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
	v_max_f32_e32 v0, v0, v0
	v_max_f32_e32 v0, 0, v0
	v_mov_b32_e32 v4, v1
	v_mov_b32_e32 v5, v2
	v_max_f32_e32 v0, 0, v0
	flat_store_dword v[4:5], v0
	s_waitcnt vmcnt(0) lgkmcnt(0)
	s_setpc_b64 s[30:31]

SDISel:

	s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
	v_mov_b32_e32 v3, v2
	v_mov_b32_e32 v2, v1
	v_max3_f32 v0, v0, 0, 0
	flat_store_dword v[2:3], v0
	s_waitcnt vmcnt(0) lgkmcnt(0)
	s_setpc_b64 s[30:31]

GISel uses 3 max instructions where SDISel manages to do the same thing with just one max3 instruction.

Note: The test case was automatically reduced hence the input values constants are not representative of the real workload.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Quentin Colombet (qcolombet)

GISel fails to use the max3 (and probably min3) instruction on AMDGPU. Instead it uses a sequence of 2 max instructions. SDISel gets this right.

I believe the AMDGPU port miss a port of the SITargetLowering::performMinMaxCombine optimization.

To Reproduce

Download the attached IR or copy/paste it from below.
Then run:

llc -O3 -march=amdgcn -mcpu=gfx942  -mtriple amdgcn-amd-hmcsa -global-isel=<0|1> repro.ll -o -

repro.ll.txt

Result

GISel:

	s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
	v_max_f32_e32 v0, v0, v0
	v_max_f32_e32 v0, 0, v0
	v_mov_b32_e32 v4, v1
	v_mov_b32_e32 v5, v2
	v_max_f32_e32 v0, 0, v0
	flat_store_dword v[4:5], v0
	s_waitcnt vmcnt(0) lgkmcnt(0)
	s_setpc_b64 s[30:31]

SDISel:

	s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
	v_mov_b32_e32 v3, v2
	v_mov_b32_e32 v2, v1
	v_max3_f32 v0, v0, 0, 0
	flat_store_dword v[2:3], v0
	s_waitcnt vmcnt(0) lgkmcnt(0)
	s_setpc_b64 s[30:31]

GISel uses 3 max instructions where SDISel manages to do the same thing with just one max3 instruction.

Note: The test case was automatically reduced hence the input values constants are not representative of the real workload.

[Ruhung]

I’m interested in this issue. Could you assign it to me? Thanks.

[arsenm]

This is also a mess because of the canonicalizes we currently need to insert (although after #112852 we should be able to drop them)

[lialan]

Just trying to track things here. @Ruhung are you actively working on this?

[Ruhung]

Yes, still working on it. It's my first time working with GlobalISel, so it might take me some time to explore.

[lialan]

Yes, still working on it. It's my first time working with GlobalISel, so it might take me some time to explore.

@Ruhung one of my trick is that you can try to post your on-going effort in a draft PR, so people can know whether it is on the right track.

[Ruhung]

Yes, still working on it. It's my first time working with GlobalISel, so it might take me some time to explore.

@Ruhung one of my trick is that you can try to post your on-going effort in a draft PR, so people can know whether it is on the right track.

Thanks for the suggestion! I’ll work on a draft PR by the end of this week.

[tpopp]

Just a note regarding something I've seen. A simple MAX(MAX) conversion to MAX3 only reduces ~half of the maxes, when the incoming IR is structured like a tree reduction because the "leaf maxes" are not sequentially feeding into each other.