Skip to content

[llvm-exegesis] Adding in llvm-exegesis for Aarch64 for handling FPR8/16/32 and FPCR setReg warning #130595

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.

Sign up for GitHub

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
Mar 11, 2025
Merged

[llvm-exegesis] Adding in llvm-exegesis for Aarch64 for handling FPR8/16/32 and FPCR setReg warning #130595

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f3de22
[llvm-exegesis] Adding support for FPR8/16/32 and FPCR for setReg war…
lakshayk-nv Mar 10, 2025
62e8e36
Modified: clang formatter changes
lakshayk-nv Mar 10, 2025
b684c97
[llvm-exegesis] Simplifying the FPR8 setReg implementation and corres…
lakshayk-nv Mar 11, 2025
fae356f
[llvm-exegesis] Style nits for testcase.
lakshayk-nv Mar 11, 2025
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
37 changes: 37 additions & 0 deletions llvm/test/tools/llvm-exegesis/AArch64/setReg_init_check.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,40 @@ RUN: FileCheck %s --check-prefix=FPR64-ASM < %t.s
FPR64-ASM: <foo>:
FPR64-ASM: movi d{{[0-9]+}}, #0000000000000000
FPR64-ASM-NEXT: addv h{{[0-9]+}}, v{{[0-9]+}}.4h

## FPR32 Register Class Initialization Testcase
RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --dump-object-to-disk=%d --opcode-name=FABSSr --benchmark-phase=assemble-measured-code 2>&1
RUN: llvm-objdump -d %d > %t.s
RUN: FileCheck %s --check-prefix=FPR32-ASM < %t.s
FPR32-ASM: <foo>:
FPR32-ASM: movi d{{[0-9]+}}, #0000000000000000
FPR32-ASM-NEXT: fabs s{{[0-9]+}}, s{{[0-9]+}}


## FPR16 Register Class Initialization Testcase
RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --dump-object-to-disk=%d --opcode-name=FABSHr --benchmark-phase=assemble-measured-code 2>&1
RUN: llvm-objdump -d %d > %t.s
RUN: FileCheck %s --check-prefix=FPR16-ASM < %t.s
FPR16-ASM: <foo>:
FPR16-ASM: movi d{{[0-9]+}}, #0000000000000000
FPR16-ASM-NEXT: fabs h{{[0-9]+}}, h{{[0-9]+}}

## FPR8 Register Class Initialization Testcase
RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --dump-object-to-disk=%d --opcode-name=SQABSv1i8 --benchmark-phase=assemble-measured-code 2>&1
RUN: llvm-objdump -d %d > %t.s
RUN: FileCheck %s --check-prefix=FPR8-ASM < %t.s
FPR8-ASM: <foo>:
FPR8-ASM: mov w{{[0-9]+}}, #0x0
FPR8-ASM-NEXT: fmov h{{[0-9]+}}, w{{[0-9]+}}
Copy link
Collaborator

@sjoerdmeijer sjoerdmeijer Mar 10, 2025

Choose a reason for hiding this comment

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

You can directly use:

    fmov h, #imm

No need to move 0 to a GPR first and then to a FPR?

Copy link
Contributor Author

@lakshayk-nv lakshayk-nv Mar 10, 2025

Choose a reason for hiding this comment

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

There was no instruction/pseudo-instruction to load for for FPR8 Reg class. Thus, can't move (fmov) directly. So moved Value to GPR and then mov to FPR.

Copy link
Collaborator

@sjoerdmeijer sjoerdmeijer Mar 11, 2025

Choose a reason for hiding this comment

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

Okay, fair enough about movi above, but why are we not also using that here then?

Copy link
Contributor Author

@lakshayk-nv lakshayk-nv Mar 11, 2025

Choose a reason for hiding this comment

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

Yupp, it makes sense to use movi as suggested by @davemgreen too.
Updated code for FPR8, now returning instruction loadFPImmediate(Reg - AArch64::B0 + AArch64::D0, 64, Value) which will generate assembly for movi $reg #0

FPR8-ASM-NEXT: sqabs b{{[0-9]+}}, b{{[0-9]+}}


## FPCR Register Class Initialization Testcase
RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --dump-object-to-disk=%d --opcode-name=BFCVT --benchmark-phase=assemble-measured-code 2>&1
RUN: llvm-objdump -d %d > %t.s
RUN: FileCheck %s --check-prefix=FPCR-ASM < %t.s
FPCR-ASM: <foo>:
FPCR-ASM: movi d{{[0-9]+}}, #0000000000000000
Copy link
Collaborator

@sjoerdmeijer sjoerdmeijer Mar 11, 2025

Choose a reason for hiding this comment

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

Same here.

FPCR-ASM-NEXT: mov x{{[0-9]+}}, #0x0
Copy link
Collaborator

@sjoerdmeijer sjoerdmeijer Mar 10, 2025

Choose a reason for hiding this comment

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

You're using x9 as a "hard-coded" temp register (in the loadFPCRImmediate), so to test that, you'll need to match that here and not any value.

Copy link
Contributor Author

@lakshayk-nv lakshayk-nv Mar 11, 2025

Choose a reason for hiding this comment

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

Done

