Skip to content

[GlobalIsel] Cleanup G_EXTRACT_VECTOR_ELT combines #109047

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

[GlobalIsel] Cleanup G_EXTRACT_VECTOR_ELT combines #109047

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
17e54a6
[GlobalIsel[NFC]] Hygiene
tschuett Sep 17, 2024
a9ee113
address review comments
tschuett 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
6 changes: 4 additions & 2 deletions llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,8 @@ class CombinerHelper {
bool matchExtractVectorElement(MachineInstr &MI, BuildFnTy &MatchInfo);

/// Combine extract vector element with a build vector on the vector register.
bool matchExtractVectorElementWithBuildVector(const MachineOperand &MO,
bool matchExtractVectorElementWithBuildVector(const MachineInstr &MI,
const MachineInstr &MI2,
BuildFnTy &MatchInfo);

/// Combine extract vector element with a build vector trunc on the vector
Expand All @@ -855,7 +856,8 @@ class CombinerHelper {

/// Combine extract vector element with a shuffle vector on the vector
/// register.
bool matchExtractVectorElementWithShuffleVector(const MachineOperand &MO,
bool matchExtractVectorElementWithShuffleVector(const MachineInstr &MI,
const MachineInstr &MI2,
BuildFnTy &MatchInfo);

/// Combine extract vector element with a insert vector element on the vector
Expand Down
137 changes: 15 additions & 122 deletions llvm/include/llvm/Target/GlobalISel/Combine.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1373,110 +1373,23 @@ def extract_vector_element_different_indices : GICombineRule<
[{ return Helper.matchExtractVectorElementWithDifferentIndices(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector2 : GICombineRule<
def extract_vector_element_build_vector : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector3 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector4 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector5 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector6 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector7 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector8 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector9 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e, $f),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector10 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e, $f, $g),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector11 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e, $f, $g, $h),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector12 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e, $f, $g, $h, $i),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector13 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e, $f, $g, $h, $i, $j),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;
(match (G_CONSTANT $idx, $imm),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could it pass imm to avoid the need to call getIConstantFromReg(Extract->getIndexReg(), MRI);, or does that not work?

Copy link
Author

Choose a reason for hiding this comment

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

APInt llvm::getIConstantFromReg(Register Reg, const MachineRegisterInfo &MRI) {
  MachineInstr *Const = MRI.getVRegDef(Reg);
  assert((Const && Const->getOpcode() == TargetOpcode::G_CONSTANT) &&
         "expected a G_CONSTANT on Reg");
  return Const->getOperand(1).getCImm()->getValue();
}

Sure everything is possible, but we would have to play with the low-level Cimm details. I would like to avoid that.

(G_BUILD_VECTOR $src, GIVariadic<>:$unused):$Build,
(G_EXTRACT_VECTOR_ELT $root, $src, $idx):$Extract,
[{ return Helper.matchExtractVectorElementWithBuildVector(*${Extract}, *${Build},
${matchinfo}); }]),
(apply [{ Helper.applyBuildFn(*${Extract}, ${matchinfo}); }])>;

def extract_vector_element_build_vector14 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector15 : GICombineRule<
def extract_vector_element_shuffle_vector : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k, $l),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_build_vector16 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_BUILD_VECTOR $src, $x, $y, $z, $a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k, $l, $m),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithBuildVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;
(match (G_CONSTANT $idx, $imm),
(G_SHUFFLE_VECTOR $src, $src1, $src2, $mask):$Shuffle,
(G_EXTRACT_VECTOR_ELT $root, $src, $idx):$Extract,
[{ return Helper.matchExtractVectorElementWithShuffleVector(*${Extract}, *${Shuffle},
${matchinfo}); }]),
(apply [{ Helper.applyBuildFn(*${Extract}, ${matchinfo}); }])>;

