-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[SPARC] Implement L and H inline asm argument modifiers #87259
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
Conversation
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.
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-backend-sparc Author: Koakuma (koachan) ChangesThis 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:
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
+}
|
There was a problem hiding this 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.
That would be a good idea as there is no reference of a bug report. |
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 " |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
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)
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
This adds support for using the L and H argument modifiers for twinword operands in inline asm code, such as:
This is needed by the Linux kernel.