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

[SPARC] Implement L and H inline asm argument modifiers #87259

Merged
merged 4 commits into from
Apr 4, 2024

Conversation

koachan
Copy link
Contributor

@koachan koachan commented Apr 1, 2024

This adds support for using the L and H argument modifiers for twinword operands in inline asm code, such as:

%1 = tail call i64 asm sideeffect "rd %pc, ${0:L} ; srlx ${0:L}, 32, ${0:H}", "={o4}"()

This is needed by the Linux kernel.

This adds support for using the L and H argument modifiers for twinword
operands in inline asm code, which is used by the Linux kernel.
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 1, 2024

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-backend-sparc

Author: Koakuma (koachan)

Changes

This adds support for using the L and H argument modifiers for twinword operands in inline asm code, which is used by the Linux kernel.


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

3 Files Affected:

  • (modified) llvm/docs/LangRef.rst (+2)
  • (modified) llvm/lib/Target/Sparc/SparcAsmPrinter.cpp (+42)
  • (modified) llvm/test/CodeGen/SPARC/inlineasm.ll (+9)
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 8bc1cab01bf0a6..3f8f4c01201a6c 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -5557,6 +5557,8 @@ RISC-V:
 
 Sparc:
 
+- ``L``: Print the low-order register of a two-register operand.
+- ``H``: Print the high-order register of a two-register operand.
 - ``r``: No effect.
 
 SystemZ:
diff --git a/llvm/lib/Target/Sparc/SparcAsmPrinter.cpp b/llvm/lib/Target/Sparc/SparcAsmPrinter.cpp
index 215a8ea8319046..35daf84a351239 100644
--- a/llvm/lib/Target/Sparc/SparcAsmPrinter.cpp
+++ b/llvm/lib/Target/Sparc/SparcAsmPrinter.cpp
@@ -434,6 +434,48 @@ bool SparcAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
     default:
       // See if this is a generic print operand
       return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, O);
+    case 'L': // Low order register of a twin word register operand
+    case 'H': // High order register of a twin word register operand
+    {
+      if (OpNo == 0)
+        return true;
+
+      const SparcSubtarget &Subtarget = MF->getSubtarget<SparcSubtarget>();
+      const MachineOperand &MO = MI->getOperand(OpNo);
+      const Register MOReg = MO.getReg();
+
+      Register HiReg, LoReg;
+      if (SP::IntPairRegClass.contains(MOReg)) {
+        // If we're given a register pair, decompose it
+        // to its constituents and use them as-is.
+        const SparcRegisterInfo *RegisterInfo = Subtarget.getRegisterInfo();
+        HiReg = RegisterInfo->getSubReg(MOReg, SP::sub_even);
+        LoReg = RegisterInfo->getSubReg(MOReg, SP::sub_odd);
+      } else {
+        // Otherwise we should be given an even-numbered register,
+        // which will become the Hi part of the pair.
+        HiReg = MOReg;
+        LoReg = MOReg + 1;
+
+        // FIXME this really should not be an assert check, but
+        // I have no good idea on how to raise an error with explainations.
+        assert(((HiReg - SP::G0) % 2 == 0) &&
+               "Hi part of pair should point to an even-numbered register!");
+      }
+
+      Register Reg;
+      switch (ExtraCode[0]) {
+      case 'L':
+        Reg = LoReg;
+        break;
+      case 'H':
+        Reg = HiReg;
+        break;
+      }
+
+      O << '%' << SparcInstPrinter::getRegisterName(Reg);
+      return false;
+    }
     case 'f':
     case 'r':
      break;
diff --git a/llvm/test/CodeGen/SPARC/inlineasm.ll b/llvm/test/CodeGen/SPARC/inlineasm.ll
index ec27598e5e83b7..cfb44817fb54a8 100644
--- a/llvm/test/CodeGen/SPARC/inlineasm.ll
+++ b/llvm/test/CodeGen/SPARC/inlineasm.ll
@@ -143,3 +143,12 @@ entry:
   %1 = call double asm sideeffect "faddd $1, $2, $0", "=f,f,e"(i64 0, i64 0)
   ret void
 }
+
+; CHECK-label:test_twinword
+; CHECK: rd  %asr5, %i1
+; CHECK: srlx %i1, 32, %i0
+
+define i64 @test_twinword(){
+  %1 = tail call i64 asm sideeffect "rd %asr5, ${0:L} \0A\09 srlx ${0:L}, 32, ${0:H}", "=r"()
+  ret i64 %1
+}

llvm/lib/Target/Sparc/SparcAsmPrinter.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Sparc/SparcAsmPrinter.cpp Outdated Show resolved Hide resolved
llvm/test/CodeGen/SPARC/inlineasm.ll Show resolved Hide resolved
llvm/lib/Target/Sparc/SparcAsmPrinter.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

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

LGTM
It might be worth adding an example inline asm to the description.

@brad0
Copy link
Contributor

brad0 commented Apr 4, 2024

LGTM It might be worth adding an example inline asm to the description.

That would be a good idea as there is no reference of a bug report.

@koachan koachan merged commit 697dd93 into llvm:main Apr 4, 2024
5 checks passed
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Apr 4, 2024
This adds support for using the L and H argument modifiers for twinword
operands in inline asm code, such as in:

```
%1 = tail call i64 asm sideeffect "rd %pc, ${0:L} ; srlx ${0:L}, 32, ${0:H}", "={o4}"()
```

This is needed by the Linux kernel.

(cherry picked from commit 697dd93)
@@ -11,3 +11,12 @@ entry:
tail call void asm sideeffect "faddq $0,$1,$2", "{f38},{f0},{f0}"(fp128 0xL0, fp128 0xL0, fp128 0xL0)
ret void
}

; CHECK-label:test_twinword_error
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bad formatting (note capitalisation, spacing and trailing colon for the label):

CHECK-LABEL: test_twinword_error:

Repeated in inlineasm.ll, where you can see the mismatch with the surrounding code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, my bad. Should I make a new commit to fix this now, or should I ask the person at #88248 which seems to be doing other work on the same test?

OutContext.reportError(
Loc, "Hi part of pair should point to an even-numbered register");
OutContext.reportError(
Loc, "(note that in some cases it might be necessary to manually "
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like a cop-out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in using OutContext.reportError to raise the error message?
If so, how should I do it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, the fact that it can just fail arbitrarily rather than the compiler ensuring it's in the right register class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah. In my own tests the compiler does seem to always assign the right register class for the operand (if the register's unspecified); the wording is just me wanting to be somewhat cautious...

Still, it is useful to catch for cases where the programmer is using the wrong register for the operand, like in this test.

tstellar pushed a commit to llvmbot/llvm-project that referenced this pull request Apr 10, 2024
This adds support for using the L and H argument modifiers for twinword
operands in inline asm code, such as in:

```
%1 = tail call i64 asm sideeffect "rd %pc, ${0:L} ; srlx ${0:L}, 32, ${0:H}", "={o4}"()
```

This is needed by the Linux kernel.

(cherry picked from commit 697dd93)
@pointhex pointhex mentioned this pull request May 7, 2024
koachan added a commit that referenced this pull request Jun 19, 2024
Fix style errors accidentally introduced in PRs #87259 and #94245.

Reviewers: rorth, jrtc27, brad0, s-barannikov

Reviewed By: s-barannikov

Pull Request: #96019
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
Fix style errors accidentally introduced in PRs llvm#87259 and llvm#94245.

Reviewers: rorth, jrtc27, brad0, s-barannikov

Reviewed By: s-barannikov

Pull Request: llvm#96019
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.

6 participants