[PowerPC] Respect endianness when bitcasting to fp128

[tgymnich]

fixes #92246

Match the behaviour of bitcast v2i64 (BUILD_PAIR %lo %hi) when encountering bitcast fp128 (BUILD_PAIR %lo $hi).
by inserting a missing swap of the arguments based on endianness.

Current behaviour:

fp128
bitcast fp128 (BUILD_PAIR %lo $hi) => BUILD_FP128 %lo %hi
BUILD_FP128 %lo %hi => MTVSRDD %hi %lo

v2i64
bitcast v2i64 (BUILD_PAIR %lo %hi) => BUILD_VECTOR %hi %lo
BUILD_VECTOR %hi %lo => MTVSRDD %lo %hi

[llvmbot]

@llvm/pr-subscribers-backend-powerpc

Author: Tim Gymnich (tgymnich)

Changes

fixes #92246

Match the behaviour of bitcast v2i64 (BUILD_PAIR %lo %hi) when encountering bitcast fp128 (BUILD_PAIR %lo $hi).
by inserting a missing swap of the arguments based on endianness.

Current behaviour:

fp128
bitcast fp128 (BUILD_PAIR %lo $hi) => BUILD_FP128 %lo %hi
BUILD_FP128 %lo %hi => MTVSRDD %hi %lo

v2i64
bitcast v2i64 (BUILD_PAIR %lo %hi) => BUILD_VECTOR %hi %lo
BUILD_VECTOR %hi %lo => MTVSRDD %lo %hi

---

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

2 Files Affected:

• (modified) llvm/lib/Target/PowerPC/PPCISelLowering.cpp (+9-5)
• (modified) llvm/test/CodeGen/PowerPC/f128-aggregates.ll (+6-6)

diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index 12e33ddb8eb53..62fd7a2ca1597 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -9346,15 +9346,19 @@ SDValue PPCTargetLowering::LowerBITCAST(SDValue Op, SelectionDAG &DAG) const {
 
   SDLoc dl(Op);
   SDValue Op0 = Op->getOperand(0);
+  
+  SDValue Lo = Op0.getOperand(0);
+  SDValue Hi = Op0.getOperand(1);
 
   if ((Op.getValueType() != MVT::f128) ||
-      (Op0.getOpcode() != ISD::BUILD_PAIR) ||
-      (Op0.getOperand(0).getValueType() != MVT::i64) ||
-      (Op0.getOperand(1).getValueType() != MVT::i64) || !Subtarget.isPPC64())
+      (Op0.getOpcode() != ISD::BUILD_PAIR) || (Lo.getValueType() != MVT::i64) ||
+      (Hi.getValueType() != MVT::i64) || !Subtarget.isPPC64())
     return SDValue();
 
