Skip to content

[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

Merged
merged 4 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions llvm/docs/LangRef.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
44 changes: 44 additions & 0 deletions llvm/lib/Target/Sparc/SparcAsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,50 @@ 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
{
const SparcSubtarget &Subtarget = MF->getSubtarget<SparcSubtarget>();
const MachineOperand &MO = MI->getOperand(OpNo);
const SparcRegisterInfo *RegisterInfo = Subtarget.getRegisterInfo();
Register MOReg = MO.getReg();

Register HiReg, LoReg;
if (!SP::IntPairRegClass.contains(MOReg)) {
// If we aren't given a register pair already, find out which pair it
// belongs to. Note that here, the specified register operand, which
// refers to the high part of the twinword, needs to be an even-numbered
// register.
MOReg = RegisterInfo->getMatchingSuperReg(MOReg, SP::sub_even,
&SP::IntPairRegClass);
if (!MOReg) {
SMLoc Loc;
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.

"bind the input/output registers instead of relying on "
"automatic allocation)");
return true;
}
}

HiReg = RegisterInfo->getSubReg(MOReg, SP::sub_even);
LoReg = RegisterInfo->getSubReg(MOReg, SP::sub_odd);

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;
Expand Down
9 changes: 9 additions & 0 deletions llvm/test/CodeGen/SPARC/inlineasm-bad.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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?

; CHECK: error: Hi part of pair should point to an even-numbered register
; CHECK: error: (note that in some cases it might be necessary to manually bind the input/output registers instead of relying on automatic allocation)

define i64 @test_twinword_error(){
%1 = tail call i64 asm sideeffect "rd %asr5, ${0:L} \0A\09 srlx ${0:L}, 32, ${0:H}", "={i1}"()
ret i64 %1
}
9 changes: 9 additions & 0 deletions llvm/test/CodeGen/SPARC/inlineasm.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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}", "={i0}"()
ret i64 %1
}