Skip to content

[GlobalIsel] Canonicalize G_ICMP #108755

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 3 commits into from
Sep 16, 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
6 changes: 6 additions & 0 deletions llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
#include "llvm/CodeGen/GlobalISel/Utils.h"
#include "llvm/CodeGen/Register.h"
#include "llvm/CodeGenTypes/LowLevelType.h"
#include "llvm/IR/InstrTypes.h"
Expand Down Expand Up @@ -909,6 +910,8 @@ class CombinerHelper {
bool matchCastOfBuildVector(const MachineInstr &CastMI,
const MachineInstr &BVMI, BuildFnTy &MatchInfo);

bool matchCanonicalizeICmp(const MachineInstr &MI, BuildFnTy &MatchInfo);

private:
/// Checks for legality of an indexed variant of \p LdSt.
bool isIndexedLoadStoreLegal(GLoadStore &LdSt) const;
Expand Down Expand Up @@ -1023,6 +1026,9 @@ class CombinerHelper {
bool tryFoldLogicOfFCmps(GLogicalBinOp *Logic, BuildFnTy &MatchInfo);

bool isCastFree(unsigned Opcode, LLT ToTy, LLT FromTy) const;

bool constantFoldICmp(const GICmp &ICmp, const GIConstant &LHSCst,
const GIConstant &RHSCst, BuildFnTy &MatchInfo);
};
} // namespace llvm

Expand Down
10 changes: 10 additions & 0 deletions llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,16 @@ class GExtOrTruncOp : public GCastOp {
};
};

/// Represents a splat vector.
class GSplatVector : public GenericMachineInstr {
public:
Register getScalarReg() const { return getOperand(1).getReg(); }

static bool classof(const MachineInstr *MI) {
return MI->getOpcode() == TargetOpcode::G_SPLAT_VECTOR;
};
};

} // namespace llvm

#endif // LLVM_CODEGEN_GLOBALISEL_GENERICMACHINEINSTRS_H
39 changes: 39 additions & 0 deletions llvm/include/llvm/CodeGen/GlobalISel/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -593,5 +593,44 @@ bool isGuaranteedNotToBeUndef(Register Reg, const MachineRegisterInfo &MRI,
/// estimate of the type.
Type *getTypeForLLT(LLT Ty, LLVMContext &C);

/// An integer-like constant.
Copy link
Contributor

Choose a reason for hiding this comment

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

This class could do with more documentation about its intended purpose, in what situation it's supposed to be used.

///
/// It abstracts over scalar, fixed-length vectors, and scalable vectors.
/// In the common case, it provides a common API and feels like an APInt,
/// while still providing low-level access.
/// It can be used for constant-folding.
///
/// bool isZero()
/// abstracts over the kind.
///
/// switch(const.getKind())
/// {
/// }
/// provides low-level access.
class GIConstant {
public:
enum class GIConstantKind { Scalar, FixedVector, ScalableVector };

private:
GIConstantKind Kind;
SmallVector<APInt> Values;
APInt Value;

public:
GIConstant(ArrayRef<APInt> Values)
: Kind(GIConstantKind::FixedVector), Values(Values) {};
GIConstant(const APInt &Value, GIConstantKind Kind)
: Kind(Kind), Value(Value) {};

/// Returns the kind of of this constant, e.g, Scalar.
GIConstantKind getKind() const { return Kind; }

/// Returns the value, if this constant is a scalar.
APInt getScalarValue() const;

static std::optional<GIConstant> getConstant(Register Const,
const MachineRegisterInfo &MRI);
};

} // End namespace llvm.
#endif
25 changes: 18 additions & 7 deletions llvm/include/llvm/Target/GlobalISel/Combine.td
Original file line number Diff line number Diff line change
Expand Up @@ -1007,9 +1007,6 @@ def double_icmp_zero_or_combine: GICombineRule<
(G_ICMP $root, $p, $ordst, 0))
>;

def double_icmp_zero_and_or_combine : GICombineGroup<[double_icmp_zero_and_combine,
double_icmp_zero_or_combine]>;

