Skip to content

[X86] Add custom operation actions for f16: FABS, FNEG, and FCOPYSIGN #128877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

StarOne01
Copy link
Contributor

@StarOne01 StarOne01 commented Feb 26, 2025

This pull request adds custom handling for several floating-point operations for the f16 type with respect to (#126892)..

Fixes #126892

@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2025

@llvm/pr-subscribers-backend-x86

Author: Prashanth (StarOne01)

Changes

This pull request adds custom handling for several floating-point operations for the f16 type with respect to (#126892)..


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

1 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+3)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index a4357197e2843..5a80754e7a3bb 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -701,6 +701,9 @@ X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
     setOperationAction(ISD::FSUB, MVT::f16, Promote);
     setOperationAction(ISD::FMUL, MVT::f16, Promote);
     setOperationAction(ISD::FDIV, MVT::f16, Promote);
+    setOperationAction(ISD::FABS, MVT::f16, Custom);
+    setOperationAction(ISD::FNEG, MVT::f16, Custom);
+    setOperationAction(ISD::FCOPYSIGN, MVT::f16, Custom);
     setOperationAction(ISD::FP_ROUND, MVT::f16, Custom);
     setOperationAction(ISD::FP_EXTEND, MVT::f32, Custom);
     setOperationAction(ISD::FP_EXTEND, MVT::f64, Custom);

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

You need to runninja check-llvm-codegen-x86 and update any of the test files that have changed, you should be able to use the llvm-project\llvm\utils\update_llc_test_checks.py script

@RKSimon RKSimon requested a review from phoebewang February 26, 2025 14:13
@StarOne01
Copy link
Contributor Author

StarOne01 commented Feb 26, 2025

Just out of curiosity, what happened when i change the default (Promote) to Custom? i understand we override the default, making a custom lowering, that means we already have a implementation of custom lowering somewhere?

@RKSimon
Copy link
Collaborator

RKSimon commented Feb 26, 2025

Just out of curiosity, what happened when i change the default (Promote) to Custom? i understand we override the default, making a custom lowering, that means we already have a implementation of custom lowering somewhere?

Yes, it needs to be set as Custom for a target's LowerOperation to be called, otherwise it will go through the generic legalization routes - take a look at SelectionDAGLegalize::LegalizeOp and how it selects the lower based on the Action.

Copy link
Contributor

@phoebewang phoebewang left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

LGTM - cheers

@RKSimon RKSimon merged commit 4d387c4 into llvm:main Feb 27, 2025
11 checks passed
@StarOne01 StarOne01 deleted the f12_unary branch February 27, 2025 09:39
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 3, 2025
…llvm#128877)

This pull request adds custom handling for several floating-point
operations for the `f16` type with respect to
(llvm#126892)..

Fixes llvm#126892
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[X86] Unnecessary promotion for fneg / fabs half calls
4 participants