[X86] Lower mathlib call ldexp into scalef when avx512 is enabled by huhu233 · Pull Request #69710 · llvm/llvm-project

huhu233 · 2023-10-20T11:23:16Z

No description provided.

huhu233 · 2023-10-20T11:24:42Z

Similar to #67552

RKSimon · 2023-10-24T15:26:34Z

llvm/test/CodeGen/X86/fold-int-pow2-with-fmul-or-fdiv.ll

Missing avx512 checks?

Supplement the check, thanks!

RKSimon · 2023-10-24T15:27:36Z

llvm/lib/Target/X86/X86ISelLowering.h

no need to create a method - just make it static inside X86ISelLowering.cpp

Done, thanks!

RKSimon · 2023-10-24T15:28:30Z

llvm/lib/Target/X86/X86ISelLowering.cpp

We'd be much better off adding a X86ISD::SCALEF nodetype rather than individual intrinsic lowering cases.

Hi, @RKSimon, sorry for the delay in replying your comments, I have made some changes to the patch, please have a check, thanks very much!

huhu233 · 2023-12-13T12:10:52Z

Rebase the branch
Use X86ISD::SCALEFS instead of specific intrinsics
Transfrom the function into static version
update the test file

RKSimon · 2023-12-13T15:54:47Z

llvm/lib/Target/X86/X86ISelLowering.cpp

Use the single line style like (most of) the previous cases.

It seems that coding style check discourages this change ...

RKSimon · 2023-12-13T15:59:41Z

llvm/lib/Target/X86/X86ISelLowering.cpp

Why do you need to vectorize to use SCALEFS? I thought SCALEFS was for scalar types and SCALEF was for vector types? (So it should be possible to add vector support here as well).

RKSimon · 2023-12-13T16:00:58Z

llvm/lib/Target/X86/X86ISelLowering.cpp

Can we use vscalefph if we have AVX512FP16?

Hi, there may be risk of truncation, as the EXP operand of FLDEXP types i32, e.g., @llvm.ldexp.f16.i32(half, i32) ->@llvm.x86.avx512fp16.mask.scalef.sh(<8 x half>, <8 x half>, <8 x half>. I didn't know how to handle the issue elegantly, so I made an extension here.

Good point! LangRef doesn't give an example f16 case. Should we define it as @llvm.ldexp.f16.i16(half, i16)? i32 is a too large range to be useful for FP16.

Could we use value tracking to check the bounds of the EXP operand?

Value tracking seems to make things very complicated.

I'd settle for a TODO comment for now

github-actions · 2023-12-14T14:50:12Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff e34c35a21ccc215ce507a1e19b4ff2a1ce9906f3 4ac55f1aecf19b4e2ce981ad757c79b0daf46e5e -- llvm/lib/Target/X86/X86ISelLowering.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 19510bbba0..7ac39cea21 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -2406,11 +2406,11 @@ X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
   }
 
   if (Subtarget.hasAVX512()) {
-    for (MVT VT : { MVT::f16, MVT::f32, MVT::f64, MVT::v4f32, MVT::v2f64 })
+    for (MVT VT : {MVT::f16, MVT::f32, MVT::f64, MVT::v4f32, MVT::v2f64})
       setOperationAction(ISD::FLDEXP, VT, Custom);
 
     if (Subtarget.hasVLX())
-      for (MVT VT : { MVT::v8f32, MVT::v4f64, MVT::v16f32, MVT::v8f64 })
+      for (MVT VT : {MVT::v8f32, MVT::v4f64, MVT::v16f32, MVT::v8f64})
         setOperationAction(ISD::FLDEXP, VT, Custom);
   }
 
@@ -32039,7 +32039,8 @@ SDValue X86TargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
   case ISD::ADDRSPACECAST:      return LowerADDRSPACECAST(Op, DAG);
   case X86ISD::CVTPS2PH:        return LowerCVTPS2PH(Op, DAG);
   case ISD::PREFETCH:           return LowerPREFETCH(Op, Subtarget, DAG);
-  case ISD::FLDEXP:             return LowerFLDEXP(Op, Subtarget, DAG);
+  case ISD::FLDEXP:
+    return LowerFLDEXP(Op, Subtarget, DAG);
   }
 }

huhu233 · 2023-12-14T14:58:08Z

Fix coding style issues
Support vector versions of ldexp, e.g., @llvm.ldexp.v8f32.v8i32 -- +avx512vl , @llvm.ldexp.v4f32.v4i32 -- +avx512f or +avx512vl, etc.

huhu233 · 2023-12-15T11:16:53Z

add TODO for avx512fp16