def extract_vector_element_build_vector_trunc2 : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
Expand Down Expand Up @@ -1547,13 +1460,6 @@ def nneg_zext : GICombineRule<
[{ return Helper.matchNonNegZext(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

def extract_vector_element_shuffle_vector : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_SHUFFLE_VECTOR $src, $src1, $src2, $mask),
(G_EXTRACT_VECTOR_ELT $root, $src, $idx),
[{ return Helper.matchExtractVectorElementWithShuffleVector(${root}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

// Combines concat operations
def concat_matchinfo : GIDefMatchData<"SmallVector<Register>">;
def combine_concat_vector : GICombineRule<
Expand Down Expand Up @@ -1647,20 +1553,7 @@ match_extract_of_element,
insert_vector_elt_oob,
extract_vector_element_not_const,
extract_vector_element_different_indices,
extract_vector_element_build_vector2,
extract_vector_element_build_vector3,
extract_vector_element_build_vector4,
extract_vector_element_build_vector5,
extract_vector_element_build_vector7,
extract_vector_element_build_vector8,
extract_vector_element_build_vector9,
extract_vector_element_build_vector10,
extract_vector_element_build_vector11,
extract_vector_element_build_vector12,
extract_vector_element_build_vector13,
extract_vector_element_build_vector14,
extract_vector_element_build_vector15,
extract_vector_element_build_vector16,
extract_vector_element_build_vector,
extract_vector_element_build_vector_trunc2,
extract_vector_element_build_vector_trunc3,
extract_vector_element_build_vector_trunc4,
Expand Down
62 changes: 10 additions & 52 deletions llvm/lib/CodeGen/GlobalISel/CombinerHelperVectorOps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ bool CombinerHelper::matchExtractVectorElementWithDifferentIndices(
}

bool CombinerHelper::matchExtractVectorElementWithBuildVector(
const MachineOperand &MO, BuildFnTy &MatchInfo) {
MachineInstr *Root = getDefIgnoringCopies(MO.getReg(), MRI);
GExtractVectorElement *Extract = cast<GExtractVectorElement>(Root);
const MachineInstr &MI, const MachineInstr &MI2, BuildFnTy &MatchInfo) {
const GExtractVectorElement *Extract = cast<GExtractVectorElement>(&MI);
const GBuildVector *Build = cast<GBuildVector>(&MI2);

//
// %zero:_(s64) = G_CONSTANT i64 0
Expand All @@ -160,23 +160,8 @@ bool CombinerHelper::matchExtractVectorElementWithBuildVector(
// %extract:_(32) = COPY %arg1(s32)
//
//
//
// %bv:_(<2 x s32>) = G_BUILD_VECTOR %arg1(s32), %arg2(s32)
Copy link
Contributor

Choose a reason for hiding this comment

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

Lost comment?

Copy link
Author

Choose a reason for hiding this comment

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

The index is never opaque. It is always a constant due to the pattern.

Copy link
Author

Choose a reason for hiding this comment

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

The case is not valid anymore.

// %extract:_(s32) = G_EXTRACT_VECTOR_ELT %bv(<2 x s32>), %opaque(s64)
//
// -->
//
// %bv:_(<2 x s32>) = G_BUILD_VECTOR %arg1(s32), %arg2(s32)
// %extract:_(s32) = G_EXTRACT_VECTOR_ELT %bv(<2 x s32>), %opaque(s64)
//

Register Vector = Extract->getVectorReg();

// We expect a buildVector on the Vector register.
GBuildVector *Build = getOpcodeDef<GBuildVector>(Vector, MRI);
if (!Build)
return false;

LLT VectorTy = MRI.getType(Vector);

// There is a one-use check. There are more combines on build vectors.
Expand All @@ -185,22 +170,15 @@ bool CombinerHelper::matchExtractVectorElementWithBuildVector(
!getTargetLowering().aggressivelyPreferBuildVectorSources(Ty))
return false;

Register Index = Extract->getIndexReg();

// If the Index is constant, then we can extract the element from the given
// offset.
std::optional<ValueAndVReg> MaybeIndex =
getIConstantVRegValWithLookThrough(Index, MRI);
if (!MaybeIndex)
return false;
APInt Index = getIConstantFromReg(Extract->getIndexReg(), MRI);

// We now know that there is a buildVector def'd on the Vector register and
// the index is const. The combine will succeed.

Register Dst = Extract->getReg(0);

MatchInfo = [=](MachineIRBuilder &B) {
B.buildCopy(Dst, Build->getSourceReg(MaybeIndex->Value.getZExtValue()));
B.buildCopy(Dst, Build->getSourceReg(Index.getZExtValue()));
};

return true;
Expand Down Expand Up @@ -274,9 +252,9 @@ bool CombinerHelper::matchExtractVectorElementWithBuildVectorTrunc(
}

bool CombinerHelper::matchExtractVectorElementWithShuffleVector(
const MachineOperand &MO, BuildFnTy &MatchInfo) {
GExtractVectorElement *Extract =
cast<GExtractVectorElement>(getDefIgnoringCopies(MO.getReg(), MRI));
const MachineInstr &MI, const MachineInstr &MI2, BuildFnTy &MatchInfo) {
const GExtractVectorElement *Extract = cast<GExtractVectorElement>(&MI);
const GShuffleVector *Shuffle = cast<GShuffleVector>(&MI2);

//
// %zero:_(s64) = G_CONSTANT i64 0
Expand All @@ -302,32 +280,12 @@ bool CombinerHelper::matchExtractVectorElementWithShuffleVector(
// %extract:_(s32) = G_IMPLICIT_DEF
//
//
//
//
//
// %sv:_(<4 x s32>) = G_SHUFFLE_SHUFFLE %arg1(<4 x s32>), %arg2(<4 x s32>),
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this comment was lost

Copy link
Author

Choose a reason for hiding this comment

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

Same here.

Copy link
Author

Choose a reason for hiding this comment

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

The case is not valid anymore.

// shufflemask(0, 0, 0, -1)
// %extract:_(s32) = G_EXTRACT_VECTOR_ELT %sv(<4 x s32>), %opaque(s64)
//
// -->
//
// %sv:_(<4 x s32>) = G_SHUFFLE_SHUFFLE %arg1(<4 x s32>), %arg2(<4 x s32>),
// shufflemask(0, 0, 0, -1)
// %extract:_(s32) = G_EXTRACT_VECTOR_ELT %sv(<4 x s32>), %opaque(s64)
//

// We try to get the value of the Index register.
std::optional<ValueAndVReg> MaybeIndex =
getIConstantVRegValWithLookThrough(Extract->getIndexReg(), MRI);
if (!MaybeIndex)
return false;

GShuffleVector *Shuffle =
cast<GShuffleVector>(getDefIgnoringCopies(Extract->getVectorReg(), MRI));
APInt Index = getIConstantFromReg(Extract->getIndexReg(), MRI);

ArrayRef<int> Mask = Shuffle->getMask();

unsigned Offset = MaybeIndex->Value.getZExtValue();
unsigned Offset = Index.getZExtValue();
int SrcIdx = Mask[Offset];

LLT Src1Type = MRI.getType(Shuffle->getSrc1Reg());
Expand Down
27 changes: 27 additions & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/combine-extract-vec-elt.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,3 +634,30 @@ body: |
RET_ReallyLR implicit $x0
...
---
name: extract_from_build_vector_const_huge
alignment: 4
liveins:
- { reg: '$x0' }
- { reg: '$x1' }
frameInfo:
maxAlignment: 1
body: |
bb.1:
liveins: $x0, $x1
; CHECK-LABEL: name: extract_from_build_vector_const_huge
; CHECK: liveins: $x0, $x1
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: %arg1:_(s64) = COPY $x0
; CHECK-NEXT: $x0 = COPY %arg1(s64)
; CHECK-NEXT: RET_ReallyLR implicit $x0
%vec:_(<2 x s64>) = COPY $q0
%idx:_(s64) = G_CONSTANT i64 0
%arg1:_(s64) = COPY $x0
%arg2:_(s64) = COPY $x1
%bv:_(<18 x s64>) = G_BUILD_VECTOR %arg1(s64), %arg2(s64), %arg1(s64), %arg2(s64), %arg1(s64), %arg2(s64), %arg1(s64), %arg2(s64), %arg1(s64), %arg2(s64), %arg1(s64), %arg2(s64), %arg1(s64), %arg2(s64), %arg1(s64), %arg2(s64), %arg1(s64), %arg2(s64)
Copy link
Author

Choose a reason for hiding this comment

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

The existing patterns covered up to 16. This should have been a negative test.

%extract:_(s64) = G_EXTRACT_VECTOR_ELT %bv(<18 x s64>), %idx(s64)
$x0 = COPY %extract(s64)
RET_ReallyLR implicit $x0

...
---
Loading