-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[AArch64] Remove copy in SVE/SME predicate spill and fill #81716
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
Changes from 13 commits
c0c0609
929b111
514b9b5
264a844
53a3177
6a4502c
808ddbb
ed8519e
6a55451
a229c76
8c5b567
a64ca22
9e59c45
1a398e1
2f6d418
f06c0ce
23b051b
a53aeb4
03aabd2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -276,6 +276,8 @@ class AArch64AsmParser : public MCTargetAsmParser { | |||||
ParseStatus tryParseSVEDataVector(OperandVector &Operands); | ||||||
template <RegKind RK> | ||||||
ParseStatus tryParseSVEPredicateVector(OperandVector &Operands); | ||||||
ParseStatus | ||||||
tryParseSVEPredicateOrPredicateAsCounterVector(OperandVector &Operands); | ||||||
template <RegKind VectorKind> | ||||||
ParseStatus tryParseVectorList(OperandVector &Operands, | ||||||
bool ExpectMatch = false); | ||||||
|
@@ -1241,6 +1243,7 @@ class AArch64Operand : public MCParsedAsmOperand { | |||||
case AArch64::PPR_p8to15RegClassID: | ||||||
case AArch64::PNRRegClassID: | ||||||
case AArch64::PNR_p8to15RegClassID: | ||||||
case AArch64::PPRorPNRRegClassID: | ||||||
RK = RegKind::SVEPredicateAsCounter; | ||||||
break; | ||||||
default: | ||||||
|
@@ -1264,6 +1267,7 @@ class AArch64Operand : public MCParsedAsmOperand { | |||||
case AArch64::PPR_p8to15RegClassID: | ||||||
case AArch64::PNRRegClassID: | ||||||
case AArch64::PNR_p8to15RegClassID: | ||||||
case AArch64::PPRorPNRRegClassID: | ||||||
RK = RegKind::SVEPredicateVector; | ||||||
break; | ||||||
default: | ||||||
|
@@ -1290,6 +1294,20 @@ class AArch64Operand : public MCParsedAsmOperand { | |||||
return DiagnosticPredicateTy::NearMatch; | ||||||
} | ||||||
|
||||||
template <int ElementWidth, unsigned Class> | ||||||
DiagnosticPredicate isSVEPredicateOrPredicateAsCounterRegOfWidth() const { | ||||||
if (Kind != k_Register || (Reg.Kind != RegKind::SVEPredicateAsCounter && | ||||||
Reg.Kind != RegKind::SVEPredicateVector)) | ||||||
return DiagnosticPredicateTy::NoMatch; | ||||||
|
||||||
if ((isSVEPredicateAsCounterReg<Class>() || | ||||||
isSVEPredicateVectorRegOfWidth<ElementWidth, Class>()) && | ||||||
(Reg.ElementWidth == ElementWidth)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: no parenthesis needed in:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
return DiagnosticPredicateTy::Match; | ||||||
|
||||||
return DiagnosticPredicateTy::NearMatch; | ||||||
} | ||||||
|
||||||
template <int ElementWidth, unsigned Class> | ||||||
DiagnosticPredicate isSVEPredicateAsCounterRegOfWidth() const { | ||||||
if (Kind != k_Register || Reg.Kind != RegKind::SVEPredicateAsCounter) | ||||||
|
@@ -1770,6 +1788,15 @@ class AArch64Operand : public MCParsedAsmOperand { | |||||
Inst.addOperand(MCOperand::createReg(AArch64::Z0 + getReg() - Base)); | ||||||
} | ||||||
|
||||||
void addPPRorPNRRegOperands(MCInst &Inst, unsigned N) const { | ||||||
assert(N == 1 && "Invalid number of operands!"); | ||||||
unsigned Reg = getReg(); | ||||||
// Normalise to PPR | ||||||
if (Reg >= AArch64::PN0) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
Reg = Reg - AArch64::PN0 + AArch64::P0; | ||||||
Inst.addOperand(MCOperand::createReg(Reg)); | ||||||
} | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
void addPNRasPPRRegOperands(MCInst &Inst, unsigned N) const { | ||||||
assert(N == 1 && "Invalid number of operands!"); | ||||||
Inst.addOperand( | ||||||
|
@@ -4167,6 +4194,15 @@ ParseStatus AArch64AsmParser::tryParseVectorRegister(MCRegister &Reg, | |||||
return ParseStatus::NoMatch; | ||||||
} | ||||||
|
||||||
ParseStatus AArch64AsmParser::tryParseSVEPredicateOrPredicateAsCounterVector( | ||||||
OperandVector &Operands) { | ||||||
ParseStatus Status = | ||||||
tryParseSVEPredicateVector<RegKind::SVEPredicateAsCounter>(Operands); | ||||||
if (!Status.isSuccess()) | ||||||
Status = tryParseSVEPredicateVector<RegKind::SVEPredicateVector>(Operands); | ||||||
return Status; | ||||||
} | ||||||
|
||||||
/// tryParseSVEPredicateVector - Parse a SVE predicate register operand. | ||||||
template <RegKind RK> | ||||||
ParseStatus | ||||||
|
@@ -6019,6 +6055,7 @@ bool AArch64AsmParser::showMatchError(SMLoc Loc, unsigned ErrCode, | |||||
return Error(Loc, "Invalid restricted vector register, expected z0.d..z15.d"); | ||||||
case Match_InvalidSVEPattern: | ||||||
return Error(Loc, "invalid predicate pattern"); | ||||||
case Match_InvalidSVEPPRorPNRAnyReg: | ||||||
case Match_InvalidSVEPredicateAnyReg: | ||||||
case Match_InvalidSVEPredicateBReg: | ||||||
case Match_InvalidSVEPredicateHReg: | ||||||
|
@@ -6653,6 +6690,7 @@ bool AArch64AsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode, | |||||
case Match_InvalidZPR_4b16: | ||||||
case Match_InvalidZPR_4b32: | ||||||
case Match_InvalidZPR_4b64: | ||||||
case Match_InvalidSVEPPRorPNRAnyReg: | ||||||
case Match_InvalidSVEPredicateAnyReg: | ||||||
case Match_InvalidSVEPattern: | ||||||
case Match_InvalidSVEVecLenSpecifier: | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,6 +143,9 @@ DecodeMatrixTileListRegisterClass(MCInst &Inst, unsigned RegMask, | |
static DecodeStatus DecodePPRRegisterClass(MCInst &Inst, unsigned RegNo, | ||
uint64_t Address, | ||
const MCDisassembler *Decoder); | ||
static DecodeStatus DecodePPRorPNRRegisterClass(MCInst &Inst, unsigned RegNo, | ||
uint64_t Addr, | ||
const MCDisassembler *Decoder); | ||
static DecodeStatus DecodePNRRegisterClass(MCInst &Inst, unsigned RegNo, | ||
uint64_t Address, | ||
const MCDisassembler *Decoder); | ||
|
@@ -741,6 +744,18 @@ static DecodeStatus DecodeMatrixTile(MCInst &Inst, unsigned RegNo, | |
return Success; | ||
} | ||
|
||
static DecodeStatus DecodePPRorPNRRegisterClass(MCInst &Inst, unsigned RegNo, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cleanup suggestion for a separate patch: All these decoder classes looks rather identical, it would be nice to clean this up with something like this:
And then remove the other functions and just specify e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds good to me. We can do it in a separate patch. |
||
uint64_t Addr, | ||
const MCDisassembler *Decoder) { | ||
if (RegNo > 15) | ||
return Fail; | ||
|
||
unsigned Register = | ||
AArch64MCRegisterClasses[AArch64::PPRorPNRRegClassID].getRegister(RegNo); | ||
Inst.addOperand(MCOperand::createReg(Register)); | ||
return Success; | ||
} | ||
|
||
static DecodeStatus DecodePPRRegisterClass(MCInst &Inst, unsigned RegNo, | ||
uint64_t Addr, | ||
const MCDisassembler *Decoder) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6689,7 +6689,7 @@ multiclass sve_mem_z_spill<string asm> { | |
} | ||
|
||
class sve_mem_p_spill<string asm> | ||
: I<(outs), (ins PPRAny:$Pt, GPR64sp:$Rn, simm9:$imm9), | ||
: I<(outs), (ins PPRorPNRAny:$Pt, GPR64sp:$Rn, simm9:$imm9), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This removes the need for having the following InstAliases in AArch64SVEInstrInfo.td:
Can you remove them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
asm, "\t$Pt, [$Rn, $imm9, mul vl]", | ||
"", | ||
[]>, Sched<[]> { | ||
|
@@ -6712,7 +6712,7 @@ multiclass sve_mem_p_spill<string asm> { | |
def NAME : sve_mem_p_spill<asm>; | ||
|
||
def : InstAlias<asm # "\t$Pt, [$Rn]", | ||
(!cast<Instruction>(NAME) PPRAny:$Pt, GPR64sp:$Rn, 0), 1>; | ||
(!cast<Instruction>(NAME) PPRorPNRAny:$Pt, GPR64sp:$Rn, 0), 1>; | ||
} | ||
|
||
//===----------------------------------------------------------------------===// | ||
|
@@ -7858,7 +7858,7 @@ multiclass sve_mem_z_fill<string asm> { | |
} | ||
|
||
class sve_mem_p_fill<string asm> | ||
: I<(outs PPRAny:$Pt), (ins GPR64sp:$Rn, simm9:$imm9), | ||
: I<(outs PPRorPNRAny:$Pt), (ins GPR64sp:$Rn, simm9:$imm9), | ||
asm, "\t$Pt, [$Rn, $imm9, mul vl]", | ||
"", | ||
[]>, Sched<[]> { | ||
|
@@ -7881,7 +7881,7 @@ multiclass sve_mem_p_fill<string asm> { | |
def NAME : sve_mem_p_fill<asm>; | ||
|
||
def : InstAlias<asm # "\t$Pt, [$Rn]", | ||
(!cast<Instruction>(NAME) PPRAny:$Pt, GPR64sp:$Rn, 0), 1>; | ||
(!cast<Instruction>(NAME) PPRorPNRAny:$Pt, GPR64sp:$Rn, 0), 1>; | ||
} | ||
|
||
class sve2_mem_gldnt_vs_base<bits<5> opc, dag iops, string asm, | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -57,11 +57,11 @@ tracksRegLiveness: true | |||||
body: | | ||||||
bb.1: | ||||||
; CHECK-LABEL: name: inlineasm_virt_reg_output | ||||||
; CHECK: INLINEASM &"mov ${0:w}, 7", 0 /* attdialect */, 1310730 /* regdef:PPR2_with_psub_in_PNR_3b_and_PPR2_with_psub1_in_PPR_p8to15 */, def %0 | ||||||
; CHECK: INLINEASM &"mov ${0:w}, 7", 0 /* attdialect */, 1769482 /* regdef:GPR32common */, def %0 | ||||||
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr(s32) = COPY %0 | ||||||
; CHECK-NEXT: $w0 = COPY [[COPY]](s32) | ||||||
; CHECK-NEXT: RET_ReallyLR implicit $w0 | ||||||
INLINEASM &"mov ${0:w}, 7", 0 /* attdialect */, 1310730 /* regdef:GPR32common */, def %0:gpr32common | ||||||
INLINEASM &"mov ${0:w}, 7", 0 /* attdialect */, 1769482 /* regdef:GPR32common */, def %0:gpr32common | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(same in other places) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can only use the regex in a CHECK statement, not in the IR being tested. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I didn't spot the leading In that case, I would suggest not using a regex for these numbers, because they must match the ones from the MIR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I've changed the .mir tests to explicitly match the regclass numbers. |
||||||
%1:_(s32) = COPY %0 | ||||||
$w0 = COPY %1(s32) | ||||||
RET_ReallyLR implicit $w0 | ||||||
|
@@ -75,12 +75,12 @@ tracksRegLiveness: true | |||||
body: | | ||||||
bb.1: | ||||||
; CHECK-LABEL: name: inlineasm_virt_mixed_types | ||||||
; CHECK: INLINEASM &"mov $0, #0; mov $1, #0", 0 /* attdialect */, 1310730 /* regdef:PPR2_with_psub_in_PNR_3b_and_PPR2_with_psub1_in_PPR_p8to15 */, def %0, 2162698 /* regdef:WSeqPairsClass */, def %1 | ||||||
; CHECK: INLINEASM &"mov $0, #0; mov $1, #0", 0 /* attdialect */, 1769482 /* regdef:GPR32common */, def %0, {{[0-9]+}} /* regdef:FPR64 */, def %1 | ||||||
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr(s32) = COPY %0 | ||||||
; CHECK-NEXT: [[COPY1:%[0-9]+]]:fpr(s64) = COPY %1 | ||||||
; CHECK-NEXT: $d0 = COPY [[COPY1]](s64) | ||||||
; CHECK-NEXT: RET_ReallyLR implicit $d0 | ||||||
INLINEASM &"mov $0, #0; mov $1, #0", 0 /* attdialect */, 1310730 /* regdef:GPR32common */, def %0:gpr32common, 2162698 /* regdef:FPR64 */, def %1:fpr64 | ||||||
INLINEASM &"mov $0, #0; mov $1, #0", 0 /* attdialect */, 1769482 /* regdef:GPR32common */, def %0:gpr32common, 2621450 /* regdef:FPR64 */, def %1:fpr64 | ||||||
%3:_(s32) = COPY %0 | ||||||
%4:_(s64) = COPY %1 | ||||||
$d0 = COPY %4(s64) | ||||||
|
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.
Now that you've made the instructions accept both p0-p15 and pn0-pn15, we can remove all the code that tries to handle PNR registers differently from P registers in
storeRegToStackSlot
andloadRegFromStackSlot
.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.
Good idea, done.