Skip to content
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

MIPS: Fix asm constraints "f" and "r" for softfloat #79116

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

wzssyqa
Copy link
Contributor

@wzssyqa wzssyqa commented Jan 23, 2024

This include 2 fixes:
1. Disallow 'f' for softfloat.
2. Allow 'r' for softfloat.

Currently, 'f' is accpeted by clang, then LLVM meets an internal error.

'r' is rejected by LLVM by: couldn't allocate input reg for constraint 'r'.

Fixes: #64241, #63632

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 23, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 23, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: YunQiang Su (wzssyqa)

Changes

Currently, clang accpets contraint f for softfloat, then LLVM meet an internal error.

See: #64241


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

2 Files Affected:

  • (modified) clang/lib/Basic/Targets/Mips.h (+3)
  • (modified) clang/test/Driver/mips-float.c (+14)
diff --git a/clang/lib/Basic/Targets/Mips.h b/clang/lib/Basic/Targets/Mips.h
index f46b95abfd75c73..2b8ad6645e605fc 100644
--- a/clang/lib/Basic/Targets/Mips.h
+++ b/clang/lib/Basic/Targets/Mips.h
@@ -238,6 +238,9 @@ class LLVM_LIBRARY_VISIBILITY MipsTargetInfo : public TargetInfo {
     case 'd': // Equivalent to "r" unless generating MIPS16 code.
     case 'y': // Equivalent to "r", backward compatibility only.
     case 'f': // floating-point registers.
+      if (*Name == 'f' && FloatABI == SoftFloat)
+        return false;
+      LLVM_FALLTHROUGH;
     case 'c': // $25 for indirect jumps
     case 'l': // lo register
     case 'x': // hilo register pair
diff --git a/clang/test/Driver/mips-float.c b/clang/test/Driver/mips-float.c
index 2f1b813a153224c..bbf17abfb13839f 100644
--- a/clang/test/Driver/mips-float.c
+++ b/clang/test/Driver/mips-float.c
@@ -102,3 +102,17 @@
 // CHECK-ABI-SOFT-MIPS16: "-target-feature" "+mips16"
 // CHECK-ABI-SOFT-MIPS16: "-msoft-float"
 // CHECK-ABI-SOFT-MIPS16: "-mfloat-abi" "soft"
+
+/// On MIPS, don't accept constraint "f" for soft-float.
+// RUN: not %clang -S %s -o %t.s 2>&1 \
+// RUN:     -target mips-linux-gnu -msoft-float \
+// RUN:     -DSOFT_FLOAT_NO_CONSTRAINT_F \
+// RUN:   | FileCheck --check-prefix=CHECK-SOFTFLOAT-ASM-NO-F %s
+// CHECK-SOFTFLOAT-ASM-NO-F: error: invalid input constraint 'f' in asm
+
+#ifdef SOFT_FLOAT_NO_CONSTRAINT_F
+void read_float(float* p) {
+    float result = *p;
+    __asm__("" ::"f"(result));
+}
+#endif // SOFT_FLOAT_NO_CONSTRAINT_F

@wzssyqa
Copy link
Contributor Author

wzssyqa commented Jan 25, 2024

@MaskRay

@wzssyqa wzssyqa changed the title MIPS/clang: Disallow constraint f for softfloat MIPS/clang: Fix asm constraint for softfloat Feb 2, 2024
@brad0
Copy link
Contributor

brad0 commented Feb 14, 2024

@wzssyqa Ping.

@wzssyqa wzssyqa force-pushed the softfloat-no-f branch 3 times, most recently from 8383948 to 0da6811 Compare February 22, 2024 02:36
@brad0 brad0 requested a review from MaskRay February 22, 2024 04:47
@brad0
Copy link
Contributor

brad0 commented Feb 27, 2024

@MaskRay

define dso_local void @read_double(ptr nocapture noundef readonly %0) local_unnamed_addr #0 {
%2 = load double, ptr %0, align 8
; MIPS32-LABEL: read_double:
; MIPS32: lw $2, 4($4)
Copy link
Member

Choose a reason for hiding this comment

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

Ensure that lw is indented by 2 compared with read_double:

@@ -238,6 +238,9 @@ class LLVM_LIBRARY_VISIBILITY MipsTargetInfo : public TargetInfo {
case 'd': // Equivalent to "r" unless generating MIPS16 code.
case 'y': // Equivalent to "r", backward compatibility only.
case 'f': // floating-point registers.
if (*Name == 'f' && FloatABI == SoftFloat)
Copy link
Member

Choose a reason for hiding this comment

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

It's better to move 'd' and 'y' below. I'll fix it.

wzssyqa and others added 2 commits February 26, 2024 21:45
This include 2 fixes:
	1. Disallow 'f' for softfloat.
	2. Allow 'r' for softfloat.

Currently, 'f' is accpeted by clang, then
LLVM meet an internal error.

'r' is rejected by LLVM by:
  couldn't allocate input reg for constraint 'r'

Fixes: llvm#64241
@MaskRay MaskRay changed the title MIPS/clang: Fix asm constraint for softfloat MIPS: Fix asm constraints "f" and "r" for softfloat Feb 27, 2024
@MaskRay MaskRay merged commit c88beb4 into llvm:main Feb 27, 2024
3 of 4 checks passed
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 27, 2024
This include 2 fixes:
        1. Disallow 'f' for softfloat.
        2. Allow 'r' for softfloat.

Currently, 'f' is accpeted by clang, then LLVM meets an internal error.

'r' is rejected by LLVM by: couldn't allocate input reg for constraint
'r'.

Fixes: llvm#64241, llvm#63632

---------

Co-authored-by: Fangrui Song <[email protected]>
(cherry picked from commit c88beb4)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 27, 2024
This include 2 fixes:
        1. Disallow 'f' for softfloat.
        2. Allow 'r' for softfloat.

Currently, 'f' is accpeted by clang, then LLVM meets an internal error.

'r' is rejected by LLVM by: couldn't allocate input reg for constraint
'r'.

Fixes: llvm#64241, llvm#63632

---------

Co-authored-by: Fangrui Song <[email protected]>
(cherry picked from commit c88beb4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang MIPS: float register constraints for inline asm deviate from gcc
4 participants