Skip to content

[Mips][GISel] Fix a couple issues with passing f64 in 32-bit GPRs. #69131

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 1 commit into from
Oct 25, 2023

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Oct 15, 2023

MipsIncomingValueHandler::assignCustomValue should return 1 instead of 2. The return value is the number of additional ArgLocs being consumed. It's assumed that at least 1 is consumed.

Correct the LocVT used for the spill when there are no registers left. It should be f64 instead of i32. This allows a workaround to be removed in the SelectionDAG path.

MipsIncomingValueHandler::assignCustomValue should return 1 instead
of 2. The return value is the number of additional ArgLocs being
consumed. It's assumed that at leat 1 is consumed.

Correct the LocVT used for the spill when there are no registers left.
It should be f64 instead of i32. This allows a workaround to be
removed in the SelectionDAG path.
@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2023

@llvm/pr-subscribers-llvm-globalisel

Author: Craig Topper (topperc)

Changes

MipsIncomingValueHandler::assignCustomValue should return 1 instead of 2. The return value is the number of additional ArgLocs being consumed. It's assumed that at least 1 is consumed.

Correct the LocVT used for the spill when there are no registers left. It should be f64 instead of i32. This allows a workaround to be removed in the SelectionDAG path.


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

3 Files Affected:

  • (modified) llvm/lib/Target/Mips/MipsCallLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/Mips/MipsISelLowering.cpp (+2-11)
  • (modified) llvm/test/CodeGen/Mips/GlobalISel/irtranslator/float_args.ll (+38)
diff --git a/llvm/lib/Target/Mips/MipsCallLowering.cpp b/llvm/lib/Target/Mips/MipsCallLowering.cpp
index 4d6ca5ac2bcc6b3..1448e7fee0b1d38 100644
--- a/llvm/lib/Target/Mips/MipsCallLowering.cpp
+++ b/llvm/lib/Target/Mips/MipsCallLowering.cpp
@@ -185,7 +185,7 @@ MipsIncomingValueHandler::assignCustomValue(CallLowering::ArgInfo &Arg,
 
   markPhysRegUsed(VALo.getLocReg());
   markPhysRegUsed(VAHi.getLocReg());
-  return 2;
+  return 1;
 }
 
 namespace {
diff --git a/llvm/lib/Target/Mips/MipsISelLowering.cpp b/llvm/lib/Target/Mips/MipsISelLowering.cpp
index 6ff829261f5c262..061d035b7e246c7 100644
--- a/llvm/lib/Target/Mips/MipsISelLowering.cpp
+++ b/llvm/lib/Target/Mips/MipsISelLowering.cpp
@@ -2948,8 +2948,6 @@ static bool CC_MipsO32(unsigned ValNo, MVT ValVT, MVT LocVT,
       Reg = State.AllocateReg(IntRegs);
     LocVT = MVT::i32;
   } else if (ValVT == MVT::f64 && AllocateFloatsInIntReg) {
-    LocVT = MVT::i32;
-
     // Allocate int register and shadow next int register. If first
     // available register is Mips::A1 or Mips::A3, shadow it too.
     Reg = State.AllocateReg(IntRegs);
@@ -2957,6 +2955,8 @@ static bool CC_MipsO32(unsigned ValNo, MVT ValVT, MVT LocVT,
       Reg = State.AllocateReg(IntRegs);
 
     if (Reg) {
+      LocVT = MVT::i32;
+
       State.addLoc(
           CCValAssign::getCustomReg(ValNo, ValVT, Reg, LocVT, LocInfo));
       MCRegister HiReg = State.AllocateReg(IntRegs);
@@ -3723,15 +3723,6 @@ SDValue MipsTargetLowering::LowerFormalArguments(
 
       assert(!VA.needsCustom() && "unexpected custom memory argument");
 
-      if (ABI.IsO32()) {
-        // We ought to be able to use LocVT directly but O32 sets it to i32
-        // when allocating floating point values to integer registers.
-        // This shouldn't influence how we load the value into registers unless
-        // we are targeting softfloat.
-        if (VA.getValVT().isFloatingPoint() && !Subtarget.useSoftFloat())
-          LocVT = VA.getValVT();
-      }
-
       // Only arguments pased on the stack should make it here. 
       assert(VA.isMemLoc());
 
diff --git a/llvm/test/CodeGen/Mips/GlobalISel/irtranslator/float_args.ll b/llvm/test/CodeGen/Mips/GlobalISel/irtranslator/float_args.ll
index ad617288278309f..89367ddf9741858 100644
--- a/llvm/test/CodeGen/Mips/GlobalISel/irtranslator/float_args.ll
+++ b/llvm/test/CodeGen/Mips/GlobalISel/irtranslator/float_args.ll
@@ -12,6 +12,7 @@ define float @float_in_fpr(float %a, float %b) {
   ; FP32-NEXT:   [[COPY1:%[0-9]+]]:_(s32) = COPY $f14
   ; FP32-NEXT:   $f0 = COPY [[COPY1]](s32)
   ; FP32-NEXT:   RetRA implicit $f0
+  ;
   ; FP64-LABEL: name: float_in_fpr
   ; FP64: bb.1.entry:
   ; FP64-NEXT:   liveins: $f12, $f14
@@ -33,6 +34,7 @@ define double @double_in_fpr(double %a, double %b) {
   ; FP32-NEXT:   [[COPY1:%[0-9]+]]:_(s64) = COPY $d7
   ; FP32-NEXT:   $d0 = COPY [[COPY1]](s64)
   ; FP32-NEXT:   RetRA implicit $d0
+  ;
   ; FP64-LABEL: name: double_in_fpr
   ; FP64: bb.1.entry:
   ; FP64-NEXT:   liveins: $d12_64, $d14_64
@@ -54,6 +56,7 @@ define float @float_in_gpr(i32 %a, float %b) {
   ; FP32-NEXT:   [[COPY1:%[0-9]+]]:_(s32) = COPY $a1
   ; FP32-NEXT:   $f0 = COPY [[COPY1]](s32)
   ; FP32-NEXT:   RetRA implicit $f0
+  ;
   ; FP64-LABEL: name: float_in_gpr
   ; FP64: bb.1.entry:
   ; FP64-NEXT:   liveins: $a0, $a1
@@ -77,6 +80,7 @@ define double @double_in_gpr(i32 %a, double %b) {
   ; FP32-NEXT:   [[MV:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[COPY1]](s32), [[COPY2]](s32)
   ; FP32-NEXT:   $d0 = COPY [[MV]](s64)
   ; FP32-NEXT:   RetRA implicit $d0
+  ;
   ; FP64-LABEL: name: double_in_gpr
   ; FP64: bb.1.entry:
   ; FP64-NEXT:   liveins: $a0, $a2, $a3
@@ -91,6 +95,36 @@ entry:
   ret double %b
 }
 
+define double @two_double_in_gpr(i32 %a, double %b, double %c) {
+  ; FP32-LABEL: name: two_double_in_gpr
+  ; FP32: bb.1.entry:
+  ; FP32-NEXT:   liveins: $a0, $a2, $a3
+  ; FP32-NEXT: {{  $}}
+  ; FP32-NEXT:   [[COPY:%[0-9]+]]:_(s32) = COPY $a0
+  ; FP32-NEXT:   [[COPY1:%[0-9]+]]:_(s32) = COPY $a2
+  ; FP32-NEXT:   [[COPY2:%[0-9]+]]:_(s32) = COPY $a3
+  ; FP32-NEXT:   [[MV:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[COPY1]](s32), [[COPY2]](s32)
+  ; FP32-NEXT:   [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %fixed-stack.0
+  ; FP32-NEXT:   [[LOAD:%[0-9]+]]:_(s64) = G_LOAD [[FRAME_INDEX]](p0) :: (load (s64) from %fixed-stack.0)
+  ; FP32-NEXT:   $d0 = COPY [[LOAD]](s64)
+  ; FP32-NEXT:   RetRA implicit $d0
+  ;
+  ; FP64-LABEL: name: two_double_in_gpr
+  ; FP64: bb.1.entry:
+  ; FP64-NEXT:   liveins: $a0, $a2, $a3
+  ; FP64-NEXT: {{  $}}
+  ; FP64-NEXT:   [[COPY:%[0-9]+]]:_(s32) = COPY $a0
+  ; FP64-NEXT:   [[COPY1:%[0-9]+]]:_(s32) = COPY $a2
+  ; FP64-NEXT:   [[COPY2:%[0-9]+]]:_(s32) = COPY $a3
+  ; FP64-NEXT:   [[MV:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[COPY1]](s32), [[COPY2]](s32)
+  ; FP64-NEXT:   [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %fixed-stack.0
+  ; FP64-NEXT:   [[LOAD:%[0-9]+]]:_(s64) = G_LOAD [[FRAME_INDEX]](p0) :: (load (s64) from %fixed-stack.0)
+  ; FP64-NEXT:   $d0_64 = COPY [[LOAD]](s64)
+  ; FP64-NEXT:   RetRA implicit $d0_64
+entry:
+  ret double %c
+}
+
 define float @call_float_in_fpr(float %a, float %b) {
   ; FP32-LABEL: name: call_float_in_fpr
   ; FP32: bb.1.entry:
@@ -106,6 +140,7 @@ define float @call_float_in_fpr(float %a, float %b) {
   ; FP32-NEXT:   ADJCALLSTACKUP 16, 0, implicit-def $sp, implicit $sp
   ; FP32-NEXT:   $f0 = COPY [[COPY2]](s32)
   ; FP32-NEXT:   RetRA implicit $f0
+  ;
   ; FP64-LABEL: name: call_float_in_fpr
   ; FP64: bb.1.entry:
   ; FP64-NEXT:   liveins: $f12, $f14
@@ -140,6 +175,7 @@ define double @call_double_in_fpr(double %a, double %b) {
   ; FP32-NEXT:   ADJCALLSTACKUP 16, 0, implicit-def $sp, implicit $sp
   ; FP32-NEXT:   $d0 = COPY [[COPY2]](s64)
   ; FP32-NEXT:   RetRA implicit $d0
+  ;
   ; FP64-LABEL: name: call_double_in_fpr
   ; FP64: bb.1.entry:
   ; FP64-NEXT:   liveins: $d12_64, $d14_64
@@ -174,6 +210,7 @@ define float @call_float_in_gpr(i32 %a, float %b) {
   ; FP32-NEXT:   ADJCALLSTACKUP 16, 0, implicit-def $sp, implicit $sp
   ; FP32-NEXT:   $f0 = COPY [[COPY2]](s32)
   ; FP32-NEXT:   RetRA implicit $f0
+  ;
   ; FP64-LABEL: name: call_float_in_gpr
   ; FP64: bb.1.entry:
   ; FP64-NEXT:   liveins: $a0, $a1
@@ -213,6 +250,7 @@ define double @call_double_in_gpr(i32 %a, double %b) {
   ; FP32-NEXT:   ADJCALLSTACKUP 16, 0, implicit-def $sp, implicit $sp
   ; FP32-NEXT:   $d0 = COPY [[COPY3]](s64)
   ; FP32-NEXT:   RetRA implicit $d0
+  ;
   ; FP64-LABEL: name: call_double_in_gpr
   ; FP64: bb.1.entry:
   ; FP64-NEXT:   liveins: $a0, $a2, $a3

@topperc topperc merged commit 7fde4ff into llvm:main Oct 25, 2023
@topperc topperc deleted the pr/mips-f64-fix branch October 25, 2023 18:28
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.

3 participants