-  return DAG.getNode(PPCISD::BUILD_FP128, dl, MVT::f128, Op0.getOperand(0),
-                     Op0.getOperand(1));
+  if (!Subtarget.isLittleEndian())
+    std::swap(Lo, Hi);
+
+  return DAG.getNode(PPCISD::BUILD_FP128, dl, MVT::f128, Lo, Hi);
 }
 
 static const SDValue *getNormalLoadInput(const SDValue &Op, bool &IsPermuted) {
diff --git a/llvm/test/CodeGen/PowerPC/f128-aggregates.ll b/llvm/test/CodeGen/PowerPC/f128-aggregates.ll
index b3d2457d31eeb..4be855e30ea1d 100644
--- a/llvm/test/CodeGen/PowerPC/f128-aggregates.ll
+++ b/llvm/test/CodeGen/PowerPC/f128-aggregates.ll
@@ -283,7 +283,7 @@ define fp128 @testMixedAggregate([3 x i128] %a.coerce) {
 ;
 ; CHECK-BE-LABEL: testMixedAggregate:
 ; CHECK-BE:       # %bb.0: # %entry
-; CHECK-BE-NEXT:    mtvsrdd v2, r8, r7
+; CHECK-BE-NEXT:    mtvsrdd v2, r7, r8
 ; CHECK-BE-NEXT:    blr
 ;
 ; CHECK-P8-LABEL: testMixedAggregate:
@@ -310,7 +310,7 @@ define fp128 @testMixedAggregate_02([4 x i128] %a.coerce) {
 ;
 ; CHECK-BE-LABEL: testMixedAggregate_02:
 ; CHECK-BE:       # %bb.0: # %entry
-; CHECK-BE-NEXT:    mtvsrdd v2, r6, r5
+; CHECK-BE-NEXT:    mtvsrdd v2, r5, r6
 ; CHECK-BE-NEXT:    blr
 ;
 ; CHECK-P8-LABEL: testMixedAggregate_02:
@@ -344,7 +344,7 @@ define fp128 @testMixedAggregate_03([4 x i128] %sa.coerce) {
 ; CHECK-BE-LABEL: testMixedAggregate_03:
 ; CHECK-BE:       # %bb.0: # %entry
 ; CHECK-BE-NEXT:    mtvsrwa v2, r4
-; CHECK-BE-NEXT:    mtvsrdd v3, r6, r5
+; CHECK-BE-NEXT:    mtvsrdd v3, r5, r6
 ; CHECK-BE-NEXT:    xscvsdqp v2, v2
 ; CHECK-BE-NEXT:    xsaddqp v2, v3, v2
 ; CHECK-BE-NEXT:    mtvsrd v3, r9
@@ -467,7 +467,7 @@ define fp128 @testUnion_01([1 x i128] %a.coerce) {
 ;
 ; CHECK-BE-LABEL: testUnion_01:
 ; CHECK-BE:       # %bb.0: # %entry
-; CHECK-BE-NEXT:    mtvsrdd v2, r4, r3
+; CHECK-BE-NEXT:    mtvsrdd v2, r3, r4
 ; CHECK-BE-NEXT:    blr
 ;
 ; CHECK-P8-LABEL: testUnion_01:
@@ -494,7 +494,7 @@ define fp128 @testUnion_02([1 x i128] %a.coerce) {
 ;
 ; CHECK-BE-LABEL: testUnion_02:
 ; CHECK-BE:       # %bb.0: # %entry
-; CHECK-BE-NEXT:    mtvsrdd v2, r4, r3
+; CHECK-BE-NEXT:    mtvsrdd v2, r3, r4
 ; CHECK-BE-NEXT:    blr
 ;
 ; CHECK-P8-LABEL: testUnion_02:
@@ -521,7 +521,7 @@ define fp128 @testUnion_03([4 x i128] %a.coerce) {
 ;
 ; CHECK-BE-LABEL: testUnion_03:
 ; CHECK-BE:       # %bb.0: # %entry
-; CHECK-BE-NEXT:    mtvsrdd v2, r8, r7
+; CHECK-BE-NEXT:    mtvsrdd v2, r7, r8
 ; CHECK-BE-NEXT:    blr
 ;
 ; CHECK-P8-LABEL: testUnion_03:

[tgross35]

If this fix is accepted, could it be considered for 18.x?

[chenzheng1030]

If this fix is accepted, could it be considered for 18.x?

https://discourse.llvm.org/t/18-1-8-released/79725

"There are no more planned 18.1.x releases, but there could be one if a critical issue is found. The release/19.x branch will be created on July 23, 2024."

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[chenzheng1030]

I am sorry I lost track for this PR. Will check on this soon.

[chenzheng1030]

LGTM. Thanks very much for fixing this.

For an i128 value, its higher 64 bit and its lower 64 bit are passed with different GPRs on 64-bit BE/LE, to make the bit order be same in the final VSR (what bitcast mean), the swap based on LE code generation looks correct to me.

[chenzheng1030]

Another evidence is: GCC also generates the changed instructions. See https://godbolt.org/z/MT8T5bcf9

[tgymnich]

Thank you for your review @chenzheng1030. Since I don't have commit access feel free to merge this anytime.

[tgross35]

Since this is a bugfix, could it be cherry picked to 19.x? (currently between rc3 and final per https://discourse.llvm.org/t/llvm-19-release-schedule-and-planning/79828)

[chenzheng1030]

Make sense. Let me do the cherry-pick.

[chenzheng1030]

/cherry-pick 408d82d

[tgross35]

Great, thank you!

[llvmbot]

/pull-request #105623

[syzaara]

This PR exposed an assertion failure, which is being address with this PR: #108062