Skip to content

[MachineVerifier] Improve checks for G_INSERT_SUBVECTOR. #109209

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 2 commits into from
Sep 19, 2024
Merged

[MachineVerifier] Improve checks for G_INSERT_SUBVECTOR. #109209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
236aad5
[MachineVerifier] Improve checks for G_INSERT_SUBVECTOR.
topperc Sep 18, 2024
7ef3859
fixup! clang-format
topperc Sep 18, 2024
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
32 changes: 21 additions & 11 deletions llvm/lib/CodeGen/MachineVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1710,41 +1710,51 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
}

LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
LLT Src0Ty = MRI->getType(Src0Op.getReg());
LLT Src1Ty = MRI->getType(Src1Op.getReg());

if (!DstTy.isVector()) {
report("Destination type must be a vector", MI);
break;
}

if (!Src0Ty.isVector()) {
report("First source must be a vector", MI);
if (!Src1Ty.isVector()) {
report("Second source must be a vector", MI);
break;
}

if (!Src1Ty.isVector()) {
report("Second source must be a vector", MI);
if (DstTy.getElementType() != Src1Ty.getElementType()) {
report("Element type of vectors must be the same", MI);
break;
}

if (DstTy != Src0Ty) {
report("Destination type must match the first source vector type", MI);
if (Src1Ty.isScalable() != DstTy.isScalable()) {
report("Vector types must both be fixed or both be scalable", MI);
break;
}

if (Src0Ty.getElementType() != Src1Ty.getElementType()) {
report("Element type of source vectors must be the same", MI);
if (ElementCount::isKnownGT(Src1Ty.getElementCount(),
DstTy.getElementCount())) {
report("Second source must be smaller than destination vector", MI);
break;
}

if (IndexOp.getImm() != 0 &&
IndexOp.getImm() % Src1Ty.getElementCount().getKnownMinValue() != 0) {
uint64_t Idx = IndexOp.getImm();
uint64_t Src1MinLen = Src1Ty.getElementCount().getKnownMinValue();
if (IndexOp.getImm() % Src1MinLen != 0) {
report("Index must be a multiple of the second source vector's "
"minimum vector length",
MI);
break;
}

uint64_t DstMinLen = DstTy.getElementCount().getKnownMinValue();
if (Idx >= DstMinLen || Idx + Src1MinLen > DstMinLen) {
report("Subvector type and index must not cause insert to overrun the "
"vector being inserted into",
MI);
break;
}

break;
}
case TargetOpcode::G_EXTRACT_SUBVECTOR: {
Expand Down
28 changes: 24 additions & 4 deletions llvm/test/MachineVerifier/test_g_insert_subvector.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ body: |
%1:_(<vscale x 2 x s32>) = G_IMPLICIT_DEF
%2:_(<vscale x 1 x s32>) = G_IMPLICIT_DEF

; CHECK: generic instruction must use register operands
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to say generic instruction must use register operands? looks unrelated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

anything that has a type declared in the .td automatically generates a check that the operand is a register. This handles a significant portion of the generic instructions like G_ADD, G_FADD without writing any custom code.

I was going to remove the error from G_INSERT_SUBVECTOR and G_EXTRACT_SUBVECTOR, but we need the call to isReg() to avoid an assert in getReg() later. The assert will crash the test without letting us get through all the test cases.

; CHECK: G_INSERT_SUBVECTOR first source must be a register
%3:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR 1, %2, 0

; CHECK: generic instruction must use register operands
; CHECK: G_INSERT_SUBVECTOR second source must be a register
%4:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %1, 1, 0

Expand All @@ -23,18 +25,18 @@ body: |
; CHECK: Destination type must be a vector
%6:_(s32) = G_INSERT_SUBVECTOR %1, %2, 0

; CHECK: First source must be a vector
; CHECK: Type mismatch in generic instruction
%7:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %0, %2, 0

; CHECK: Second source must be a vector
%8:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %1, %0, 0

; CHECK: Destination type must match the first source vector type
; CHECK: Type mismatch in generic instruction
%9:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %2, %1, 0

%10:_(<vscale x 1 x s16>) = G_IMPLICIT_DEF

; CHECK: Element type of source vectors must be the same
; CHECK: Element type of vectors must be the same
%11:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %1, %10, 0

%12:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF
Expand All @@ -43,5 +45,23 @@ body: |
%13:_(<vscale x 4 x s32>) = G_INSERT_SUBVECTOR %12, %1, 3

; CHECK: Index must be a multiple of the second source vector's minimum vector length
%13:_(<vscale x 4 x s32>) = G_INSERT_SUBVECTOR %12, %1, 1
%14:_(<vscale x 4 x s32>) = G_INSERT_SUBVECTOR %12, %1, 1

%15:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF

; CHECK: Second source must be smaller than destination vector
%16:_(<vscale x 2 x s32>) = G_INSERT_SUBVECTOR %1, %15, 0

; CHECK: Subvector type and index must not cause insert to overrun the vector being inserted into
%17:_(<vscale x 4 x s32>) = G_INSERT_SUBVECTOR %12, %1, 4

%18:_(<vscale x 3 x s32>) = G_IMPLICIT_DEF

; CHECK: Subvector type and index must not cause insert to overrun the vector being inserted into
%19:_(<vscale x 3 x s32>) = G_INSERT_SUBVECTOR %18, %1, 2

%20:_(<2 x s32>) = G_IMPLICIT_DEF

; CHECK: Vector types must both be fixed or both be scalable
%21:_(<vscale x 1 x s32>) = G_INSERT_SUBVECTOR %12, %20, 2
...
Loading