[SelectionDAG] Remove `UnsafeFPMath` in LegalizeDAG #146316

paperchalice · 2025-06-30T08:19:40Z

These global flags hinder further improvements like [RFC] Honor pragmas with -ffp-contract=fast and pass concurrency support. Remove them incrementally.

llvmbot · 2025-07-24T09:38:34Z

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-llvm-selectiondag

Author: None (paperchalice)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/146316.diff

4 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp (+1-1)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+5-1)
(modified) llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll (+3-3)
(modified) llvm/test/CodeGen/ARM/fp16.ll (+2-2)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
index 74172b230361d..b7a96cb2dc826 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
@@ -3853,7 +3853,7 @@ bool SelectionDAGLegalize::ExpandNode(SDNode *Node) {
     break;
   case ISD::FP_TO_FP16:
     LLVM_DEBUG(dbgs() << "Legalizing FP_TO_FP16\n");
-    if (!TLI.useSoftFloat() && TM.Options.UnsafeFPMath) {
+    if (!TLI.useSoftFloat() && Node->getFlags().hasApproximateFuncs()) {
       SDValue Op = Node->getOperand(0);
       MVT SVT = Op.getSimpleValueType();
       if ((SVT == MVT::f64 || SVT == MVT::f80) &&
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 163646513918d..6eca7b73a9d76 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -3923,11 +3923,15 @@ void SelectionDAGBuilder::visitFPTrunc(const User &I) {
   // FPTrunc is never a no-op cast, no need to check
   SDValue N = getValue(I.getOperand(0));
   SDLoc dl = getCurSDLoc();
+  SDNodeFlags Flags;
+  if (auto *TruncInst = dyn_cast<FPMathOperator>(&I))
+    Flags.copyFMF(*TruncInst);
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
   EVT DestVT = TLI.getValueType(DAG.getDataLayout(), I.getType());
   setValue(&I, DAG.getNode(ISD::FP_ROUND, dl, DestVT, N,
                            DAG.getTargetConstant(
-                               0, dl, TLI.getPointerTy(DAG.getDataLayout()))));
+                               0, dl, TLI.getPointerTy(DAG.getDataLayout())),
+                           Flags));
 }
 
 void SelectionDAGBuilder::visitFPExt(const User &I) {
diff --git a/llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll b/llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll
index 0a900f904bec5..89ce0bda41f8e 100644
--- a/llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll
+++ b/llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll
@@ -196,7 +196,7 @@ define amdgpu_kernel void @fptrunc_f32_to_f16(
     ptr addrspace(1) %a) {
 entry:
   %a.val = load float, ptr addrspace(1) %a
-  %r.val = fptrunc float %a.val to half
+  %r.val = fptrunc afn float %a.val to half
   store half %r.val, ptr addrspace(1) %r
   ret void
 }
@@ -401,7 +401,7 @@ define amdgpu_kernel void @fptrunc_f64_to_f16(
     ptr addrspace(1) %a) {
 entry:
   %a.val = load double, ptr addrspace(1) %a
-  %r.val = fptrunc double %a.val to half
+  %r.val = fptrunc afn double %a.val to half
   store half %r.val, ptr addrspace(1) %r
   ret void
 }
@@ -863,7 +863,7 @@ define amdgpu_kernel void @fptrunc_v2f64_to_v2f16(
     ptr addrspace(1) %a) {
 entry:
   %a.val = load <2 x double>, ptr addrspace(1) %a
-  %r.val = fptrunc <2 x double> %a.val to <2 x half>
+  %r.val = fptrunc afn <2 x double> %a.val to <2 x half>
   store <2 x half> %r.val, ptr addrspace(1) %r
   ret void
 }
diff --git a/llvm/test/CodeGen/ARM/fp16.ll b/llvm/test/CodeGen/ARM/fp16.ll
index dc35fa34f42c1..9ff701050ac7e 100644
--- a/llvm/test/CodeGen/ARM/fp16.ll
+++ b/llvm/test/CodeGen/ARM/fp16.ll
@@ -86,8 +86,8 @@ define i16 @test_to_fp16(double %in) {
 
 ; CHECK-FP16-SAFE: bl __aeabi_d2h
 
-; CHECK-FP16-UNSAFE:      vcvt.f32.f64 s0, d0
-; CHECK-FP16-UNSAFE-NEXT: vcvtb.f16.f32 s0, s0
+; CHECK-FP16-UNSAFE:      vmov r0, r1, d0
+; CHECK-FP16-UNSAFE-NEXT: bl __aeabi_d2h
 
 ; CHECK-ARMV8: vcvtb.f16.f64 [[TMP:s[0-9]+]], d0
 ; CHECK-ARMV8: vmov r0, [[TMP]]

llvmbot · 2025-07-24T09:38:35Z

@llvm/pr-subscribers-backend-amdgpu

Author: None (paperchalice)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/146316.diff

4 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp (+1-1)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+5-1)
(modified) llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll (+3-3)
(modified) llvm/test/CodeGen/ARM/fp16.ll (+2-2)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
index 74172b230361d..b7a96cb2dc826 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
@@ -3853,7 +3853,7 @@ bool SelectionDAGLegalize::ExpandNode(SDNode *Node) {
     break;
   case ISD::FP_TO_FP16:
     LLVM_DEBUG(dbgs() << "Legalizing FP_TO_FP16\n");
-    if (!TLI.useSoftFloat() && TM.Options.UnsafeFPMath) {
+    if (!TLI.useSoftFloat() && Node->getFlags().hasApproximateFuncs()) {
       SDValue Op = Node->getOperand(0);
       MVT SVT = Op.getSimpleValueType();
       if ((SVT == MVT::f64 || SVT == MVT::f80) &&
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 163646513918d..6eca7b73a9d76 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -3923,11 +3923,15 @@ void SelectionDAGBuilder::visitFPTrunc(const User &I) {
   // FPTrunc is never a no-op cast, no need to check
   SDValue N = getValue(I.getOperand(0));
   SDLoc dl = getCurSDLoc();
+  SDNodeFlags Flags;
+  if (auto *TruncInst = dyn_cast<FPMathOperator>(&I))
+    Flags.copyFMF(*TruncInst);
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
   EVT DestVT = TLI.getValueType(DAG.getDataLayout(), I.getType());
   setValue(&I, DAG.getNode(ISD::FP_ROUND, dl, DestVT, N,
                            DAG.getTargetConstant(
-                               0, dl, TLI.getPointerTy(DAG.getDataLayout()))));
+                               0, dl, TLI.getPointerTy(DAG.getDataLayout())),
+                           Flags));
 }
 
 void SelectionDAGBuilder::visitFPExt(const User &I) {
diff --git a/llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll b/llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll
index 0a900f904bec5..89ce0bda41f8e 100644
--- a/llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll
+++ b/llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll
@@ -196,7 +196,7 @@ define amdgpu_kernel void @fptrunc_f32_to_f16(
     ptr addrspace(1) %a) {
 entry:
   %a.val = load float, ptr addrspace(1) %a
-  %r.val = fptrunc float %a.val to half
+  %r.val = fptrunc afn float %a.val to half
   store half %r.val, ptr addrspace(1) %r
   ret void
 }
@@ -401,7 +401,7 @@ define amdgpu_kernel void @fptrunc_f64_to_f16(
     ptr addrspace(1) %a) {
 entry:
   %a.val = load double, ptr addrspace(1) %a
-  %r.val = fptrunc double %a.val to half
+  %r.val = fptrunc afn double %a.val to half
   store half %r.val, ptr addrspace(1) %r
   ret void
 }
@@ -863,7 +863,7 @@ define amdgpu_kernel void @fptrunc_v2f64_to_v2f16(
     ptr addrspace(1) %a) {
 entry:
   %a.val = load <2 x double>, ptr addrspace(1) %a
-  %r.val = fptrunc <2 x double> %a.val to <2 x half>
+  %r.val = fptrunc afn <2 x double> %a.val to <2 x half>
   store <2 x half> %r.val, ptr addrspace(1) %r
   ret void
 }
diff --git a/llvm/test/CodeGen/ARM/fp16.ll b/llvm/test/CodeGen/ARM/fp16.ll
index dc35fa34f42c1..9ff701050ac7e 100644
--- a/llvm/test/CodeGen/ARM/fp16.ll
+++ b/llvm/test/CodeGen/ARM/fp16.ll
@@ -86,8 +86,8 @@ define i16 @test_to_fp16(double %in) {
 
 ; CHECK-FP16-SAFE: bl __aeabi_d2h
 
-; CHECK-FP16-UNSAFE:      vcvt.f32.f64 s0, d0
-; CHECK-FP16-UNSAFE-NEXT: vcvtb.f16.f32 s0, s0
+; CHECK-FP16-UNSAFE:      vmov r0, r1, d0
+; CHECK-FP16-UNSAFE-NEXT: bl __aeabi_d2h
 
 ; CHECK-ARMV8: vcvtb.f16.f64 [[TMP:s[0-9]+]], d0
 ; CHECK-ARMV8: vmov r0, [[TMP]]

paperchalice · 2025-07-24T09:40:50Z

Currently I couldn't fix the test CodeGen/ARM/fp16.ll, because llvm.convert.to.fp16.f64 in this test is incompatible with fast math flags.

davemgreen · 2025-07-25T10:01:09Z

Currently I couldn't fix the test CodeGen/ARM/fp16.ll, because llvm.convert.to.fp16.f64 in this test is incompatible with fast math flags.

I believe we do not use them any more because useFP16ConversionIntrinsics==false in clang.

arsenm · 2025-07-25T10:26:53Z

Currently I couldn't fix the test CodeGen/ARM/fp16.ll, because llvm.convert.to.fp16.f64 in this test is incompatible with fast math flags.

Just take the regressions, these intrinsics should be removed from the IR

arsenm · 2025-07-25T10:49:16Z

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

@@ -3853,7 +3853,7 @@ bool SelectionDAGLegalize::ExpandNode(SDNode *Node) {
    break;
  case ISD::FP_TO_FP16:
    LLVM_DEBUG(dbgs() << "Legalizing FP_TO_FP16\n");
-    if (!TLI.useSoftFloat() && TM.Options.UnsafeFPMath) {
+    if (!TLI.useSoftFloat() && Node->getFlags().hasApproximateFuncs()) {


These checks should be swapped

arsenm · 2025-07-26T00:44:50Z

llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll

@@ -196,7 +196,7 @@ define amdgpu_kernel void @fptrunc_f32_to_f16(
    ptr addrspace(1) %a) {
 entry:
  %a.val = load float, ptr addrspace(1) %a
-  %r.val = fptrunc float %a.val to half
+  %r.val = fptrunc afn float %a.val to half


Can you duplicate the functions to have a safe and unsafe version, and remove the -enable-unsafe-fp-math command line arguments

Regenerate selection dag part, global isel part is a work in progress.

This change is not the solution, due to the type of `llvm.convert.to.fp16.f64`.

arsenm · 2025-07-28T09:56:22Z

llvm/test/CodeGen/AMDGPU/fptrunc.f16.ll

+; GFX9-SDAG-NEXT:    s_mov_b32 s1, s9
+; GFX9-SDAG-NEXT:    s_movk_i32 s4, 0x7e00
+; GFX9-SDAG-NEXT:    s_waitcnt vmcnt(0)
+; GFX9-SDAG-NEXT:    v_readfirstlane_b32 s5, v1


We probably should have separate s and v versions but I won't subject you to that

paperchalice force-pushed the dag-legalize branch 2 times, most recently from 1eac26d to c386827 Compare July 24, 2025 09:37

paperchalice marked this pull request as ready for review July 24, 2025 09:38

paperchalice changed the title ~~WIP: [SelectionDAG] Remove UnsafeFPMath in LegalizeDAG~~ [SelectionDAG] Remove UnsafeFPMath in LegalizeDAG Jul 24, 2025

llvmbot added backend:ARM backend:AMDGPU llvm:SelectionDAG SelectionDAGISel as well labels Jul 24, 2025

paperchalice requested review from arsenm, antoniofrighetto, davemgreen and paulwalker-arm July 24, 2025 09:38

arsenm reviewed Jul 25, 2025

View reviewed changes

arsenm reviewed Jul 26, 2025

View reviewed changes

paperchalice force-pushed the dag-legalize branch from 6179283 to c033e05 Compare July 26, 2025 03:29

paperchalice added 5 commits July 26, 2025 13:00

Consider fastmath flags in visitFPTrunc

70851d3

Remove UnsafeFPMath in LegalizeDAG

d0d2cd6

Fix arm test

ad2ac08

This change is not the solution, due to the type of `llvm.convert.to.fp16.f64`.

swap check conditions when expand ISD::FP_TO_FP16

303a487

regenerate fptrunc.f16.ll

0a32e08

paperchalice force-pushed the dag-legalize branch from c033e05 to 0a32e08 Compare July 26, 2025 05:02

paperchalice mentioned this pull request Jul 26, 2025

[GlobalISel] Remove UnsafeFPMath references #146319

Merged

arsenm approved these changes Jul 28, 2025

View reviewed changes

paperchalice merged commit 21836f4 into llvm:main Jul 29, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SelectionDAG] Remove `UnsafeFPMath` in LegalizeDAG #146316

[SelectionDAG] Remove `UnsafeFPMath` in LegalizeDAG #146316

Uh oh!

paperchalice commented Jun 30, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 24, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 24, 2025

Uh oh!

paperchalice commented Jul 24, 2025

Uh oh!

davemgreen commented Jul 25, 2025

Uh oh!

arsenm commented Jul 25, 2025

Uh oh!

arsenm Jul 25, 2025

Uh oh!

arsenm Jul 26, 2025

Uh oh!

paperchalice Jul 26, 2025

Uh oh!

arsenm Jul 28, 2025

Uh oh!

Uh oh!

Uh oh!

[SelectionDAG] Remove UnsafeFPMath in LegalizeDAG #146316

[SelectionDAG] Remove UnsafeFPMath in LegalizeDAG #146316

Uh oh!

Conversation

paperchalice commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 24, 2025

Uh oh!

paperchalice commented Jul 24, 2025

Uh oh!

davemgreen commented Jul 25, 2025

Uh oh!

arsenm commented Jul 25, 2025

Uh oh!

arsenm Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

paperchalice Jul 26, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[SelectionDAG] Remove `UnsafeFPMath` in LegalizeDAG #146316

[SelectionDAG] Remove `UnsafeFPMath` in LegalizeDAG #146316

paperchalice commented Jun 30, 2025 •

edited

Loading

llvmbot commented Jul 24, 2025 •

edited

Loading