Skip to content

Commit

Permalink
[AMDGPU] Refresh fmin_legacy.ll checks to fix issue reported on D77354
Browse files Browse the repository at this point in the history 
Some of the condition codes had inverted since this was generated
  • Loading branch information
@RKSimon
RKSimon committed Apr 8, 2020
1 parent 0125db9 commit 5450247
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions llvm/test/CodeGen/AMDGPU/fmin_legacy.ll
View file
Expand Up @@ -130,7 +130,7 @@ define amdgpu_kernel void @test_fmin_legacy_ole_f32(float addrspace(1)* %out, fl

; SI-SAFE: v_min_legacy_f32_e32 {{v[0-9]+}}, [[A]], [[B]]

; VI-SAFE v_cmp_lt_f32_e32 vcc, [[A]], [[B]]
; VI-SAFE: v_cmp_lt_f32_e32 vcc, [[A]], [[B]]
; VI-SAFE: v_cndmask_b32_e32 v{{[0-9]+}}, [[B]], [[A]]

; GCN-NONAN: v_min_f32_e32 {{v[0-9]+}}, [[A]], [[B]]
Expand All @@ -154,7 +154,7 @@ define amdgpu_kernel void @test_fmin_legacy_olt_f32(float addrspace(1)* %out, fl

; SI-SAFE: v_min_legacy_f32_e32 {{v[0-9]+}}, [[B]], [[A]]

; VI-SAFE v_cmp_lt_f32_e32 vcc, [[A]], [[B]]
; VI-SAFE: v_cmp_nge_f32_e32 vcc, [[A]], [[B]]
; VI-SAFE: v_cndmask_b32_e32 v{{[0-9]+}}, [[B]], [[A]]

; GCN-NONAN: v_min_f32_e32 {{v[0-9]+}}, [[A]], [[B]]
Expand All @@ -178,7 +178,7 @@ define amdgpu_kernel void @test_fmin_legacy_ult_f32(float addrspace(1)* %out, fl

; SI-SAFE: v_min_legacy_f32_e32 {{v[0-9]+}}, [[B]], [[A]]

; VI-SAFE v_cmp_lt_f32_e32 vcc, [[A]], [[B]]
; VI-SAFE: v_cmp_nge_f32_e32 vcc, [[A]], [[B]]
; VI-SAFE: v_cndmask_b32_e32 v{{[0-9]+}}, [[B]], [[A]]

; GCN-NONAN: v_min_f32_e32 {{v[0-9]+}}, [[A]], [[B]]
Expand All @@ -202,9 +202,9 @@ define amdgpu_kernel void @test_fmin_legacy_ult_v1f32(<1 x float> addrspace(1)*
; SI-SAFE: v_min_legacy_f32_e32
; SI-SAFE: v_min_legacy_f32_e32

; VI-SAFE v_cmp_lt_f32_e32
; VI-SAFE: v_cmp_nge_f32_e32
; VI-SAFE: v_cndmask_b32_e32
; VI-SAFE v_cmp_lt_f32_e32
; VI-SAFE: v_cmp_nge_f32_e32
; VI-SAFE: v_cndmask_b32_e32

; GCN-NONAN: v_min_f32_e32
Expand Down

3 comments on commit 5450247

@jayfoad
Copy link
Contributor

@jayfoad jayfoad commented on 5450247 Apr 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lt and nge handle NaNs differently so are you sure this is OK?

@nhaehnle
Copy link
Collaborator

@nhaehnle nhaehnle commented on 5450247 Apr 9, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah scratch my initial comment here.

You're right, the exact complement of lt would be nlt, so this is indeed somewhat suspicious.

@RKSimon
Copy link
Collaborator Author

@RKSimon RKSimon commented on 5450247 Apr 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the codegen almost certainly needs reviewing - my commit was purely to do with actually enabling the check instead of it blindly passing. It could be that the code is correct in broader context - maybe regenerating with the update scripts would be useful to see/test whats really going on?

Please sign in to comment.