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

[ARM] Fix MRC cp10 and cp11 warning #126407

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hstk30-hw
Copy link
Contributor

@hstk30-hw hstk30-hw commented Feb 9, 2025

The MRC format is
MRC GPRwithAPSR:$Rt, p_imm:$cop, imm0_7:$opc1,
c_imm:$CRn, c_imm:$CRm, 0, pred:$p
The $cop is the second operand.
Patch for b7d41a1.

@llvmbot llvmbot added backend:ARM mc Machine (object) code labels Feb 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 9, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-arm

Author: None (hstk30-hw)

Changes

The MRC format is
MRC GPRwithAPSR:$Rt, p_imm:$cop, imm0_7:$opc1,
c_imm:$CRn, c_imm:$CRm, 0, pred:$p
The $cop is the second operand.


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

2 Files Affected:

  • (modified) llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp (+2-2)
  • (modified) llvm/test/MC/ARM/diagnostics.s (+4)
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
index 01a271327049f19..e1af3150a840317 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
@@ -77,8 +77,8 @@ static bool getMCRDeprecationInfo(MCInst &MI, const MCSubtargetInfo &STI,
 static bool getMRCDeprecationInfo(MCInst &MI, const MCSubtargetInfo &STI,
                                   std::string &Info) {
   if (STI.hasFeature(llvm::ARM::HasV7Ops) &&
-      ((MI.getOperand(0).isImm() && MI.getOperand(0).getImm() == 10) ||
-       (MI.getOperand(0).isImm() && MI.getOperand(0).getImm() == 11))) {
+      ((MI.getOperand(1).isImm() && MI.getOperand(1).getImm() == 10) ||
+       (MI.getOperand(1).isImm() && MI.getOperand(1).getImm() == 11))) {
     Info = "since v7, cp10 and cp11 are reserved for advanced SIMD or floating "
            "point instructions";
     return true;
diff --git a/llvm/test/MC/ARM/diagnostics.s b/llvm/test/MC/ARM/diagnostics.s
index fa23a7da1e40489..8f91d820bd80fcd 100644
--- a/llvm/test/MC/ARM/diagnostics.s
+++ b/llvm/test/MC/ARM/diagnostics.s
@@ -173,6 +173,10 @@
         @ p10 and p11 are reserved for NEON
         mcr p10, #2, r5, c1, c1, #4
         mcrr p11, #8, r5, r4, c1
+        mrc p10, #7, r5, c1, c1, #0
+        mrc p11, #7, r5, c1, c1, #0
+@ CHECK-WARN: warning: since v7, cp10 and cp11 are reserved for advanced SIMD or floating point instructions
+@ CHECK-WARN: warning: since v7, cp10 and cp11 are reserved for advanced SIMD or floating point instructions
 @ CHECK-WARN: warning: since v7, cp10 and cp11 are reserved for advanced SIMD or floating point instructions
 @ CHECK-WARN: warning: since v7, cp10 and cp11 are reserved for advanced SIMD or floating point instructions
 

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.

This sounds OK to me, given that the first operand is a reg.

llvm/test/MC/ARM/diagnostics.s Outdated Show resolved Hide resolved
@davemgreen davemgreen requested a review from ostannard February 9, 2025 13:57
The MRC format is
    MRC GPRwithAPSR:$Rt, p_imm:$cop, imm0_7:$opc1,
        c_imm:$CRn, c_imm:$CRm, 0, pred:$p
The $cop is the second operand.
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, other than the question about it applying to V8 too.

mcr p11, #2, r5, c1, c1, #4
mrc p10, #7, r5, c1, c1, #0
mrc p11, #7, r5, c1, c1, #0
@ CHECK-ERROR-V7: warning: since v7, cp10 and cp11 are reserved for advanced SIMD or floating point instructions
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this work with V8 too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In armv8 is an error: invalid operand for instruction

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah OK. Sounds good then. Thanks for checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:ARM mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants