Skip to content

Conversation

HolyMolyCowMan
Copy link
Contributor

When widening a G_FCONSTANT it is converted to a G_CONSTANT to avoid loss in accuracy (see #56454). This means that some folds such as G_FPEXT(G_FCONSTANT) fail to work when the scalar has been widened.

This PR legalizes s16s by default in line with how s16 G_CONSTANTs are treated.

@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2025

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-aarch64

Author: Ryan Cowan (HolyMolyCowMan)

Changes

When widening a G_FCONSTANT it is converted to a G_CONSTANT to avoid loss in accuracy (see #56454). This means that some folds such as G_FPEXT(G_FCONSTANT) fail to work when the scalar has been widened.

This PR legalizes s16s by default in line with how s16 G_CONSTANTs are treated.


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

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp (+2-2)
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
index 7ee54c5932b15..1593f32d1fc6c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -678,8 +678,8 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
       .widenScalarToNextPow2(0)
       .clampScalar(0, s8, s64);
   getActionDefinitionsBuilder(G_FCONSTANT)
-      .legalFor({s32, s64, s128})
-      .legalFor(HasFP16, {s16})
+      // Always legalize s16 to prevent G_FCONSTANT being widened to G_CONSTANT
+      .legalFor({s16, s32, s64, s128})
       .clampScalar(0, MinFPScalar, s128);
 
   // FIXME: fix moreElementsToNextPow2

@davemgreen davemgreen requested a review from aemerson September 29, 2025 18:19
@aemerson
Copy link
Contributor

Yeah this needs to address the regbank regressions.

@HolyMolyCowMan
Copy link
Contributor Author

By moving the matchFConstantToConstant to the post-legalizer lowering, patches such as #160902 can more easily fold operations involving G_FCONSTANTs, and still make use of zero registers when applicable.

Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

LGTM

@davemgreen davemgreen merged commit cac5bfa into llvm:main Oct 8, 2025
9 checks passed
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.

4 participants