FPCR-ASM-NEXT: msr FPCR, x{{[0-9]+}}
FPCR-ASM-NEXT: bfcvt h{{[0-9]+}}, s{{[0-9]+}}
60 changes: 52 additions & 8 deletions llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ static unsigned getLoadImmediateOpcode(unsigned RegBitWidth) {
// Generates instruction to load an immediate value into a register.
static MCInst loadImmediate(MCRegister Reg, unsigned RegBitWidth,
const APInt &Value) {
assert (Value.getBitWidth() <= RegBitWidth &&
"Value must fit in the Register");
assert(Value.getBitWidth() <= RegBitWidth &&
"Value must fit in the Register");
return MCInstBuilder(getLoadImmediateOpcode(RegBitWidth))
.addReg(Reg)
.addImm(Value.getZExtValue());
Expand All @@ -53,11 +53,46 @@ static MCInst loadPPRImmediate(MCRegister Reg, unsigned RegBitWidth,
.addImm(31); // All lanes true for 16 bits
}

// Generates instructions to load an immediate value into an FPCR register.
static std::vector<MCInst>
loadFPCRImmediate(MCRegister Reg, unsigned RegBitWidth, const APInt &Value) {
MCRegister TempReg = AArch64::X8;
MCInst LoadImm = MCInstBuilder(AArch64::MOVi64imm).addReg(TempReg).addImm(0);
MCInst MoveToFPCR =
MCInstBuilder(AArch64::MSR).addImm(AArch64SysReg::FPCR).addReg(TempReg);
return {LoadImm, MoveToFPCR};
}

// Generates instructions to load an immediate value into an FPR8 register.
static std::vector<MCInst>
loadFP8Immediate(MCRegister Reg, unsigned RegBitWidth, const APInt &Value) {
assert(Value.getBitWidth() <= 8 && "Value must fit in 8 bits");

// Use a temporary general-purpose register (W8) to hold the 8-bit value
MCRegister TempReg = AArch64::W8;

// Load the 8-bit value into a general-purpose register (W8)
MCInst LoadImm = MCInstBuilder(AArch64::MOVi32imm)
.addReg(TempReg)
.addImm(Value.getZExtValue());

// Move the value from the general-purpose register to the FPR16 register
// Convert the FPR8 register to an FPR16 register
MCRegister FPR16Reg = Reg + (AArch64::H0 - AArch64::B0);
MCInst MoveToFPR =
MCInstBuilder(AArch64::FMOVWHr).addReg(FPR16Reg).addReg(TempReg);
return {LoadImm, MoveToFPR};
}

// Fetch base-instruction to load an FP immediate value into a register.
static unsigned getLoadFPImmediateOpcode(unsigned RegBitWidth) {
switch (RegBitWidth) {
case 16:
return AArch64::FMOVH0; // FMOVHi;
case 32:
return AArch64::FMOVS0; // FMOVSi;
case 64:
return AArch64::MOVID; //FMOVDi;
return AArch64::MOVID; // FMOVDi;
case 128:
return AArch64::MOVIv2d_ns;
}
Expand All @@ -67,11 +102,12 @@ static unsigned getLoadFPImmediateOpcode(unsigned RegBitWidth) {
// Generates instruction to load an FP immediate value into a register.
static MCInst loadFPImmediate(MCRegister Reg, unsigned RegBitWidth,
const APInt &Value) {
assert(Value.getZExtValue() == 0 &&
"Expected initialisation value 0");
return MCInstBuilder(getLoadFPImmediateOpcode(RegBitWidth))
.addReg(Reg)
.addImm(Value.getZExtValue());
assert(Value.getZExtValue() == 0 && "Expected initialisation value 0");
MCInst Instructions =
MCInstBuilder(getLoadFPImmediateOpcode(RegBitWidth)).addReg(Reg);
if (RegBitWidth >= 64)
Instructions.addOperand(MCOperand::createImm(Value.getZExtValue()));
return Instructions;
}

#include "AArch64GenExegesis.inc"
Expand All @@ -92,12 +128,20 @@ class ExegesisAArch64Target : public ExegesisTarget {
return {loadImmediate(Reg, 64, Value)};
if (AArch64::PPRRegClass.contains(Reg))
return {loadPPRImmediate(Reg, 16, Value)};
if (AArch64::FPR8RegClass.contains(Reg))
return loadFP8Immediate(Reg, 8, Value);
Copy link
Collaborator

@davemgreen davemgreen Mar 10, 2025

Choose a reason for hiding this comment

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

Could this use return {loadFPImmediate(Reg - AArch64::B0 + AArch64::D0, 64, Value.zext(64))}?

Copy link
Contributor Author

@lakshayk-nv lakshayk-nv Mar 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, better to use movi for FPR8 too.
Updated
Thanks for suggestion.

if (AArch64::FPR16RegClass.contains(Reg))
return {loadFPImmediate(Reg, 16, Value)};
if (AArch64::FPR32RegClass.contains(Reg))
return {loadFPImmediate(Reg, 32, Value)};
if (AArch64::FPR64RegClass.contains(Reg))
return {loadFPImmediate(Reg, 64, Value)};
if (AArch64::FPR128RegClass.contains(Reg))
return {loadFPImmediate(Reg, 128, Value)};
if (AArch64::ZPRRegClass.contains(Reg))
return {loadZPRImmediate(Reg, 128, Value)};
if (Reg == AArch64::FPCR)
return {loadFPCRImmediate(Reg, 32, Value)};

errs() << "setRegTo is not implemented, results will be unreliable\n";
return {};
Expand Down