Skip to content

Conversation

@Amichaxx
Copy link
Contributor

@Amichaxx Amichaxx commented Aug 11, 2025

This pull request improves support for scalar floating-point conversions from integer vectors on AArch64, specifically for the scvtf and ucvtf instructions. It fixes pattern matching so that single-element conversions from vectors now generate the expected scalar instructions and adds a new test to verify correct behavior for extracting a lane from a widened vector.

Pattern matching and code generation improvements:

  • Added new patterns in AArch64InstrInfo.td to correctly match conversions from v2i32 to v1f64 using scvtf and ucvtf, ensuring the scalar instructions (scvtf d0, s0 and ucvtf d0, s0) are generated when extracting a single lane.

Test updates and additions:

  • Updated scvtf_f64i32_simple and ucvtf_f64i32_simple tests in fprcvt-cvtf.ll to reflect the correct generation of scalar instructions, removing previous comments about incorrect codegen and showing the expected output.
  • Added a new test uitofp_sext_v2i32_extract_lane0 to verify correct code generation when extracting a lane from a widened vector and converting to double.

@llvmbot
Copy link
Member

llvmbot commented Aug 11, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Amina Chabane (Amichaxx)

Changes

Improve the pattern matching for scalar floating-point conversion instructions on AArch64, ensuring that more efficient scalar instructions are generated from vector inputs. Added a new pattern to the instruction definitions and updating test cases to reflect the improved code generation.

  • Added a new pattern in 'AArch64InstrFormats.td'
  • Updated 'scvtf_f64i32_simple' test in 'fprcvt-cvtf.ll' to expect the scvtf d0, s0 scalar instruction
  • Removed outdated comment

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

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64InstrFormats.td (+2)
  • (modified) llvm/test/CodeGen/AArch64/fprcvt-cvtf.ll (+1-7)
diff --git a/llvm/lib/Target/AArch64/AArch64InstrFormats.td b/llvm/lib/Target/AArch64/AArch64InstrFormats.td
index ba7cbccc0bcd6..4e2c30a7aa7ee 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrFormats.td
+++ b/llvm/lib/Target/AArch64/AArch64InstrFormats.td
@@ -5520,6 +5520,8 @@ multiclass IntegerToFPSIMDScalar<bits<2> rmode, bits<3> opcode, string asm, SDPa
     let Inst{31} = 1; // 64-bit FPR flag
     let Inst{23-22} = 0b00; // 32-bit FPR flag
   }
+  def : Pat<(v1f64 (extract_subvector (v2f64 (node (v2i64 (sext (v2i32 V64:$Rn))))), (i64 0))),
+        (!cast<Instruction>(NAME # DSr) (EXTRACT_SUBREG V64:$Rn, ssub))>;
 
   def : Pat<(f16 (node (i32 (extractelt (v4i32 V128:$Rn), (i64 0))))),
           (!cast<Instruction>(NAME # HSr) (EXTRACT_SUBREG V128:$Rn, ssub))>;
diff --git a/llvm/test/CodeGen/AArch64/fprcvt-cvtf.ll b/llvm/test/CodeGen/AArch64/fprcvt-cvtf.ll
index 9da6f583cec01..254510abacdde 100644
--- a/llvm/test/CodeGen/AArch64/fprcvt-cvtf.ll
+++ b/llvm/test/CodeGen/AArch64/fprcvt-cvtf.ll
@@ -101,9 +101,7 @@ define double @scvtf_f64i32_neg(<4 x i32> %x) {
 define <1 x double> @scvtf_f64i32_simple(<1 x i32> %x) {
 ; CHECK-LABEL: scvtf_f64i32_simple:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    sshll v0.2d, v0.2s, #0
-; CHECK-NEXT:    scvtf v0.2d, v0.2d
-; CHECK-NEXT:    // kill: def $d0 killed $d0 killed $q0
+; CHECK-NEXT:    scvtf d0, s0
 ; CHECK-NEXT:    ret
 ;
 ; CHECK-NO-FPRCVT-LABEL: scvtf_f64i32_simple:
@@ -202,10 +200,6 @@ define float @scvtf_f32i64_neg(<2 x i64> %x) {
  ret float %conv
 }
 
-; This test does not give the indended result of scvtf s0, d0
-; This is due to the input being loaded as a 2 item vector and
-; therefore using vector inputs that do not match the pattern
-; This test will be fixed in a future revision
 define <1 x float> @scvtf_f32i64_simple(<1 x i64> %x) {
 ; CHECK-LABEL: scvtf_f32i64_simple:
 ; CHECK:       // %bb.0:

@CarolineConcatto CarolineConcatto self-requested a review August 11, 2025 10:26
@Amichaxx Amichaxx force-pushed the vector-pattern-opt branch from 15dd684 to e82a221 Compare August 19, 2025 08:42
Copy link
Contributor

@CarolineConcatto CarolineConcatto left a comment

Choose a reason for hiding this comment

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

Thank you Amina,

Align before merging

@Amichaxx Amichaxx changed the title [AArch64] Correct SCVTF instructions for vector input [AArch64] Correct SCVTF/UCVTF instructions for vector input Aug 21, 2025
Amended patterns to match the signed path (sitofp(sext(...))) to SCVTFDSr and
the unsigned path (uitofp(zext(...))) to UCVTFDSr. Moved to AArch64InstrInfo.td.
Added test in fprcvt-cvtf.ll to verify.
@Amichaxx
Copy link
Contributor Author

Amichaxx commented Sep 8, 2025

@efriedma-quic Thanks for pointing out the errors with the patterns. I've since updated the patterns and added a test which I believe references the issue you pointed out.

- Removed change to check lines of added test uitofp_sext_v2i32_extract_lane0
- Removed irrelevant whitespace
- Reverted to original comment on scvtf_f32i64_neg for future revision
Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

LGTM

@CarolineConcatto CarolineConcatto merged commit 248ad71 into llvm:main Sep 12, 2025
9 checks passed
@Amichaxx
Copy link
Contributor Author

Thank you for reviewing @efriedma-quic and @CarolineConcatto!

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