def and_or_disjoint_mask : GICombineRule<
(defs root:$root, build_fn_matchinfo:$info),
(match (wip_match_opcode G_AND):$root,
Expand Down Expand Up @@ -1918,6 +1915,20 @@ def cast_combines: GICombineGroup<[
integer_of_truncate
]>;

def canonicalize_icmp : GICombineRule<
(defs root:$root, build_fn_matchinfo:$matchinfo),
(match (G_ICMP $root, $pred, $lhs, $rhs):$cmp,
[{ return Helper.matchCanonicalizeICmp(*${cmp}, ${matchinfo}); }]),
(apply [{ Helper.applyBuildFn(*${cmp}, ${matchinfo}); }])>;

def icmp_combines: GICombineGroup<[
canonicalize_icmp,
icmp_to_true_false_known_bits,
icmp_to_lhs_known_bits,
double_icmp_zero_and_combine,
double_icmp_zero_or_combine,
redundant_binop_in_equality
]>;

// FIXME: These should use the custom predicate feature once it lands.
def undef_combines : GICombineGroup<[undef_to_fp_zero, undef_to_int_zero,
Expand Down Expand Up @@ -1951,7 +1962,7 @@ def const_combines : GICombineGroup<[constant_fold_fp_ops, const_ptradd_to_i2p,

def known_bits_simplifications : GICombineGroup<[
redundant_and, redundant_sext_inreg, redundant_or, urem_pow2_to_mask,
zext_trunc_fold, icmp_to_true_false_known_bits, icmp_to_lhs_known_bits,
zext_trunc_fold,
sext_inreg_to_zext_inreg]>;

def width_reduction_combines : GICombineGroup<[reduce_shl_of_extend,
Expand Down Expand Up @@ -1984,7 +1995,7 @@ def all_combines : GICombineGroup<[integer_reassoc_combines, trivial_combines,
combine_extracted_vector_load,
undef_combines, identity_combines, phi_combines,
simplify_add_to_sub, hoist_logic_op_with_same_opcode_hands, shifts_too_big,
reassocs, ptr_add_immed_chain,
reassocs, ptr_add_immed_chain, icmp_combines,
shl_ashr_to_sext_inreg, sext_inreg_of_load,
width_reduction_combines, select_combines,
known_bits_simplifications,
Expand All @@ -1998,9 +2009,9 @@ def all_combines : GICombineGroup<[integer_reassoc_combines, trivial_combines,
constant_fold_cast_op, fabs_fneg_fold,
intdiv_combines, mulh_combines, redundant_neg_operands,
and_or_disjoint_mask, fma_combines, fold_binop_into_select,
sub_add_reg, select_to_minmax, redundant_binop_in_equality,
sub_add_reg, select_to_minmax,
fsub_to_fneg, commute_constant_to_rhs, match_ands, match_ors,
combine_concat_vector, double_icmp_zero_and_or_combine, match_addos,
combine_concat_vector, match_addos,
sext_trunc, zext_trunc, prefer_sign_combines, combine_shuffle_concat]>;

// A combine group used to for prelegalizer combiners at -O0. The combines in
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ add_llvm_component_library(LLVMGlobalISel
Combiner.cpp
CombinerHelper.cpp
CombinerHelperCasts.cpp
CombinerHelperCompares.cpp
CombinerHelperVectorOps.cpp
GIMatchTableExecutor.cpp
GISelChangeObserver.cpp
Expand Down
86 changes: 86 additions & 0 deletions llvm/lib/CodeGen/GlobalISel/CombinerHelperCompares.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
//===- CombinerHelperCompares.cpp------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// This file implements CombinerHelper for G_ICMP.
//
//===----------------------------------------------------------------------===//
#include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
#include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
#include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
#include "llvm/CodeGen/GlobalISel/Utils.h"
#include "llvm/CodeGen/LowLevelTypeUtils.h"
#include "llvm/CodeGen/MachineInstr.h"
#include "llvm/CodeGen/MachineOperand.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/TargetOpcodes.h"
#include "llvm/IR/Instructions.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
#include <cstdlib>

#define DEBUG_TYPE "gi-combiner"

using namespace llvm;

bool CombinerHelper::constantFoldICmp(const GICmp &ICmp,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not put this in the builder like other constant folds?

Copy link
Author

Choose a reason for hiding this comment

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

The builder only folds icmps that it builds. This PR adds one combine that matches G_ICMP. While running the combiner, we could find G_ICMPs where the parameters are constants due to combining. The builder would never build this G_ICMP again. In DAG terms, this is visitICmp.

Copy link
Author

Choose a reason for hiding this comment

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

We already have ConstantFoldICmp, but I needed this on top of GIConstants.

llvm::ConstantFoldICmp(unsigned Pred, const Register Op1, const Register Op2,

This used by the builder.

const GIConstant &LHSCst,
const GIConstant &RHSCst,
BuildFnTy &MatchInfo) {
if (LHSCst.getKind() != GIConstant::GIConstantKind::Scalar)
return false;

Register Dst = ICmp.getReg(0);
LLT DstTy = MRI.getType(Dst);

if (!isConstantLegalOrBeforeLegalizer(DstTy))
return false;

CmpInst::Predicate Pred = ICmp.getCond();
APInt LHS = LHSCst.getScalarValue();
APInt RHS = RHSCst.getScalarValue();

bool Result = ICmpInst::compare(LHS, RHS, Pred);

MatchInfo = [=](MachineIRBuilder &B) {
if (Result)
B.buildConstant(Dst, getICmpTrueVal(getTargetLowering(),
/*IsVector=*/DstTy.isVector(),
/*IsFP=*/false));
else
B.buildConstant(Dst, 0);
};

return true;
}

bool CombinerHelper::matchCanonicalizeICmp(const MachineInstr &MI,
BuildFnTy &MatchInfo) {
const GICmp *Cmp = cast<GICmp>(&MI);

Register Dst = Cmp->getReg(0);
Register LHS = Cmp->getLHSReg();
Register RHS = Cmp->getRHSReg();

CmpInst::Predicate Pred = Cmp->getCond();
assert(CmpInst::isIntPredicate(Pred) && "Not an integer compare!");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also handle fcmp, you don't need to know what the constant kind or value is, just that it's a constant

Copy link
Author

Choose a reason for hiding this comment

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

That would be a different PR. This PR has one pattern that matches G_ICMP. We would need a different pattern for G_FCMP. This PR introduces GIConstant, which is a wrapper around a vector of APInt. For G_FCMP, we would need to introduce GFConstant, which is a wrapper around a vector of APFloat.

This PR canonicalizes and constant folds icmps.

Copy link
Author

Choose a reason for hiding this comment

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

This is a split out from #105991. It only handles icmps. We could add the same for fcmps, but the market share is probably lower without having data.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah we can always generalize this later.

if (auto CLHS = GIConstant::getConstant(LHS, MRI)) {
if (auto CRHS = GIConstant::getConstant(RHS, MRI))
return constantFoldICmp(*Cmp, *CLHS, *CRHS, MatchInfo);

// If we have a constant, make sure it is on the RHS.
std::swap(LHS, RHS);
Pred = CmpInst::getSwappedPredicate(Pred);

MatchInfo = [=](MachineIRBuilder &B) { B.buildICmp(Pred, Dst, LHS, RHS); };
Copy link
Contributor

Choose a reason for hiding this comment

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

This could also mutate the instruction in place without creating a new one

Copy link
Author

Choose a reason for hiding this comment

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

I never do that!

Copy link
Contributor

Choose a reason for hiding this comment

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

I never do that!

That's not a real answer to a review suggestion. If it's easy to mutate in place we should generally try to do that first. The idea is that it's cheaper to do that than creating a new MachineInstr and the operands. For niche optimizations it's not a big deal, but it's a nice to have.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed! But I don't like this low-level manipulation of instructions. It is error prone. I prefer the build and erase pattern.

return true;
}

return false;
}
40 changes: 40 additions & 0 deletions llvm/lib/CodeGen/GlobalISel/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1968,3 +1968,43 @@ Type *llvm::getTypeForLLT(LLT Ty, LLVMContext &C) {
Ty.getElementCount());
return IntegerType::get(C, Ty.getSizeInBits());
}

APInt llvm::GIConstant::getScalarValue() const {
assert(Kind == GIConstantKind::Scalar && "Expected scalar constant");

return Value;
}

std::optional<GIConstant>
llvm::GIConstant::getConstant(Register Const, const MachineRegisterInfo &MRI) {
MachineInstr *Constant = getDefIgnoringCopies(Const, MRI);

if (GSplatVector *Splat = dyn_cast<GSplatVector>(Constant)) {
std::optional<ValueAndVReg> MayBeConstant =
getIConstantVRegValWithLookThrough(Splat->getScalarReg(), MRI);
if (!MayBeConstant)
return std::nullopt;
return GIConstant(MayBeConstant->Value, GIConstantKind::ScalableVector);
}

if (GBuildVector *Build = dyn_cast<GBuildVector>(Constant)) {
SmallVector<APInt> Values;
unsigned NumSources = Build->getNumSources();
for (unsigned I = 0; I < NumSources; ++I) {
Register SrcReg = Build->getSourceReg(I);
std::optional<ValueAndVReg> MayBeConstant =
getIConstantVRegValWithLookThrough(SrcReg, MRI);
if (!MayBeConstant)
return std::nullopt;
Values.push_back(MayBeConstant->Value);
}
return GIConstant(Values);
}

std::optional<ValueAndVReg> MayBeConstant =
getIConstantVRegValWithLookThrough(Const, MRI);
if (!MayBeConstant)
return std::nullopt;

return GIConstant(MayBeConstant->Value, GIConstantKind::Scalar);
}
95 changes: 95 additions & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/combine-canonicalize-icmp.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -o - -mtriple=aarch64-unknown-unknown -run-pass=aarch64-prelegalizer-combiner -verify-machineinstrs %s | FileCheck %s --check-prefixes=CHECK

---
name: test_icmp_canon
body: |
bb.1:
; CHECK-LABEL: name: test_icmp_canon
; CHECK: %lhs:_(s64) = G_CONSTANT i64 11
; CHECK-NEXT: %rhs:_(s64) = COPY $x0
; CHECK-NEXT: %res:_(s32) = G_ICMP intpred(sgt), %rhs(s64), %lhs
; CHECK-NEXT: $w0 = COPY %res(s32)
%lhs:_(s64) = G_CONSTANT i64 11
%rhs:_(s64) = COPY $x0
%res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
$w0 = COPY %res(s32)
...
---
name: test_icmp_no_canon
body: |
bb.1:
; CHECK-LABEL: name: test_icmp_no_canon
; CHECK: %lhs:_(s64) = COPY $x0
; CHECK-NEXT: %rhs:_(s64) = G_CONSTANT i64 11
; CHECK-NEXT: %res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
; CHECK-NEXT: $w0 = COPY %res(s32)
%lhs:_(s64) = COPY $x0
%rhs:_(s64) = G_CONSTANT i64 11
%res:_(s32) = G_ICMP intpred(slt), %lhs(s64), %rhs
$w0 = COPY %res(s32)
...
---
name: test_icmp_canon_bv
body: |
bb.1:
; CHECK-LABEL: name: test_icmp_canon_bv
; CHECK: %opaque1:_(s64) = COPY $x0
; CHECK-NEXT: %opaque2:_(s64) = COPY $x0
; CHECK-NEXT: %const1:_(s64) = G_CONSTANT i64 11
; CHECK-NEXT: %const2:_(s64) = G_CONSTANT i64 12
; CHECK-NEXT: %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %const2(s64)
; CHECK-NEXT: %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %opaque2(s64)
; CHECK-NEXT: %res:_(<2 x s32>) = G_ICMP intpred(sgt), %rhs(<2 x s64>), %lhs
; CHECK-NEXT: $x0 = COPY %res(<2 x s32>)
%opaque1:_(s64) = COPY $x0
%opaque2:_(s64) = COPY $x0
%const1:_(s64) = G_CONSTANT i64 11
%const2:_(s64) = G_CONSTANT i64 12
%lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %const2(s64)
%rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %opaque2(s64)
%res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
$x0 = COPY %res(<2 x s32>)
...
---
name: test_icmp_no_canon_bv_neither_const
body: |
bb.1:
; CHECK-LABEL: name: test_icmp_no_canon_bv
; CHECK: %opaque1:_(s64) = COPY $x0
; CHECK-NEXT: %opaque2:_(s64) = COPY $x0
; CHECK-NEXT: %const1:_(s64) = G_CONSTANT i64 11
; CHECK-NEXT: %const2:_(s64) = G_CONSTANT i64 12
; CHECK-NEXT: %lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %opaque2(s64)
; CHECK-NEXT: %rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %const2(s64)
; CHECK-NEXT: %res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
; CHECK-NEXT: $x0 = COPY %res(<2 x s32>)
%opaque1:_(s64) = COPY $x0
%opaque2:_(s64) = COPY $x0
%const1:_(s64) = G_CONSTANT i64 11
%const2:_(s64) = G_CONSTANT i64 12
%lhs:_(<2 x s64>) = G_BUILD_VECTOR %const1(s64), %opaque2(s64)
%rhs:_(<2 x s64>) = G_BUILD_VECTOR %opaque1(s64), %const2(s64)
%res:_(<2 x s32>) = G_ICMP intpred(slt), %lhs(<2 x s64>), %rhs
$x0 = COPY %res(<2 x s32>)
...
---
name: test_icmp_canon_splat
body: |
bb.1:
; CHECK-LABEL: name: test_icmp_canon_splat
; CHECK: %const:_(s64) = G_CONSTANT i64 11
; CHECK-NEXT: %lhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %const(s64)
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x1
; CHECK-NEXT: %rhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR [[COPY]](s64)
; CHECK-NEXT: %res:_(<vscale x 2 x s32>) = G_ICMP intpred(sgt), %rhs(<vscale x 2 x s64>), %lhs
; CHECK-NEXT: %z:_(<vscale x 2 x s64>) = G_ZEXT %res(<vscale x 2 x s32>)
; CHECK-NEXT: $z0 = COPY %z(<vscale x 2 x s64>)
%const:_(s64) = G_CONSTANT i64 11
%lhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %const:_(s64)
%1:_(s64) = COPY $x1
%rhs:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %1:_(s64)
%res:_(<vscale x 2 x s32>) = G_ICMP intpred(slt), %lhs(<vscale x 2 x s64>), %rhs
%z:_(<vscale x 2 x s64>) = G_ZEXT %res
$z0 = COPY %z(<vscale x 2 x s64>)
...
Loading
Loading