RKSimon · 2023-12-28T15:04:33Z

llvm/lib/Target/X86/X86ISelLowering.cpp

  }

+  if (Subtarget.hasAVX512()) {
+    for (MVT VT : { MVT::f16, MVT::f32, MVT::f64, MVT::v4f32, MVT::v2f64 })


Are we sure this is right? I'd expect MVT::v4f32, MVT::v2f64, MVT::v8f32, MVT::v4f64 to be the VLX cases.

arsenm · 2024-01-17T09:00:36Z

llvm/test/CodeGen/X86/call-ldexp.ll

+; AVX512VL-NEXT:    vpinsrw $0, %eax, %xmm0, %xmm0
+; AVX512VL-NEXT:    retq
+entry:
+  %r = tail call fast half @llvm.ldexp.f16.i32(half %x, i32 %exp)


Drop unnecessary fast flags?

RKSimon · 2024-04-03T10:59:15Z

@huhu233 reverse-ping

Andarwinux · 2025-10-29T03:04:31Z

Any progress on this PR?

RKSimon · 2025-10-30T11:10:16Z

Closing this abandoned PR and adding a issue (#165694) to track future work - I'll add the test coverage from this patch to trunk so we have something to work from.

Pulled out of the abandoned patch llvm#69710 to act as a baseline for llvm#165694

Pulled out of the abandoned patch #69710 to act as a baseline for #165694

Pulled out of the abandoned patch llvm#69710 to act as a baseline for llvm#165694

llvmbot added the backend:X86 label Oct 20, 2023

huhu233 requested review from RKSimon, arsenm and nikic October 24, 2023 13:39

RKSimon reviewed Oct 24, 2023

View reviewed changes

huhu233 force-pushed the feature-ldexp-x86 branch from 7355b45 to 85f59ff Compare December 13, 2023 11:56

huhu233 requested a review from RKSimon December 13, 2023 12:14

RKSimon reviewed Dec 13, 2023

View reviewed changes

RKSimon requested a review from phoebewang December 13, 2023 16:02

nikic removed their request for review December 14, 2023 08:31

huhu233 force-pushed the feature-ldexp-x86 branch from 85f59ff to c365199 Compare December 14, 2023 14:47

[X86] Lower mathlib call ldexp into scalef when avx512 is enabled

4ac55f1

huhu233 force-pushed the feature-ldexp-x86 branch from c365199 to 4ac55f1 Compare December 15, 2023 11:14

huhu233 requested a review from RKSimon December 27, 2023 06:25

RKSimon reviewed Dec 28, 2023

View reviewed changes

arsenm reviewed Jan 17, 2024

View reviewed changes

RKSimon mentioned this pull request Oct 30, 2025

[X86] Lower mathlib call ldexp into scalef when avx512 is enabled #165694

Closed

RKSimon closed this Oct 30, 2025

RKSimon added a commit to RKSimon/llvm-project that referenced this pull request Oct 30, 2025

[X86] Add ldexp test coverage for avx512 targets

634b0f4

Pulled out of the abandoned patch llvm#69710 to act as a baseline for llvm#165694

RKSimon mentioned this pull request Oct 30, 2025

[X86] Add ldexp test coverage for avx512 targets #165698

Merged

RKSimon added a commit that referenced this pull request Oct 30, 2025

[X86] Add ldexp test coverage for avx512 targets (#165698)

3b30010

Pulled out of the abandoned patch #69710 to act as a baseline for #165694

aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025

[X86] Add ldexp test coverage for avx512 targets (llvm#165698)

5df3f68

Pulled out of the abandoned patch llvm#69710 to act as a baseline for llvm#165694

luciechoi pushed a commit to luciechoi/llvm-project that referenced this pull request Nov 1, 2025

[X86] Add ldexp test coverage for avx512 targets (llvm#165698)

23666da

Pulled out of the abandoned patch llvm#69710 to act as a baseline for llvm#165694

DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025

[X86] Add ldexp test coverage for avx512 targets (llvm#165698)

23af488

Pulled out of the abandoned patch llvm#69710 to act as a baseline for llvm#165694

Conversation

huhu233 commented Oct 20, 2023

Uh oh!

huhu233 commented Oct 20, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huhu233 commented Dec 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huhu233 Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huhu233 commented Dec 14, 2023

Uh oh!

huhu233 commented Dec 15, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon commented Apr 3, 2024

Uh oh!

Andarwinux commented Oct 29, 2025

Uh oh!

RKSimon commented Oct 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

huhu233 Dec 14, 2023 •

edited

Loading

github-actions bot commented Dec 14, 2023 •

edited

Loading