[AMDGPU] Update log lowering to remove contract for AMDGCN backend #168916
+627
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
Member
|
@llvm/pr-subscribers-backend-amdgpu Author: Adel Ejjeh (adelejjeh) ChangesProblem SummaryPyTorch's - float logf(float __x) { return __FAST_OR_SLOW(__logf, __ocml_log_f32)(__x); }
+ float logf(float __x) { return __FAST_OR_SLOW(__logf, __builtin_logf)(__x); }This change exposed a problem in the AMDGCN back-end as described below: Key Findings1. Contract flag propagation: When 2. Behavior change from OCML to builtin path:
; Function Attrs: alwaysinline convergent mustprogress nounwind
define internal noundef float @<!-- -->_ZL4logff(float noundef %__x) #<!-- -->6 {
entry:
%retval = alloca float, align 4, addrspace(5)
%__x.addr = alloca float, align 4, addrspace(5)
%retval.ascast = addrspacecast ptr addrspace(5) %retval to ptr
%__x.addr.ascast = addrspacecast ptr addrspace(5) %__x.addr to ptr
store float %__x, ptr %__x.addr.ascast, align 4, !tbaa !23
%0 = load float, ptr %__x.addr.ascast, align 4, !tbaa !23
%call = call contract float @<!-- -->__ocml_log_f32(float noundef %0) #<!-- -->23
ret float %call
}
; Function Attrs: convergent mustprogress nofree norecurse nosync nounwind willreturn memory(none)
define internal noundef float @<!-- -->__ocml_log_f32(float noundef %0) #<!-- -->7 {
%2 = tail call float @<!-- -->llvm.log.f32(float %0)
ret float %2
}
; Function Attrs: alwaysinline convergent mustprogress nounwind
define internal noundef float @<!-- -->_ZL4logff(float noundef %__x) #<!-- -->6 {
entry:
%retval = alloca float, align 4, addrspace(5)
%__x.addr = alloca float, align 4, addrspace(5)
%retval.ascast = addrspacecast ptr addrspace(5) %retval to ptr
%__x.addr.ascast = addrspacecast ptr addrspace(5) %__x.addr to ptr
store float %__x, ptr %__x.addr.ascast, align 4, !tbaa !24
%0 = load float, ptr %__x.addr.ascast, align 4, !tbaa !24
%1 = call contract float @<!-- -->llvm.log.f32(float %0)
ret float %1
}3. Why contract breaks log: Our AMDGCM target back end implements the natural logarithm by taking the result of the hardware log, then multiplying that by r = y * c1; // y is result of v_log_ instruction, c1 = ln(2)
r = r + fma(y, c2, fma(y, c1, -r)) // c2 is another error-correcting constant v_log_f32_e32 v1, v1
s_mov_b32 s2, 0x3f317217
v_mul_f32_e32 v3, 0x3f317217, v1
v_fma_f32 v4, v1, s2, -v3
v_fmac_f32_e32 v4, 0x3377d1cf, v1
v_add_f32_e32 v3, v3, v4With the presence of the r = y * c1;
r = fma(y, c1, fma(y, c2, fma(y, c1, -r))); v_log_f32_e32 v1, v1
s_mov_b32 s2, 0x3f317217
v_mul_f32_e32 v3, 0x3f317217, v1
v_fma_f32 v3, v1, s2, -v3
v_fmac_f32_e32 v3, 0x3377d1cf, v1
v_fmac_f32_e32 v3, 0x3f317217, v1Solution and Proposed FixBased on our implementation of Note: I had originally implemented this fix in the FE by removing the Patch is 36.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/168916.diff 4 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
index db890df7c50f9..ffafe06211109 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
@@ -2766,7 +2766,9 @@ SDValue AMDGPUTargetLowering::LowerFLOGCommon(SDValue Op,
EVT VT = Op.getValueType();
SDNodeFlags Flags = Op->getFlags();
SDLoc DL(Op);
-
+ // Our implementation of LOG is not contract safe, so disable instruction
+ // contraction.
+ Flags.setAllowContract(false);
const bool IsLog10 = Op.getOpcode() == ISD::FLOG10;
assert(IsLog10 || Op.getOpcode() == ISD::FLOG);
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
index 1a13b2226ecd6..5c8b720c54761 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
@@ -3508,6 +3508,9 @@ bool AMDGPULegalizerInfo::legalizeFlogCommon(MachineInstr &MI,
MachineRegisterInfo &MRI = *B.getMRI();
Register Dst = MI.getOperand(0).getReg();
Register X = MI.getOperand(1).getReg();
+ // Our implementation of LOG is not contract safe, so disable contraction in
+ // the flags before reading the field.
+ MI.clearFlags(MachineInstr::FmContract);
unsigned Flags = MI.getFlags();
const LLT Ty = MRI.getType(X);
MachineFunction &MF = B.getMF();
diff --git a/llvm/test/CodeGen/AMDGPU/llvm.log.ll b/llvm/test/CodeGen/AMDGPU/llvm.log.ll
index fc6b2d95b2af8..88f202adba07e 100644
--- a/llvm/test/CodeGen/AMDGPU/llvm.log.ll
+++ b/llvm/test/CodeGen/AMDGPU/llvm.log.ll
@@ -316,6 +316,309 @@ define amdgpu_kernel void @s_log_f32(ptr addrspace(1) %out, float %in) {
ret void
}
+define amdgpu_kernel void @s_log_contract_f32(ptr addrspace(1) %out, float %in) {
+; SI-SDAG-LABEL: s_log_contract_f32:
+; SI-SDAG: ; %bb.0:
+; SI-SDAG-NEXT: s_load_dword s6, s[4:5], 0xb
+; SI-SDAG-NEXT: s_load_dwordx2 s[0:1], s[4:5], 0x9
+; SI-SDAG-NEXT: v_mov_b32_e32 v0, 0x800000
+; SI-SDAG-NEXT: v_mov_b32_e32 v1, 0x41b17218
+; SI-SDAG-NEXT: s_mov_b32 s4, 0x3f317217
+; SI-SDAG-NEXT: s_waitcnt lgkmcnt(0)
+; SI-SDAG-NEXT: v_cmp_lt_f32_e32 vcc, s6, v0
+; SI-SDAG-NEXT: s_and_b64 s[2:3], vcc, exec
+; SI-SDAG-NEXT: s_cselect_b32 s2, 32, 0
+; SI-SDAG-NEXT: v_cndmask_b32_e32 v0, 0, v1, vcc
+; SI-SDAG-NEXT: v_mov_b32_e32 v1, s2
+; SI-SDAG-NEXT: v_ldexp_f32_e32 v1, s6, v1
+; SI-SDAG-NEXT: v_log_f32_e32 v1, v1
+; SI-SDAG-NEXT: s_mov_b32 s3, 0xf000
+; SI-SDAG-NEXT: s_mov_b32 s2, -1
+; SI-SDAG-NEXT: v_mul_f32_e32 v2, 0x3f317217, v1
+; SI-SDAG-NEXT: v_fma_f32 v3, v1, s4, -v2
+; SI-SDAG-NEXT: s_mov_b32 s4, 0x3377d1cf
+; SI-SDAG-NEXT: v_fma_f32 v3, v1, s4, v3
+; SI-SDAG-NEXT: s_mov_b32 s4, 0x7f800000
+; SI-SDAG-NEXT: v_add_f32_e32 v2, v2, v3
+; SI-SDAG-NEXT: v_cmp_lt_f32_e64 vcc, |v1|, s4
+; SI-SDAG-NEXT: v_cndmask_b32_e32 v1, v1, v2, vcc
+; SI-SDAG-NEXT: v_sub_f32_e32 v0, v1, v0
+; SI-SDAG-NEXT: buffer_store_dword v0, off, s[0:3], 0
+; SI-SDAG-NEXT: s_endpgm
+;
+; SI-GISEL-LABEL: s_log_contract_f32:
+; SI-GISEL: ; %bb.0:
+; SI-GISEL-NEXT: s_load_dword s0, s[4:5], 0xb
+; SI-GISEL-NEXT: s_load_dwordx2 s[4:5], s[4:5], 0x9
+; SI-GISEL-NEXT: v_mov_b32_e32 v0, 0x800000
+; SI-GISEL-NEXT: v_mov_b32_e32 v1, 0x3f317217
+; SI-GISEL-NEXT: v_mov_b32_e32 v2, 0x3377d1cf
+; SI-GISEL-NEXT: s_waitcnt lgkmcnt(0)
+; SI-GISEL-NEXT: v_cmp_lt_f32_e32 vcc, s0, v0
+; SI-GISEL-NEXT: v_cndmask_b32_e64 v0, 0, 1, vcc
+; SI-GISEL-NEXT: v_lshlrev_b32_e32 v0, 5, v0
+; SI-GISEL-NEXT: v_ldexp_f32_e32 v0, s0, v0
+; SI-GISEL-NEXT: v_log_f32_e32 v0, v0
+; SI-GISEL-NEXT: v_mov_b32_e32 v3, 0x7f800000
+; SI-GISEL-NEXT: s_mov_b32 s6, -1
+; SI-GISEL-NEXT: s_mov_b32 s7, 0xf000
+; SI-GISEL-NEXT: v_mul_f32_e32 v4, 0x3f317217, v0
+; SI-GISEL-NEXT: v_fma_f32 v1, v0, v1, -v4
+; SI-GISEL-NEXT: v_fma_f32 v1, v0, v2, v1
+; SI-GISEL-NEXT: v_add_f32_e32 v1, v4, v1
+; SI-GISEL-NEXT: v_cmp_lt_f32_e64 s[0:1], |v0|, v3
+; SI-GISEL-NEXT: v_cndmask_b32_e64 v0, v0, v1, s[0:1]
+; SI-GISEL-NEXT: v_mov_b32_e32 v1, 0x41b17218
+; SI-GISEL-NEXT: v_cndmask_b32_e32 v1, 0, v1, vcc
+; SI-GISEL-NEXT: v_sub_f32_e32 v0, v0, v1
+; SI-GISEL-NEXT: buffer_store_dword v0, off, s[4:7], 0
+; SI-GISEL-NEXT: s_endpgm
+;
+; VI-SDAG-LABEL: s_log_contract_f32:
+; VI-SDAG: ; %bb.0:
+; VI-SDAG-NEXT: s_load_dword s2, s[4:5], 0x2c
+; VI-SDAG-NEXT: v_mov_b32_e32 v0, 0x800000
+; VI-SDAG-NEXT: v_mov_b32_e32 v1, 0x41b17218
+; VI-SDAG-NEXT: s_waitcnt lgkmcnt(0)
+; VI-SDAG-NEXT: v_cmp_lt_f32_e32 vcc, s2, v0
+; VI-SDAG-NEXT: s_and_b64 s[0:1], vcc, exec
+; VI-SDAG-NEXT: s_cselect_b32 s0, 32, 0
+; VI-SDAG-NEXT: v_cndmask_b32_e32 v0, 0, v1, vcc
+; VI-SDAG-NEXT: v_mov_b32_e32 v1, s0
+; VI-SDAG-NEXT: v_ldexp_f32 v1, s2, v1
+; VI-SDAG-NEXT: v_log_f32_e32 v1, v1
+; VI-SDAG-NEXT: s_load_dwordx2 s[0:1], s[4:5], 0x24
+; VI-SDAG-NEXT: s_mov_b32 s2, 0x7f800000
+; VI-SDAG-NEXT: v_and_b32_e32 v2, 0xfffff000, v1
+; VI-SDAG-NEXT: v_sub_f32_e32 v3, v1, v2
+; VI-SDAG-NEXT: v_mul_f32_e32 v4, 0x3805fdf4, v2
+; VI-SDAG-NEXT: v_mul_f32_e32 v5, 0x3f317000, v3
+; VI-SDAG-NEXT: v_mul_f32_e32 v3, 0x3805fdf4, v3
+; VI-SDAG-NEXT: v_add_f32_e32 v3, v4, v3
+; VI-SDAG-NEXT: v_mul_f32_e32 v2, 0x3f317000, v2
+; VI-SDAG-NEXT: v_add_f32_e32 v3, v5, v3
+; VI-SDAG-NEXT: v_add_f32_e32 v2, v2, v3
+; VI-SDAG-NEXT: v_cmp_lt_f32_e64 vcc, |v1|, s2
+; VI-SDAG-NEXT: v_cndmask_b32_e32 v1, v1, v2, vcc
+; VI-SDAG-NEXT: v_sub_f32_e32 v2, v1, v0
+; VI-SDAG-NEXT: s_waitcnt lgkmcnt(0)
+; VI-SDAG-NEXT: v_mov_b32_e32 v0, s0
+; VI-SDAG-NEXT: v_mov_b32_e32 v1, s1
+; VI-SDAG-NEXT: flat_store_dword v[0:1], v2
+; VI-SDAG-NEXT: s_endpgm
+;
+; VI-GISEL-LABEL: s_log_contract_f32:
+; VI-GISEL: ; %bb.0:
+; VI-GISEL-NEXT: s_load_dword s0, s[4:5], 0x2c
+; VI-GISEL-NEXT: s_load_dwordx2 s[2:3], s[4:5], 0x24
+; VI-GISEL-NEXT: v_mov_b32_e32 v0, 0x800000
+; VI-GISEL-NEXT: v_mov_b32_e32 v1, 0x7f800000
+; VI-GISEL-NEXT: s_waitcnt lgkmcnt(0)
+; VI-GISEL-NEXT: v_cmp_lt_f32_e32 vcc, s0, v0
+; VI-GISEL-NEXT: v_cndmask_b32_e64 v0, 0, 1, vcc
+; VI-GISEL-NEXT: v_lshlrev_b32_e32 v0, 5, v0
+; VI-GISEL-NEXT: v_ldexp_f32 v0, s0, v0
+; VI-GISEL-NEXT: v_log_f32_e32 v0, v0
+; VI-GISEL-NEXT: v_and_b32_e32 v2, 0xfffff000, v0
+; VI-GISEL-NEXT: v_sub_f32_e32 v3, v0, v2
+; VI-GISEL-NEXT: v_mul_f32_e32 v4, 0x3805fdf4, v2
+; VI-GISEL-NEXT: v_mul_f32_e32 v5, 0x3805fdf4, v3
+; VI-GISEL-NEXT: v_mul_f32_e32 v3, 0x3f317000, v3
+; VI-GISEL-NEXT: v_add_f32_e32 v4, v4, v5
+; VI-GISEL-NEXT: v_mul_f32_e32 v2, 0x3f317000, v2
+; VI-GISEL-NEXT: v_add_f32_e32 v3, v3, v4
+; VI-GISEL-NEXT: v_add_f32_e32 v2, v2, v3
+; VI-GISEL-NEXT: v_cmp_lt_f32_e64 s[0:1], |v0|, v1
+; VI-GISEL-NEXT: v_mov_b32_e32 v1, 0x41b17218
+; VI-GISEL-NEXT: v_cndmask_b32_e64 v0, v0, v2, s[0:1]
+; VI-GISEL-NEXT: v_cndmask_b32_e32 v1, 0, v1, vcc
+; VI-GISEL-NEXT: v_sub_f32_e32 v2, v0, v1
+; VI-GISEL-NEXT: v_mov_b32_e32 v0, s2
+; VI-GISEL-NEXT: v_mov_b32_e32 v1, s3
+; VI-GISEL-NEXT: flat_store_dword v[0:1], v2
+; VI-GISEL-NEXT: s_endpgm
+;
+; GFX900-SDAG-LABEL: s_log_contract_f32:
+; GFX900-SDAG: ; %bb.0:
+; GFX900-SDAG-NEXT: s_load_dword s6, s[4:5], 0x2c
+; GFX900-SDAG-NEXT: s_load_dwordx2 s[0:1], s[4:5], 0x24
+; GFX900-SDAG-NEXT: v_mov_b32_e32 v0, 0x800000
+; GFX900-SDAG-NEXT: v_mov_b32_e32 v1, 0x41b17218
+; GFX900-SDAG-NEXT: v_mov_b32_e32 v2, 0
+; GFX900-SDAG-NEXT: s_waitcnt lgkmcnt(0)
+; GFX900-SDAG-NEXT: v_cmp_lt_f32_e32 vcc, s6, v0
+; GFX900-SDAG-NEXT: s_and_b64 s[2:3], vcc, exec
+; GFX900-SDAG-NEXT: s_cselect_b32 s2, 32, 0
+; GFX900-SDAG-NEXT: v_cndmask_b32_e32 v0, 0, v1, vcc
+; GFX900-SDAG-NEXT: v_mov_b32_e32 v1, s2
+; GFX900-SDAG-NEXT: v_ldexp_f32 v1, s6, v1
+; GFX900-SDAG-NEXT: v_log_f32_e32 v1, v1
+; GFX900-SDAG-NEXT: s_mov_b32 s2, 0x3f317217
+; GFX900-SDAG-NEXT: s_mov_b32 s3, 0x3377d1cf
+; GFX900-SDAG-NEXT: v_mul_f32_e32 v3, 0x3f317217, v1
+; GFX900-SDAG-NEXT: v_fma_f32 v4, v1, s2, -v3
+; GFX900-SDAG-NEXT: v_fma_f32 v4, v1, s3, v4
+; GFX900-SDAG-NEXT: s_mov_b32 s2, 0x7f800000
+; GFX900-SDAG-NEXT: v_add_f32_e32 v3, v3, v4
+; GFX900-SDAG-NEXT: v_cmp_lt_f32_e64 vcc, |v1|, s2
+; GFX900-SDAG-NEXT: v_cndmask_b32_e32 v1, v1, v3, vcc
+; GFX900-SDAG-NEXT: v_sub_f32_e32 v0, v1, v0
+; GFX900-SDAG-NEXT: global_store_dword v2, v0, s[0:1]
+; GFX900-SDAG-NEXT: s_endpgm
+;
+; GFX900-GISEL-LABEL: s_log_contract_f32:
+; GFX900-GISEL: ; %bb.0:
+; GFX900-GISEL-NEXT: s_load_dword s0, s[4:5], 0x2c
+; GFX900-GISEL-NEXT: s_load_dwordx2 s[2:3], s[4:5], 0x24
+; GFX900-GISEL-NEXT: v_mov_b32_e32 v0, 0x800000
+; GFX900-GISEL-NEXT: v_mov_b32_e32 v2, 0x3f317217
+; GFX900-GISEL-NEXT: v_mov_b32_e32 v3, 0x3377d1cf
+; GFX900-GISEL-NEXT: s_waitcnt lgkmcnt(0)
+; GFX900-GISEL-NEXT: v_cmp_lt_f32_e32 vcc, s0, v0
+; GFX900-GISEL-NEXT: v_cndmask_b32_e64 v0, 0, 1, vcc
+; GFX900-GISEL-NEXT: v_lshlrev_b32_e32 v0, 5, v0
+; GFX900-GISEL-NEXT: v_ldexp_f32 v0, s0, v0
+; GFX900-GISEL-NEXT: v_log_f32_e32 v0, v0
+; GFX900-GISEL-NEXT: v_mov_b32_e32 v4, 0x7f800000
+; GFX900-GISEL-NEXT: v_mov_b32_e32 v1, 0
+; GFX900-GISEL-NEXT: v_mul_f32_e32 v5, 0x3f317217, v0
+; GFX900-GISEL-NEXT: v_fma_f32 v2, v0, v2, -v5
+; GFX900-GISEL-NEXT: v_fma_f32 v2, v0, v3, v2
+; GFX900-GISEL-NEXT: v_add_f32_e32 v2, v5, v2
+; GFX900-GISEL-NEXT: v_cmp_lt_f32_e64 s[0:1], |v0|, v4
+; GFX900-GISEL-NEXT: v_cndmask_b32_e64 v0, v0, v2, s[0:1]
+; GFX900-GISEL-NEXT: v_mov_b32_e32 v2, 0x41b17218
+; GFX900-GISEL-NEXT: v_cndmask_b32_e32 v2, 0, v2, vcc
+; GFX900-GISEL-NEXT: v_sub_f32_e32 v0, v0, v2
+; GFX900-GISEL-NEXT: global_store_dword v1, v0, s[2:3]
+; GFX900-GISEL-NEXT: s_endpgm
+;
+; GFX1100-SDAG-LABEL: s_log_contract_f32:
+; GFX1100-SDAG: ; %bb.0:
+; GFX1100-SDAG-NEXT: s_load_b32 s0, s[4:5], 0x2c
+; GFX1100-SDAG-NEXT: s_waitcnt lgkmcnt(0)
+; GFX1100-SDAG-NEXT: v_cmp_gt_f32_e64 s1, 0x800000, s0
+; GFX1100-SDAG-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_2) | instid1(SALU_CYCLE_1)
+; GFX1100-SDAG-NEXT: v_cndmask_b32_e64 v0, 0, 0x41b17218, s1
+; GFX1100-SDAG-NEXT: s_and_b32 s1, s1, exec_lo
+; GFX1100-SDAG-NEXT: s_cselect_b32 s1, 32, 0
+; GFX1100-SDAG-NEXT: v_ldexp_f32 v1, s0, s1
+; GFX1100-SDAG-NEXT: s_load_b64 s[0:1], s[4:5], 0x24
+; GFX1100-SDAG-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_3) | instid1(VALU_DEP_2)
+; GFX1100-SDAG-NEXT: v_log_f32_e32 v1, v1
+; GFX1100-SDAG-NEXT: s_waitcnt_depctr 0xfff
+; GFX1100-SDAG-NEXT: v_mul_f32_e32 v2, 0x3f317217, v1
+; GFX1100-SDAG-NEXT: v_cmp_gt_f32_e64 vcc_lo, 0x7f800000, |v1|
+; GFX1100-SDAG-NEXT: v_fma_f32 v3, 0x3f317217, v1, -v2
+; GFX1100-SDAG-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
+; GFX1100-SDAG-NEXT: v_fmamk_f32 v3, v1, 0x3377d1cf, v3
+; GFX1100-SDAG-NEXT: v_add_f32_e32 v2, v2, v3
+; GFX1100-SDAG-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
+; GFX1100-SDAG-NEXT: v_dual_cndmask_b32 v1, v1, v2 :: v_dual_mov_b32 v2, 0
+; GFX1100-SDAG-NEXT: v_sub_f32_e32 v0, v1, v0
+; GFX1100-SDAG-NEXT: s_waitcnt lgkmcnt(0)
+; GFX1100-SDAG-NEXT: global_store_b32 v2, v0, s[0:1]
+; GFX1100-SDAG-NEXT: s_endpgm
+;
+; GFX1100-GISEL-LABEL: s_log_contract_f32:
+; GFX1100-GISEL: ; %bb.0:
+; GFX1100-GISEL-NEXT: s_load_b32 s0, s[4:5], 0x2c
+; GFX1100-GISEL-NEXT: s_waitcnt lgkmcnt(0)
+; GFX1100-GISEL-NEXT: v_cmp_gt_f32_e64 s2, 0x800000, s0
+; GFX1100-GISEL-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
+; GFX1100-GISEL-NEXT: v_cndmask_b32_e64 v0, 0, 1, s2
+; GFX1100-GISEL-NEXT: v_lshlrev_b32_e32 v0, 5, v0
+; GFX1100-GISEL-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_1) | instid1(VALU_DEP_1)
+; GFX1100-GISEL-NEXT: v_ldexp_f32 v0, s0, v0
+; GFX1100-GISEL-NEXT: s_load_b64 s[0:1], s[4:5], 0x24
+; GFX1100-GISEL-NEXT: v_log_f32_e32 v0, v0
+; GFX1100-GISEL-NEXT: s_waitcnt_depctr 0xfff
+; GFX1100-GISEL-NEXT: v_mul_f32_e32 v1, 0x3f317217, v0
+; GFX1100-GISEL-NEXT: v_cmp_gt_f32_e64 vcc_lo, 0x7f800000, |v0|
+; GFX1100-GISEL-NEXT: s_delay_alu instid0(VALU_DEP_2) | instskip(NEXT) | instid1(VALU_DEP_1)
+; GFX1100-GISEL-NEXT: v_fma_f32 v2, 0x3f317217, v0, -v1
+; GFX1100-GISEL-NEXT: v_fmac_f32_e32 v2, 0x3377d1cf, v0
+; GFX1100-GISEL-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
+; GFX1100-GISEL-NEXT: v_dual_add_f32 v1, v1, v2 :: v_dual_mov_b32 v2, 0
+; GFX1100-GISEL-NEXT: v_cndmask_b32_e32 v0, v0, v1, vcc_lo
+; GFX1100-GISEL-NEXT: v_cndmask_b32_e64 v1, 0, 0x41b17218, s2
+; GFX1100-GISEL-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GFX1100-GISEL-NEXT: v_sub_f32_e32 v0, v0, v1
+; GFX1100-GISEL-NEXT: s_waitcnt lgkmcnt(0)
+; GFX1100-GISEL-NEXT: global_store_b32 v2, v0, s[0:1]
+; GFX1100-GISEL-NEXT: s_endpgm
+;
+; R600-LABEL: s_log_contract_f32:
+; R600: ; %bb.0:
+; R600-NEXT: ALU 23, @4, KC0[CB0:0-32], KC1[]
+; R600-NEXT: MEM_RAT_CACHELESS STORE_RAW T0.X, T1.X, 1
+; R600-NEXT: CF_END
+; R600-NEXT: PAD
+; R600-NEXT: ALU clause starting at 4:
+; R600-NEXT: SETGT * T0.W, literal.x, KC0[2].Z,
+; R600-NEXT: 8388608(1.175494e-38), 0(0.000000e+00)
+; R600-NEXT: CNDE * T1.W, PV.W, 1.0, literal.x,
+; R600-NEXT: 1333788672(4.294967e+09), 0(0.000000e+00)
+; R600-NEXT: MUL_IEEE * T1.W, KC0[2].Z, PV.W,
+; R600-NEXT: LOG_IEEE * T0.X, PV.W,
+; R600-NEXT: AND_INT * T1.W, PS, literal.x,
+; R600-NEXT: -4096(nan), 0(0.000000e+00)
+; R600-NEXT: ADD * T2.W, T0.X, -PV.W,
+; R600-NEXT: MUL_IEEE * T3.W, PV.W, literal.x,
+; R600-NEXT: 939916788(3.194618e-05), 0(0.000000e+00)
+; R600-NEXT: MULADD_IEEE * T3.W, T1.W, literal.x, PV.W,
+; R600-NEXT: 939916788(3.194618e-05), 0(0.000000e+00)
+; R600-NEXT: MULADD_IEEE * T2.W, T2.W, literal.x, PV.W,
+; R600-NEXT: 1060204544(6.931152e-01), 0(0.000000e+00)
+; R600-NEXT: MULADD_IEEE T1.W, T1.W, literal.x, PV.W,
+; R600-NEXT: SETGT * T2.W, literal.y, |T0.X|,
+; R600-NEXT: 1060204544(6.931152e-01), 2139095040(INF)
+; R600-NEXT: CNDE T1.W, PS, T0.X, PV.W,
+; R600-NEXT: CNDE * T0.W, T0.W, 0.0, literal.x,
+; R600-NEXT: 1102148120(2.218071e+01), 0(0.000000e+00)
+; R600-NEXT: ADD T0.X, PV.W, -PS,
+; R600-NEXT: LSHR * T1.X, KC0[2].Y, literal.x,
+; R600-NEXT: 2(2.802597e-45), 0(0.000000e+00)
+;
+; CM-LABEL: s_log_contract_f32:
+; CM: ; %bb.0:
+; CM-NEXT: ALU 26, @4, KC0[CB0:0-32], KC1[]
+; CM-NEXT: MEM_RAT_CACHELESS STORE_DWORD T0.X, T1.X
+; CM-NEXT: CF_END
+; CM-NEXT: PAD
+; CM-NEXT: ALU clause starting at 4:
+; CM-NEXT: SETGT * T0.W, literal.x, KC0[2].Z,
+; CM-NEXT: 8388608(1.175494e-38), 0(0.000000e+00)
+; CM-NEXT: CNDE * T1.W, PV.W, 1.0, literal.x,
+; CM-NEXT: 1333788672(4.294967e+09), 0(0.000000e+00)
+; CM-NEXT: MUL_IEEE * T1.W, KC0[2].Z, PV.W,
+; CM-NEXT: LOG_IEEE T0.X, T1.W,
+; CM-NEXT: LOG_IEEE T0.Y (MASKED), T1.W,
+; CM-NEXT: LOG_IEEE T0.Z (MASKED), T1.W,
+; CM-NEXT: LOG_IEEE * T0.W (MASKED), T1.W,
+; CM-NEXT: AND_INT * T1.W, PV.X, literal.x,
+; CM-NEXT: -4096(nan), 0(0.000000e+00)
+; CM-NEXT: ADD * T2.W, T0.X, -PV.W,
+; CM-NEXT: MUL_IEEE * T3.W, PV.W, literal.x,
+; CM-NEXT: 939916788(3.194618e-05), 0(0.000000e+00)
+; CM-NEXT: MULADD_IEEE * T3.W, T1.W, literal.x, PV.W,
+; CM-NEXT: 939916788(3.194618e-05), 0(0.000000e+00)
+; CM-NEXT: MULADD_IEEE * T2.W, T2.W, literal.x, PV.W,
+; CM-NEXT: 1060204544(6.931152e-01), 0(0.000000e+00)
+; CM-NEXT: MULADD_IEEE T0.Z, T1.W, literal.x, PV.W,
+; CM-NEXT: SETGT * T1.W, literal.y, |T0.X|,
+; CM-NEXT: 1060204544(6.931152e-01), 2139095040(INF)
+; CM-NEXT: CNDE T0.Z, PV.W, T0.X, PV.Z,
+; CM-NEXT: CNDE * T0.W, T0.W, 0.0, literal.x,
+; CM-NEXT: 1102148120(2.218071e+01), 0(0.000000e+00)
+; CM-NEXT: ADD * T0.X, PV.Z, -PV.W,
+; CM-NEXT: LSHR * T1.X, KC0[2].Y, literal.x,
+; CM-NEXT: 2(2.802597e-45), 0(0.000000e+00)
+ %result = call contract float @llvm.log.f32(float %in)
+ store float %result, ptr addrspace(1) %out
+ ret void
+}
+
; FIXME: We should be able to merge these packets together on Cayman so we
; have a maximum of 4 instructions.
define amdgpu_kernel void @s_log_v2f32(ptr addrspace(1) %out, <2 x float> %in) {
@@ -6439,6 +6742,8 @@ define half @v_log_f16_fast(half %in) {
; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v0, v0
; SI-SDAG-NEXT: v_log_f32_e32 v0, v0
; SI-SDAG-NEXT: v_mul_f32_e32 v0, 0x3f317218, v0
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v0, v0
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v0, v0
; SI-SDAG-NEXT: s_setpc_b64 s[30:31]
;
; SI-GISEL-LABEL: v_log_f16_fast:
@@ -7100,6 +7405,10 @@ define <2 x half> @v_log_v2f16_fast(<2 x half> %in) {
; SI-SDAG-NEXT: v_log_f32_e32 v1, v1
; SI-SDAG-NEXT: v_mul_f32_e32 v0, 0x3f317218, v0
; SI-SDAG-NEXT: v_mul_f32_e32 v1, 0x3f317218, v1
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v0, v0
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v1, v1
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v0, v0
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v1, v1
; SI-SDAG-NEXT: s_setpc_b64 s[30:31]
;
; SI-GISEL-LABEL: v_log_v2f16_fast:
@@ -7365,6 +7674,12 @@ define <3 x half> @v_log_v3f16_fast(<3 x half> %in) {
; SI-SDAG-NEXT: v_mul_f32_e32 v0, 0x3f317218, v0
; SI-SDAG-NEXT: v_mul_f32_e32 v1, 0x3f317218, v1
; SI-SDAG-NEXT: v_mul_f32_e32 v2, 0x3f317218, v2
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v0, v0
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v1, v1
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v2, v2
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v0, v0
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v1, v1
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v2, v2
; SI-SDAG-NEXT: s_setpc_b64 s[30:31]
;
; SI-GISEL-LABEL: v_log_v3f16_fast:
@@ -7691,20 +8006,28 @@ define <4 x half> @v_log_v4f16_fast(<4 x half> %in) {
; SI-SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v0, v0
; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v1, v1
-; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v2, v2
; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v3, v3
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v2, v2
; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v0, v0
; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v1, v1
-; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v2, v2
; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v3, v3
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v2, v2
; SI-SDAG-NEXT: v_log_f32_e32 v0, v0
; SI-SDAG-NEXT: v_log_f32_e32 v1, v1
-; SI-SDAG-NEXT: v_log_f32_e32 v2, v2
; SI-SDAG-NEXT: v_log_f32_e32 v3, v3
+; SI-SDAG-NEXT: v_log_f32_e32 v2, v2
; SI-SDAG-NEXT: v_mul_f32_e32 v0, 0x3f317218, v0
; SI-SDAG-NEXT: v_mul_f32_e32 v1, 0x3f317218, v1
-; SI-SDAG-NEXT: v_mul_f32_e32 v2, 0x3f317218, v2
; SI-SDAG-NEXT: v_mul_f32_e32 v3, 0x3f317218, v3
+; SI-SDAG-NEXT: v_mul_f32_e32 v2, 0x3f317218, v2
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v0, v0
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v1, v1
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v2, v2
+; SI-SDAG-NEXT: v_cvt_f16_f32_e32 v3, v3
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v0, v0
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v1, v1
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v2, v2
+; SI-SDAG-NEXT: v_cvt_f32_f16_e32 v3, v3
; SI-SDAG-NEXT: s_setpc_b64 s[30:31]
;
; SI-GISEL-LABEL: v_log_v4f16_fast:
diff --git a/llvm/test/CodeGen/AMDGPU/llvm.log10.ll b/llvm/test/CodeGen/AMDGPU/llvm.log10.ll
index a141bceb3ce86..6dbb5823a2b0f 100644
--- a/llvm/test/CodeGen/AMDGPU/llvm.log10.ll
+++ b/llvm/test/CodeGen/AMDGPU/llvm.log10.ll
@@ -316,6 +316,309 @@ define amdgpu_kernel void @s_log10_f32(ptr addrspace(1) %out, float %in) {
ret void
}
+define amdgpu_kernel void @s_log10_contract_f32(ptr addrspace(1) %out, float %in) {
+; SI-SDAG-LABEL: s_log10_contract_f32:
+; SI-SDAG: ; %bb.0:
+; SI-SDAG-NEXT: s_load_dword s6, s[4:5], 0xb
+; SI-SDAG-NEXT: s_load_dwordx2 s[0:1], s[4:5], 0x9
+; SI-SDAG-NEXT: v_mov_b32_e32 v0, 0x800000
+; SI-SDAG-NEXT: v_mov_b32_e32 v1, 0x411a209b
+; SI-SDAG-NEXT: s_mov_b32 s4, 0x3e9a209a
+; SI-SDAG-NEXT: s_waitcnt lgkmcnt(0)
+; SI-SDAG-NEXT: v_cmp_lt_f32_e32 vcc, s6, v0
+; SI-SDAG-NEXT: s_and_b64 s[2:3], vcc, exec
+; SI-SDAG-NEXT: s_cselect_b32 s2, 32, 0
+; SI-SDAG-NEXT: v_cndm...
[truncated]
|
arsenm
reviewed
Nov 20, 2025
Comment on lines
2769
to
2771
| // Our implementation of LOG is not contract safe, so disable instruction | ||
| // contraction. | ||
| Flags.setAllowContract(false); |
Contributor
There was a problem hiding this comment.
Can you sink this down into the actual implementation, for the sequences where it's a problem
Contributor
Author
There was a problem hiding this comment.
Do you mean remove the flag from the actual MIR instructions that we generate for llvm.log where the flag is problematic, instead of removing it from llvm.log itself?
Contributor
There was a problem hiding this comment.
No, I mean this placement is far removed away from the problematic contraction (which I'm guessing is
and )
Contributor
Author
There was a problem hiding this comment.
Gotcha! I updated it.
arsenm
reviewed
Nov 20, 2025
Comment on lines
2807
to
2808
| // Our implementation of LOG is not contract safe, so disable instruction | ||
| // contraction. |
Contributor
There was a problem hiding this comment.
Comment should explain why it's unsafe, not simply state it's unsafe
Contributor
Author
There was a problem hiding this comment.
Update comment. Also fixed global-isel pass.
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
b-sumner
reviewed
Nov 20, 2025
| SDValue C = DAG.getConstantFP(IsLog10 ? c_log10 : c_log, DL, VT); | ||
| SDValue CC = DAG.getConstantFP(IsLog10 ? cc_log10 : cc_log, DL, VT); | ||
|
|
||
| // Our implementation of LOG is not contract safe because we generate |
There was a problem hiding this comment.
Is it not sufficient to say that contraction here raises the error of the approximation? Is it really necessary to explain that it causes the error of the first product to be incorrectly added in a second time?
There was a problem hiding this comment.
Suggest "generate error-correcting summations" --> "add correction terms"
b-sumner
reviewed
Nov 21, 2025
b-sumner
left a comment
b-sumner
left a comment
There was a problem hiding this comment.
Thanks @adelejjeh. This looks good to me.
Contributor
Author
|
@arsenm are you ok with this PR going in now or are there any remaining concerns that you have? |
arsenm
reviewed
Nov 24, 2025
Comment on lines
2807
to
2809
| // Our implementation of LOG is not contract safe because we add correction | ||
| // terms for which contraction may lead to an increase in the error of the | ||
| // approximation. Disable contraction for the expanded instructions. |
Contributor
There was a problem hiding this comment.
Suggested change
| // Our implementation of LOG is not contract safe because we add correction | |
| // terms for which contraction may lead to an increase in the error of the | |
| // approximation. Disable contraction for the expanded instructions. | |
| // This adds correction terms for which contraction may lead to an increase in | |
| // the error of the approximation, so disable it. |
Contributor
Author
There was a problem hiding this comment.
I update the comments.
adelejjeh
force-pushed
the
log_lowering_nocontract
branch
from
62f8e61 to
38196b7
Compare
adelejjeh
force-pushed
the
log_lowering_nocontract
branch
from
38196b7 to
2693815
Compare
Contributor
Author
|
@arsenm do you have any other concerns about the PR? |
arsenm
approved these changes
Dec 4, 2025
|
@adelejjeh Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem Summary
PyTorch's
test_warp_softmax_64bit_indexingis failing with a numerical precision error wherelog(1.1422761679)computed with 54% higher error than expected (9.042e-09 vs 5.859e-09), causing gradient computations to exceed tolerance thresholds. This precision degradation was reproducible across all AMD GPU architectures (gfx1100, gfx1200, gfx90a, gfx950). I tracked down the problem to the commit 4703f8b (March 6, 2025) which changed HIP math headers to call__builtin_logf()directly instead of__ocml_log_f32():This change exposed a problem in the AMDGCN back-end as described below:
Key Findings
1. Contract flag propagation: When
-ffp-contract=fastis enabled (default for HIP), Clang's CodeGen adds thecontractflag to allCallInstinstructions within the scope ofCGFPOptionsRAII, including calls to LLVM intrinsics likellvm.log.f32.2. Behavior change from OCML to builtin path:
__ocml_log_f32): The preprocessed IR showed the call to the OCML library function had the contract flag, but the OCML implementation internally dropped the contract flag when calling thellvm.log.f32intrinsic.__builtin_logf): The call goes directly tollvm.log.f32intrinsic with the contract flag preserved, causing the backend to apply FMA contraction during polynomial expansion.3. Why contract breaks log: Our AMDGCM target back end implements the natural logarithm by taking the result of the hardware log, then multiplying that by
ln(2), and applying some rounding error correction to that multiplication. This results in something like:With the presence of the
contractflag, the back-end fuses the add (r + Z) with the multiply thinking that it is legal, thus eliminating the intermediate rounding. The error compensation term, which was calculated based on the rounded product, is now being added to the full-precision result from the FMA, leading to incorrect error correction and degraded accuracy. The corresponding contracted operations become the following:Solution and Proposed Fix
Based on our implementation of
llvm.logandllvm.log10, it should be illegal for the back-end to propagate thecontractflag when it is present on the intrinsic call because it uses error-correcting summation. My proposed fix is to modify the instruction selection passes (both global-isel and sdag) to drop thecontractflag when lowering llvm.log. That way, when the instruction selection performs the contraction optimization, it will not fuse the multiply and add.Note: I had originally implemented this fix in the FE by removing the
contractflag when lowering the llvm.log builtin (PR #168770). I have since closed that PR.