[GlobalISel] An optimizing legal MIR builder #132282
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Author
Member
|
@llvm/pr-subscribers-llvm-globalisel Author: None (john-stuart2) ChangesIt optimizes instructions while building them. It looks at the constness and values of its parameters to optimize them. It might build copies. In some cases partial constness or values are sufficient to optimize. The only context relied on are constants and their values. The builder will never use known bits to optimize. It always relies on legality before building. It uses undefness for optimization. The CSEMIRBuilder can build maybe illegal build vectors pass the legalizer without any means to verify. It is inconsistent whether to optimize fixed-length vectors. It never optimizes scalable vectors. The win of the new approach are separation of concern and correctness. TODO: move constant folding out of CSEMIRBuilder I put the new builder only into the post legalizer combiner to keep the patch small and show the demand for a legal builder. I like the GIConstant. It keeps the builder smaller and simpler. Patch is 101.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132282.diff 18 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/OptMIRBuilder.h b/llvm/include/llvm/CodeGen/GlobalISel/OptMIRBuilder.h
new file mode 100644
index 0000000000000..7df63bef1d9ff
--- /dev/null
+++ b/llvm/include/llvm/CodeGen/GlobalISel/OptMIRBuilder.h
@@ -0,0 +1,83 @@
+//===-- llvm/CodeGen/GlobalISel/OptMIRBuilder.h --*- C++ -*-==---------------//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+/// \file This file implements a legal version of MachineIRBuilder which
+/// optimizes insts while building.
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CODEGEN_GLOBALISEL_OPTMIRBUILDER_H
+#define LLVM_CODEGEN_GLOBALISEL_OPTMIRBUILDER_H
+
+#include "llvm/CodeGen/GlobalISel/CSEMIRBuilder.h"
+
+namespace llvm {
+
+class LegalizerInfo;
+struct LegalityQuery;
+
+/// OptMIRBuilder optimizes instructions while building them. It
+/// checks its operands whether they are constant or undef. It never
+/// checks whether an operand is defined by G_FSINCOS. It checks
+/// operands and registers for G_IMPLICIT_DEF, G_CONSTANT,
+/// G_BUILD_VECTOR, and G_SPLAT_VECTOR and nothing else.
+/// Based on undef, the constants and their values, it folds
+/// instructions into constants, undef, or other instructions. For
+/// optmizations and constant folding it relies on GIConstant.
+/// It can fold G_MUL into G_ADD and G_SUB. Before folding
+/// it always queries the legalizer. When it fails to fold, it
+/// delegates the building to the CSEMIRBuilder. It is the users
+/// responsibility to only attempt to build legal instructions pass
+/// the legalizer. OptMIRBuilder can safely be used in optimization
+/// passes pass the legalizer.
+class OptMIRBuilder : public CSEMIRBuilder {
+ const LegalizerInfo *LI;
+ const bool IsPrelegalize;
+
+ /// Legality tests.
+ bool isPrelegalize() const;
+ bool isLegal(const LegalityQuery &Query) const;
+ bool isConstantLegal(LLT);
+ bool isLegalOrBeforeLegalizer(const LegalityQuery &Query) const;
+ bool isConstantLegalOrBeforeLegalizer(LLT);
+
+ /// Returns true if the register \p R is defined by G_IMPLICIT_DEF.
+ bool isUndef(Register R) const;
+
+ /// Builds 0 - X.
+ MachineInstrBuilder buildNegation(const DstOp &, const SrcOp &);
+
+ // Constants.
+ MachineInstrBuilder buildGIConstant(const DstOp &DstOp, const GIConstant &);
+
+ /// Integer.
+ MachineInstrBuilder optimizeAdd(unsigned Opc, ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag = std::nullopt);
+ MachineInstrBuilder optimizeSub(unsigned Opc, ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag = std::nullopt);
+ MachineInstrBuilder optimizeMul(unsigned Opc, ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag = std::nullopt);
+
+public:
+ OptMIRBuilder(MachineFunction &MF, GISelCSEInfo *CSEInfo,
+ GISelChangeObserver &Observer, const LegalizerInfo *LI,
+ bool IsPrelegalize)
+ : LI(LI), IsPrelegalize(IsPrelegalize) {
+ setMF(MF);
+ setCSEInfo(CSEInfo);
+ setChangeObserver(Observer);
+ };
+
+ MachineInstrBuilder
+ buildInstr(unsigned Opc, ArrayRef<DstOp> DstOps, ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag = std::nullopt) override;
+};
+
+} // namespace llvm
+
+#endif // LLVM_CODEGEN_GLOBALISEL_OPTMIRBUILDER_H
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index a35ecae5d18bf..25f8b84f381e2 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -635,8 +635,28 @@ class GIConstant {
/// Returns the value, if this constant is a scalar.
APInt getScalarValue() const;
+ /// Returns the value, if this constant is a scalable vector.
+ APInt getSplatValue() const;
+
+ /// Returns the values, if this constant is a fixed vector.
+ ArrayRef<APInt> getAsArrayRef() const;
+
static std::optional<GIConstant> getConstant(Register Const,
const MachineRegisterInfo &MRI);
+
+ /// Returns a new constant where add(this, x) was applied.
+ GIConstant add(const GIConstant &) const;
+
+ /// Returns a new constant where sub(this, x) was applied.
+ GIConstant sub(const GIConstant &) const;
+
+ /// Returns a new constant where mul(this, x) was applied.
+ GIConstant mul(const GIConstant &) const;
+
+ bool isZero() const;
+ bool isOne() const;
+ bool isTwo() const;
+ bool isAllOnes() const;
};
/// An floating-point-like constant.
diff --git a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
index a45024d120be6..a19e433f1401a 100644
--- a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
+++ b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
@@ -26,6 +26,7 @@ add_llvm_component_library(LLVMGlobalISel
Localizer.cpp
LostDebugLocObserver.cpp
MachineIRBuilder.cpp
+ OptMIRBuilder.cpp
RegBankSelect.cpp
Utils.cpp
diff --git a/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
index bf8e847011d7c..23ff50f5296af 100644
--- a/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
@@ -199,15 +199,12 @@ MachineInstrBuilder CSEMIRBuilder::buildInstr(unsigned Opc,
}
break;
}
- case TargetOpcode::G_ADD:
case TargetOpcode::G_PTR_ADD:
case TargetOpcode::G_AND:
case TargetOpcode::G_ASHR:
case TargetOpcode::G_LSHR:
- case TargetOpcode::G_MUL:
case TargetOpcode::G_OR:
case TargetOpcode::G_SHL:
- case TargetOpcode::G_SUB:
case TargetOpcode::G_XOR:
case TargetOpcode::G_UDIV:
case TargetOpcode::G_SDIV:
diff --git a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
index 359677027f52f..f96c65ae4993e 100644
--- a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
@@ -321,8 +321,12 @@ MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
assert(EltTy.getScalarSizeInBits() == Val.getBitWidth() &&
"creating constant with the wrong size");
- assert(!Ty.isScalableVector() &&
- "unexpected scalable vector in buildConstant");
+ if (Ty.isScalableVector()) {
+ auto Const = buildInstr(TargetOpcode::G_CONSTANT)
+ .addDef(getMRI()->createGenericVirtualRegister(EltTy))
+ .addCImm(&Val);
+ return buildSplatVector(Res, Const);
+ }
if (Ty.isFixedVector()) {
auto Const = buildInstr(TargetOpcode::G_CONSTANT)
diff --git a/llvm/lib/CodeGen/GlobalISel/OptMIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/OptMIRBuilder.cpp
new file mode 100644
index 0000000000000..66b879f5113dd
--- /dev/null
+++ b/llvm/lib/CodeGen/GlobalISel/OptMIRBuilder.cpp
@@ -0,0 +1,206 @@
+//===-- llvm/CodeGen/GlobalISel/OptMIRBuilder.cpp -----------------*- C++-*-==//
+// 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
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file implements the OptMIRBuilder class which optimizes as it builds
+/// instructions.
+//===----------------------------------------------------------------------===//
+//
+
+#include "llvm/CodeGen/GlobalISel/OptMIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/CSEMIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/Utils.h"
+#include "llvm/CodeGen/TargetOpcodes.h"
+
+using namespace llvm;
+
+bool OptMIRBuilder::isPrelegalize() const { return IsPrelegalize; }
+
+bool OptMIRBuilder::isLegal(const LegalityQuery &Query) const {
+ assert(LI != nullptr && "legalizer info is not available");
+ return LI->isLegal(Query);
+}
+
+bool OptMIRBuilder::isConstantLegal(LLT Ty) {
+ if (Ty.isScalar())
+ return isLegal({TargetOpcode::G_CONSTANT, {Ty}});
+
+ LLT EltTy = Ty.getElementType();
+ if (Ty.isFixedVector())
+ return isLegal({TargetOpcode::G_BUILD_VECTOR, {Ty, EltTy}}) &&
+ isLegal({TargetOpcode::G_CONSTANT, {EltTy}});
+
+ // scalable vector
+ assert(Ty.isScalableVector() && "Unexpected LLT");
+ return isLegal({TargetOpcode::G_SPLAT_VECTOR, {Ty, EltTy}}) &&
+ isLegal({TargetOpcode::G_CONSTANT, {EltTy}});
+}
+
+bool OptMIRBuilder::isLegalOrBeforeLegalizer(const LegalityQuery &Query) const {
+ return isPrelegalize() || isLegal(Query);
+}
+
+bool OptMIRBuilder::isConstantLegalOrBeforeLegalizer(LLT Ty) {
+ return isPrelegalize() || isConstantLegal(Ty);
+}
+
+bool OptMIRBuilder::isUndef(Register Reg) const {
+ const MachineInstr *MI = getMRI()->getVRegDef(Reg);
+ return MI->getOpcode() == TargetOpcode::G_IMPLICIT_DEF;
+}
+
+MachineInstrBuilder OptMIRBuilder::buildNegation(const DstOp &DstOp,
+ const SrcOp &SrcOp) {
+ LLT DstTy = DstOp.getLLTTy(*getMRI());
+
+ auto Zero = buildConstant(DstTy, 0);
+ return buildSub(DstOp, Zero, SrcOp);
+}
+
+MachineInstrBuilder OptMIRBuilder::buildGIConstant(const DstOp &DstOp,
+ const GIConstant &Const) {
+ LLT DstTy = DstOp.getLLTTy(*getMRI());
+
+ switch (Const.getKind()) {
+ case GIConstant::GIConstantKind::Scalar:
+ return buildConstant(DstOp, Const.getScalarValue());
+ case GIConstant::GIConstantKind::FixedVector:
+ return buildBuildVectorConstant(DstOp, Const.getAsArrayRef());
+ case GIConstant::GIConstantKind::ScalableVector: {
+ auto Constant =
+ buildConstant(DstTy.getElementType(), Const.getSplatValue());
+ return buildSplatVector(DstOp, Constant);
+ }
+ }
+}
+
+MachineInstrBuilder OptMIRBuilder::optimizeAdd(unsigned Opc,
+ ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag) {
+ assert(SrcOps.size() == 2 && "Invalid sources");
+ assert(DstOps.size() == 1 && "Invalid dsts");
+
+ LLT DstTy = DstOps[0].getLLTTy(*getMRI());
+
+ if (isUndef(SrcOps[1].getReg()) || isUndef(SrcOps[0].getReg()))
+ return buildUndef(DstTy);
+
+ std::optional<GIConstant> RHS =
+ GIConstant::getConstant(SrcOps[1].getReg(), *getMRI());
+ if (!RHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ if (RHS->isZero())
+ return buildCopy(DstOps[0], SrcOps[0]);
+
+ if (!isConstantLegalOrBeforeLegalizer(DstTy))
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ std::optional<GIConstant> LHS =
+ GIConstant::getConstant(SrcOps[0].getReg(), *getMRI());
+ if (!LHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ GIConstant Add = LHS->add(*RHS);
+
+ return buildGIConstant(DstOps[0], Add);
+}
+
+MachineInstrBuilder OptMIRBuilder::optimizeSub(unsigned Opc,
+ ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag) {
+ assert(SrcOps.size() == 2 && "Invalid sources");
+ assert(DstOps.size() == 1 && "Invalid dsts");
+
+ LLT DstTy = DstOps[0].getLLTTy(*getMRI());
+
+ if (isUndef(SrcOps[1].getReg()) || isUndef(SrcOps[0].getReg()))
+ return buildUndef(DstTy);
+
+ std::optional<GIConstant> RHS =
+ GIConstant::getConstant(SrcOps[1].getReg(), *getMRI());
+ if (!RHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ if (RHS->isZero())
+ return buildCopy(DstOps[0], SrcOps[0]);
+
+ if (!isConstantLegalOrBeforeLegalizer(DstTy))
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ std::optional<GIConstant> LHS =
+ GIConstant::getConstant(SrcOps[0].getReg(), *getMRI());
+ if (!LHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ GIConstant Sub = LHS->sub(*RHS);
+
+ return buildGIConstant(DstOps[0], Sub);
+}
+
+MachineInstrBuilder OptMIRBuilder::optimizeMul(unsigned Opc,
+ ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag) {
+ assert(SrcOps.size() == 2 && "Invalid sources");
+ assert(DstOps.size() == 1 && "Invalid dsts");
+
+ LLT DstTy = DstOps[0].getLLTTy(*getMRI());
+
+ if ((isUndef(SrcOps[1].getReg()) || isUndef(SrcOps[0].getReg())) &&
+ isConstantLegalOrBeforeLegalizer(DstTy))
+ return buildConstant(DstTy, 0);
+
+ std::optional<GIConstant> RHS =
+ GIConstant::getConstant(SrcOps[1].getReg(), *getMRI());
+ if (!RHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ if (RHS->isZero() && isConstantLegalOrBeforeLegalizer(DstTy))
+ return buildConstant(DstTy, 0);
+
+ if (RHS->isOne())
+ return buildCopy(DstOps[0], SrcOps[0]);
+
+ if (RHS->isAllOnes() && isConstantLegalOrBeforeLegalizer(DstTy) &&
+ isLegalOrBeforeLegalizer({TargetOpcode::G_SUB, {DstTy}}))
+ return buildNegation(DstOps[0], SrcOps[0]);
+
+ if (RHS->isTwo() && isLegalOrBeforeLegalizer({TargetOpcode::G_ADD, {DstTy}}))
+ return buildAdd(DstOps[0], SrcOps[0], SrcOps[0]);
+
+ if (!isConstantLegalOrBeforeLegalizer(DstTy))
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ std::optional<GIConstant> LHS =
+ GIConstant::getConstant(SrcOps[0].getReg(), *getMRI());
+ if (!LHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ GIConstant Mul = LHS->mul(*RHS);
+
+ return buildGIConstant(DstOps[0], Mul);
+}
+
+MachineInstrBuilder OptMIRBuilder::buildInstr(unsigned Opc,
+ ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag) {
+ switch (Opc) {
+ case TargetOpcode::G_ADD:
+ return optimizeAdd(Opc, DstOps, SrcOps, Flag);
+ case TargetOpcode::G_SUB:
+ return optimizeSub(Opc, DstOps, SrcOps, Flag);
+ case TargetOpcode::G_MUL:
+ return optimizeMul(Opc, DstOps, SrcOps, Flag);
+ default:
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+ }
+}
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 625d556e3ff5e..1b417789676e1 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -1997,6 +1997,19 @@ APInt llvm::GIConstant::getScalarValue() const {
return Value;
}
+APInt llvm::GIConstant::getSplatValue() const {
+ assert(Kind == GIConstantKind::ScalableVector &&
+ "Expected scalable constant");
+
+ return Value;
+}
+
+ArrayRef<APInt> llvm::GIConstant::getAsArrayRef() const {
+ assert(Kind == GIConstantKind::FixedVector &&
+ "Expected fixed vector constant");
+ return Values;
+}
+
std::optional<GIConstant>
llvm::GIConstant::getConstant(Register Const, const MachineRegisterInfo &MRI) {
MachineInstr *Constant = getDefIgnoringCopies(Const, MRI);
@@ -2031,6 +2044,117 @@ llvm::GIConstant::getConstant(Register Const, const MachineRegisterInfo &MRI) {
return GIConstant(MayBeConstant->Value, GIConstantKind::Scalar);
}
+/// Returns a new constant where add(x) was applied.
+GIConstant llvm::GIConstant::add(const GIConstant &RHS) const {
+ switch (getKind()) {
+ case GIConstantKind::ScalableVector:
+ return GIConstant(Value + RHS.Value, GIConstantKind::ScalableVector);
+ case GIConstantKind::Scalar: {
+ return GIConstant(Value + RHS.Value, GIConstantKind::Scalar);
+ }
+ case GIConstantKind::FixedVector: {
+ SmallVector<APInt> Adds;
+ for (unsigned I = 0, E = Values.size(); I < E; ++I)
+ Adds.push_back(Values[I] + RHS.Values[I]);
+ return GIConstant(Adds);
+ }
+ }
+}
+
+/// Returns a new constant where sub(x) was applied.
+GIConstant llvm::GIConstant::sub(const GIConstant &RHS) const {
+ switch (getKind()) {
+ case GIConstantKind::ScalableVector:
+ return GIConstant(Value - RHS.Value, GIConstantKind::ScalableVector);
+ case GIConstantKind::Scalar: {
+ return GIConstant(Value - RHS.Value, GIConstantKind::Scalar);
+ }
+ case GIConstantKind::FixedVector: {
+ SmallVector<APInt> Subs;
+ for (unsigned I = 0, E = Values.size(); I < E; ++I)
+ Subs.push_back(Values[I] - RHS.Values[I]);
+ return GIConstant(Subs);
+ }
+ }
+}
+
+/// Returns a new constant where mul(x) was applied.
+GIConstant llvm::GIConstant::mul(const GIConstant &RHS) const {
+ switch (getKind()) {
+ case GIConstantKind::ScalableVector:
+ return GIConstant(Value * RHS.Value, GIConstantKind::ScalableVector);
+ case GIConstantKind::Scalar: {
+ return GIConstant(Value * RHS.Value, GIConstantKind::Scalar);
+ }
+ case GIConstantKind::FixedVector: {
+ SmallVector<APInt> Muls;
+ for (unsigned I = 0, E = Values.size(); I < E; ++I)
+ Muls.push_back(Values[I] * RHS.Values[I]);
+ return GIConstant(Muls);
+ }
+ }
+}
+
+bool llvm::GIConstant::isZero() const {
+ switch (Kind) {
+ case GIConstantKind::Scalar:
+ return Value.isZero();
+ case GIConstantKind::ScalableVector:
+ return Value.isZero();
+ case GIConstantKind::FixedVector: {
+ for (const APInt &V : Values)
+ if (!V.isZero())
+ return false;
+ return true;
+ }
+ }
+}
+
+bool llvm::GIConstant::isOne() const {
+ switch (Kind) {
+ case GIConstantKind::Scalar:
+ return Value.isOne();
+ case GIConstantKind::ScalableVector:
+ return Value.isOne();
+ case GIConstantKind::FixedVector: {
+ for (const APInt &V : Values)
+ if (!V.isOne())
+ return false;
+ return true;
+ }
+ }
+}
+
+bool llvm::GIConstant::isTwo() const {
+ switch (Kind) {
+ case GIConstantKind::Scalar:
+ return Value.getLimitedValue() == 2;
+ case GIConstantKind::ScalableVector:
+ return Value.getLimitedValue() == 2;
+ case GIConstantKind::FixedVector: {
+ for (const APInt &V : Values)
+ if (V.getLimitedValue() != 2)
+ return false;
+ return true;
+ }
+ }
+}
+
+bool llvm::GIConstant::isAllOnes() const {
+ switch (Kind) {
+ case GIConstantKind::Scalar:
+ return Value.isAllOnes();
+ case GIConstantKind::ScalableVector:
+ return Value.isAllOnes();
+ case GIConstantKind::FixedVector: {
+ for (const APInt &V : Values)
+ if (!V.isAllOnes())
+ return false;
+ return true;
+ }
+ }
+}
+
APFloat llvm::GFConstant::getScalarValue() const {
assert(Kind == GFConstantKind::Scalar && "Expected scalar constant");
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
index cf6b2ce9c5341..87b47098a2ff2 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
@@ -32,6 +32,7 @@
#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
#include "llvm/CodeGen/GlobalISel/MIPatternMatch.h"
#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/OptMIRBuilder.h"
#include "llvm/CodeGen/GlobalISel/Utils.h"
#include "llvm/CodeGen/MachineDominators.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
@@ -440,6 +441,9 @@ void applyCombineMulCMLT(MachineInstr &MI, MachineRegisterInfo &MRI,
class AArch64PostLegalizerCombinerImpl : public Combiner {
protected:
+ std::unique_ptr<MachineIRBuilder> Opt;
+ // hides Combiner::B
+ MachineIRBuilder &B;
const CombinerHelper Helper;
const AArch64PostLegalizerCombinerImplRuleConfig &RuleConfig;
const AArch64Subtarget &STI;
@@ -473,7 +477,9 @@ AArch64PostLegalizerCombinerImpl::AArch64PostLegalizerCombinerImpl(
const AArch64Subtarget &STI, MachineDominatorTree *MDT,
const LegalizerInfo *LI)
: Combiner(MF, CInfo...
[truncated]
|
Member
|
@llvm/pr-subscribers-backend-aarch64 Author: None (john-stuart2) ChangesIt optimizes instructions while building them. It looks at the constness and values of its parameters to optimize them. It might build copies. In some cases partial constness or values are sufficient to optimize. The only context relied on are constants and their values. The builder will never use known bits to optimize. It always relies on legality before building. It uses undefness for optimization. The CSEMIRBuilder can build maybe illegal build vectors pass the legalizer without any means to verify. It is inconsistent whether to optimize fixed-length vectors. It never optimizes scalable vectors. The win of the new approach are separation of concern and correctness. TODO: move constant folding out of CSEMIRBuilder I put the new builder only into the post legalizer combiner to keep the patch small and show the demand for a legal builder. I like the GIConstant. It keeps the builder smaller and simpler. Patch is 101.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132282.diff 18 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/OptMIRBuilder.h b/llvm/include/llvm/CodeGen/GlobalISel/OptMIRBuilder.h
new file mode 100644
index 0000000000000..7df63bef1d9ff
--- /dev/null
+++ b/llvm/include/llvm/CodeGen/GlobalISel/OptMIRBuilder.h
@@ -0,0 +1,83 @@
+//===-- llvm/CodeGen/GlobalISel/OptMIRBuilder.h --*- C++ -*-==---------------//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+/// \file This file implements a legal version of MachineIRBuilder which
+/// optimizes insts while building.
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CODEGEN_GLOBALISEL_OPTMIRBUILDER_H
+#define LLVM_CODEGEN_GLOBALISEL_OPTMIRBUILDER_H
+
+#include "llvm/CodeGen/GlobalISel/CSEMIRBuilder.h"
+
+namespace llvm {
+
+class LegalizerInfo;
+struct LegalityQuery;
+
+/// OptMIRBuilder optimizes instructions while building them. It
+/// checks its operands whether they are constant or undef. It never
+/// checks whether an operand is defined by G_FSINCOS. It checks
+/// operands and registers for G_IMPLICIT_DEF, G_CONSTANT,
+/// G_BUILD_VECTOR, and G_SPLAT_VECTOR and nothing else.
+/// Based on undef, the constants and their values, it folds
+/// instructions into constants, undef, or other instructions. For
+/// optmizations and constant folding it relies on GIConstant.
+/// It can fold G_MUL into G_ADD and G_SUB. Before folding
+/// it always queries the legalizer. When it fails to fold, it
+/// delegates the building to the CSEMIRBuilder. It is the users
+/// responsibility to only attempt to build legal instructions pass
+/// the legalizer. OptMIRBuilder can safely be used in optimization
+/// passes pass the legalizer.
+class OptMIRBuilder : public CSEMIRBuilder {
+ const LegalizerInfo *LI;
+ const bool IsPrelegalize;
+
+ /// Legality tests.
+ bool isPrelegalize() const;
+ bool isLegal(const LegalityQuery &Query) const;
+ bool isConstantLegal(LLT);
+ bool isLegalOrBeforeLegalizer(const LegalityQuery &Query) const;
+ bool isConstantLegalOrBeforeLegalizer(LLT);
+
+ /// Returns true if the register \p R is defined by G_IMPLICIT_DEF.
+ bool isUndef(Register R) const;
+
+ /// Builds 0 - X.
+ MachineInstrBuilder buildNegation(const DstOp &, const SrcOp &);
+
+ // Constants.
+ MachineInstrBuilder buildGIConstant(const DstOp &DstOp, const GIConstant &);
+
+ /// Integer.
+ MachineInstrBuilder optimizeAdd(unsigned Opc, ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag = std::nullopt);
+ MachineInstrBuilder optimizeSub(unsigned Opc, ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag = std::nullopt);
+ MachineInstrBuilder optimizeMul(unsigned Opc, ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag = std::nullopt);
+
+public:
+ OptMIRBuilder(MachineFunction &MF, GISelCSEInfo *CSEInfo,
+ GISelChangeObserver &Observer, const LegalizerInfo *LI,
+ bool IsPrelegalize)
+ : LI(LI), IsPrelegalize(IsPrelegalize) {
+ setMF(MF);
+ setCSEInfo(CSEInfo);
+ setChangeObserver(Observer);
+ };
+
+ MachineInstrBuilder
+ buildInstr(unsigned Opc, ArrayRef<DstOp> DstOps, ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag = std::nullopt) override;
+};
+
+} // namespace llvm
+
+#endif // LLVM_CODEGEN_GLOBALISEL_OPTMIRBUILDER_H
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index a35ecae5d18bf..25f8b84f381e2 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -635,8 +635,28 @@ class GIConstant {
/// Returns the value, if this constant is a scalar.
APInt getScalarValue() const;
+ /// Returns the value, if this constant is a scalable vector.
+ APInt getSplatValue() const;
+
+ /// Returns the values, if this constant is a fixed vector.
+ ArrayRef<APInt> getAsArrayRef() const;
+
static std::optional<GIConstant> getConstant(Register Const,
const MachineRegisterInfo &MRI);
+
+ /// Returns a new constant where add(this, x) was applied.
+ GIConstant add(const GIConstant &) const;
+
+ /// Returns a new constant where sub(this, x) was applied.
+ GIConstant sub(const GIConstant &) const;
+
+ /// Returns a new constant where mul(this, x) was applied.
+ GIConstant mul(const GIConstant &) const;
+
+ bool isZero() const;
+ bool isOne() const;
+ bool isTwo() const;
+ bool isAllOnes() const;
};
/// An floating-point-like constant.
diff --git a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
index a45024d120be6..a19e433f1401a 100644
--- a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
+++ b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
@@ -26,6 +26,7 @@ add_llvm_component_library(LLVMGlobalISel
Localizer.cpp
LostDebugLocObserver.cpp
MachineIRBuilder.cpp
+ OptMIRBuilder.cpp
RegBankSelect.cpp
Utils.cpp
diff --git a/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
index bf8e847011d7c..23ff50f5296af 100644
--- a/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
@@ -199,15 +199,12 @@ MachineInstrBuilder CSEMIRBuilder::buildInstr(unsigned Opc,
}
break;
}
- case TargetOpcode::G_ADD:
case TargetOpcode::G_PTR_ADD:
case TargetOpcode::G_AND:
case TargetOpcode::G_ASHR:
case TargetOpcode::G_LSHR:
- case TargetOpcode::G_MUL:
case TargetOpcode::G_OR:
case TargetOpcode::G_SHL:
- case TargetOpcode::G_SUB:
case TargetOpcode::G_XOR:
case TargetOpcode::G_UDIV:
case TargetOpcode::G_SDIV:
diff --git a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
index 359677027f52f..f96c65ae4993e 100644
--- a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
@@ -321,8 +321,12 @@ MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
assert(EltTy.getScalarSizeInBits() == Val.getBitWidth() &&
"creating constant with the wrong size");
- assert(!Ty.isScalableVector() &&
- "unexpected scalable vector in buildConstant");
+ if (Ty.isScalableVector()) {
+ auto Const = buildInstr(TargetOpcode::G_CONSTANT)
+ .addDef(getMRI()->createGenericVirtualRegister(EltTy))
+ .addCImm(&Val);
+ return buildSplatVector(Res, Const);
+ }
if (Ty.isFixedVector()) {
auto Const = buildInstr(TargetOpcode::G_CONSTANT)
diff --git a/llvm/lib/CodeGen/GlobalISel/OptMIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/OptMIRBuilder.cpp
new file mode 100644
index 0000000000000..66b879f5113dd
--- /dev/null
+++ b/llvm/lib/CodeGen/GlobalISel/OptMIRBuilder.cpp
@@ -0,0 +1,206 @@
+//===-- llvm/CodeGen/GlobalISel/OptMIRBuilder.cpp -----------------*- C++-*-==//
+// 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
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file implements the OptMIRBuilder class which optimizes as it builds
+/// instructions.
+//===----------------------------------------------------------------------===//
+//
+
+#include "llvm/CodeGen/GlobalISel/OptMIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/CSEMIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/Utils.h"
+#include "llvm/CodeGen/TargetOpcodes.h"
+
+using namespace llvm;
+
+bool OptMIRBuilder::isPrelegalize() const { return IsPrelegalize; }
+
+bool OptMIRBuilder::isLegal(const LegalityQuery &Query) const {
+ assert(LI != nullptr && "legalizer info is not available");
+ return LI->isLegal(Query);
+}
+
+bool OptMIRBuilder::isConstantLegal(LLT Ty) {
+ if (Ty.isScalar())
+ return isLegal({TargetOpcode::G_CONSTANT, {Ty}});
+
+ LLT EltTy = Ty.getElementType();
+ if (Ty.isFixedVector())
+ return isLegal({TargetOpcode::G_BUILD_VECTOR, {Ty, EltTy}}) &&
+ isLegal({TargetOpcode::G_CONSTANT, {EltTy}});
+
+ // scalable vector
+ assert(Ty.isScalableVector() && "Unexpected LLT");
+ return isLegal({TargetOpcode::G_SPLAT_VECTOR, {Ty, EltTy}}) &&
+ isLegal({TargetOpcode::G_CONSTANT, {EltTy}});
+}
+
+bool OptMIRBuilder::isLegalOrBeforeLegalizer(const LegalityQuery &Query) const {
+ return isPrelegalize() || isLegal(Query);
+}
+
+bool OptMIRBuilder::isConstantLegalOrBeforeLegalizer(LLT Ty) {
+ return isPrelegalize() || isConstantLegal(Ty);
+}
+
+bool OptMIRBuilder::isUndef(Register Reg) const {
+ const MachineInstr *MI = getMRI()->getVRegDef(Reg);
+ return MI->getOpcode() == TargetOpcode::G_IMPLICIT_DEF;
+}
+
+MachineInstrBuilder OptMIRBuilder::buildNegation(const DstOp &DstOp,
+ const SrcOp &SrcOp) {
+ LLT DstTy = DstOp.getLLTTy(*getMRI());
+
+ auto Zero = buildConstant(DstTy, 0);
+ return buildSub(DstOp, Zero, SrcOp);
+}
+
+MachineInstrBuilder OptMIRBuilder::buildGIConstant(const DstOp &DstOp,
+ const GIConstant &Const) {
+ LLT DstTy = DstOp.getLLTTy(*getMRI());
+
+ switch (Const.getKind()) {
+ case GIConstant::GIConstantKind::Scalar:
+ return buildConstant(DstOp, Const.getScalarValue());
+ case GIConstant::GIConstantKind::FixedVector:
+ return buildBuildVectorConstant(DstOp, Const.getAsArrayRef());
+ case GIConstant::GIConstantKind::ScalableVector: {
+ auto Constant =
+ buildConstant(DstTy.getElementType(), Const.getSplatValue());
+ return buildSplatVector(DstOp, Constant);
+ }
+ }
+}
+
+MachineInstrBuilder OptMIRBuilder::optimizeAdd(unsigned Opc,
+ ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag) {
+ assert(SrcOps.size() == 2 && "Invalid sources");
+ assert(DstOps.size() == 1 && "Invalid dsts");
+
+ LLT DstTy = DstOps[0].getLLTTy(*getMRI());
+
+ if (isUndef(SrcOps[1].getReg()) || isUndef(SrcOps[0].getReg()))
+ return buildUndef(DstTy);
+
+ std::optional<GIConstant> RHS =
+ GIConstant::getConstant(SrcOps[1].getReg(), *getMRI());
+ if (!RHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ if (RHS->isZero())
+ return buildCopy(DstOps[0], SrcOps[0]);
+
+ if (!isConstantLegalOrBeforeLegalizer(DstTy))
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ std::optional<GIConstant> LHS =
+ GIConstant::getConstant(SrcOps[0].getReg(), *getMRI());
+ if (!LHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ GIConstant Add = LHS->add(*RHS);
+
+ return buildGIConstant(DstOps[0], Add);
+}
+
+MachineInstrBuilder OptMIRBuilder::optimizeSub(unsigned Opc,
+ ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag) {
+ assert(SrcOps.size() == 2 && "Invalid sources");
+ assert(DstOps.size() == 1 && "Invalid dsts");
+
+ LLT DstTy = DstOps[0].getLLTTy(*getMRI());
+
+ if (isUndef(SrcOps[1].getReg()) || isUndef(SrcOps[0].getReg()))
+ return buildUndef(DstTy);
+
+ std::optional<GIConstant> RHS =
+ GIConstant::getConstant(SrcOps[1].getReg(), *getMRI());
+ if (!RHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ if (RHS->isZero())
+ return buildCopy(DstOps[0], SrcOps[0]);
+
+ if (!isConstantLegalOrBeforeLegalizer(DstTy))
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ std::optional<GIConstant> LHS =
+ GIConstant::getConstant(SrcOps[0].getReg(), *getMRI());
+ if (!LHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ GIConstant Sub = LHS->sub(*RHS);
+
+ return buildGIConstant(DstOps[0], Sub);
+}
+
+MachineInstrBuilder OptMIRBuilder::optimizeMul(unsigned Opc,
+ ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag) {
+ assert(SrcOps.size() == 2 && "Invalid sources");
+ assert(DstOps.size() == 1 && "Invalid dsts");
+
+ LLT DstTy = DstOps[0].getLLTTy(*getMRI());
+
+ if ((isUndef(SrcOps[1].getReg()) || isUndef(SrcOps[0].getReg())) &&
+ isConstantLegalOrBeforeLegalizer(DstTy))
+ return buildConstant(DstTy, 0);
+
+ std::optional<GIConstant> RHS =
+ GIConstant::getConstant(SrcOps[1].getReg(), *getMRI());
+ if (!RHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ if (RHS->isZero() && isConstantLegalOrBeforeLegalizer(DstTy))
+ return buildConstant(DstTy, 0);
+
+ if (RHS->isOne())
+ return buildCopy(DstOps[0], SrcOps[0]);
+
+ if (RHS->isAllOnes() && isConstantLegalOrBeforeLegalizer(DstTy) &&
+ isLegalOrBeforeLegalizer({TargetOpcode::G_SUB, {DstTy}}))
+ return buildNegation(DstOps[0], SrcOps[0]);
+
+ if (RHS->isTwo() && isLegalOrBeforeLegalizer({TargetOpcode::G_ADD, {DstTy}}))
+ return buildAdd(DstOps[0], SrcOps[0], SrcOps[0]);
+
+ if (!isConstantLegalOrBeforeLegalizer(DstTy))
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ std::optional<GIConstant> LHS =
+ GIConstant::getConstant(SrcOps[0].getReg(), *getMRI());
+ if (!LHS)
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+
+ GIConstant Mul = LHS->mul(*RHS);
+
+ return buildGIConstant(DstOps[0], Mul);
+}
+
+MachineInstrBuilder OptMIRBuilder::buildInstr(unsigned Opc,
+ ArrayRef<DstOp> DstOps,
+ ArrayRef<SrcOp> SrcOps,
+ std::optional<unsigned> Flag) {
+ switch (Opc) {
+ case TargetOpcode::G_ADD:
+ return optimizeAdd(Opc, DstOps, SrcOps, Flag);
+ case TargetOpcode::G_SUB:
+ return optimizeSub(Opc, DstOps, SrcOps, Flag);
+ case TargetOpcode::G_MUL:
+ return optimizeMul(Opc, DstOps, SrcOps, Flag);
+ default:
+ return CSEMIRBuilder::buildInstr(Opc, DstOps, SrcOps, Flag);
+ }
+}
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 625d556e3ff5e..1b417789676e1 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -1997,6 +1997,19 @@ APInt llvm::GIConstant::getScalarValue() const {
return Value;
}
+APInt llvm::GIConstant::getSplatValue() const {
+ assert(Kind == GIConstantKind::ScalableVector &&
+ "Expected scalable constant");
+
+ return Value;
+}
+
+ArrayRef<APInt> llvm::GIConstant::getAsArrayRef() const {
+ assert(Kind == GIConstantKind::FixedVector &&
+ "Expected fixed vector constant");
+ return Values;
+}
+
std::optional<GIConstant>
llvm::GIConstant::getConstant(Register Const, const MachineRegisterInfo &MRI) {
MachineInstr *Constant = getDefIgnoringCopies(Const, MRI);
@@ -2031,6 +2044,117 @@ llvm::GIConstant::getConstant(Register Const, const MachineRegisterInfo &MRI) {
return GIConstant(MayBeConstant->Value, GIConstantKind::Scalar);
}
+/// Returns a new constant where add(x) was applied.
+GIConstant llvm::GIConstant::add(const GIConstant &RHS) const {
+ switch (getKind()) {
+ case GIConstantKind::ScalableVector:
+ return GIConstant(Value + RHS.Value, GIConstantKind::ScalableVector);
+ case GIConstantKind::Scalar: {
+ return GIConstant(Value + RHS.Value, GIConstantKind::Scalar);
+ }
+ case GIConstantKind::FixedVector: {
+ SmallVector<APInt> Adds;
+ for (unsigned I = 0, E = Values.size(); I < E; ++I)
+ Adds.push_back(Values[I] + RHS.Values[I]);
+ return GIConstant(Adds);
+ }
+ }
+}
+
+/// Returns a new constant where sub(x) was applied.
+GIConstant llvm::GIConstant::sub(const GIConstant &RHS) const {
+ switch (getKind()) {
+ case GIConstantKind::ScalableVector:
+ return GIConstant(Value - RHS.Value, GIConstantKind::ScalableVector);
+ case GIConstantKind::Scalar: {
+ return GIConstant(Value - RHS.Value, GIConstantKind::Scalar);
+ }
+ case GIConstantKind::FixedVector: {
+ SmallVector<APInt> Subs;
+ for (unsigned I = 0, E = Values.size(); I < E; ++I)
+ Subs.push_back(Values[I] - RHS.Values[I]);
+ return GIConstant(Subs);
+ }
+ }
+}
+
+/// Returns a new constant where mul(x) was applied.
+GIConstant llvm::GIConstant::mul(const GIConstant &RHS) const {
+ switch (getKind()) {
+ case GIConstantKind::ScalableVector:
+ return GIConstant(Value * RHS.Value, GIConstantKind::ScalableVector);
+ case GIConstantKind::Scalar: {
+ return GIConstant(Value * RHS.Value, GIConstantKind::Scalar);
+ }
+ case GIConstantKind::FixedVector: {
+ SmallVector<APInt> Muls;
+ for (unsigned I = 0, E = Values.size(); I < E; ++I)
+ Muls.push_back(Values[I] * RHS.Values[I]);
+ return GIConstant(Muls);
+ }
+ }
+}
+
+bool llvm::GIConstant::isZero() const {
+ switch (Kind) {
+ case GIConstantKind::Scalar:
+ return Value.isZero();
+ case GIConstantKind::ScalableVector:
+ return Value.isZero();
+ case GIConstantKind::FixedVector: {
+ for (const APInt &V : Values)
+ if (!V.isZero())
+ return false;
+ return true;
+ }
+ }
+}
+
+bool llvm::GIConstant::isOne() const {
+ switch (Kind) {
+ case GIConstantKind::Scalar:
+ return Value.isOne();
+ case GIConstantKind::ScalableVector:
+ return Value.isOne();
+ case GIConstantKind::FixedVector: {
+ for (const APInt &V : Values)
+ if (!V.isOne())
+ return false;
+ return true;
+ }
+ }
+}
+
+bool llvm::GIConstant::isTwo() const {
+ switch (Kind) {
+ case GIConstantKind::Scalar:
+ return Value.getLimitedValue() == 2;
+ case GIConstantKind::ScalableVector:
+ return Value.getLimitedValue() == 2;
+ case GIConstantKind::FixedVector: {
+ for (const APInt &V : Values)
+ if (V.getLimitedValue() != 2)
+ return false;
+ return true;
+ }
+ }
+}
+
+bool llvm::GIConstant::isAllOnes() const {
+ switch (Kind) {
+ case GIConstantKind::Scalar:
+ return Value.isAllOnes();
+ case GIConstantKind::ScalableVector:
+ return Value.isAllOnes();
+ case GIConstantKind::FixedVector: {
+ for (const APInt &V : Values)
+ if (!V.isAllOnes())
+ return false;
+ return true;
+ }
+ }
+}
+
APFloat llvm::GFConstant::getScalarValue() const {
assert(Kind == GFConstantKind::Scalar && "Expected scalar constant");
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
index cf6b2ce9c5341..87b47098a2ff2 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
@@ -32,6 +32,7 @@
#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
#include "llvm/CodeGen/GlobalISel/MIPatternMatch.h"
#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
+#include "llvm/CodeGen/GlobalISel/OptMIRBuilder.h"
#include "llvm/CodeGen/GlobalISel/Utils.h"
#include "llvm/CodeGen/MachineDominators.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
@@ -440,6 +441,9 @@ void applyCombineMulCMLT(MachineInstr &MI, MachineRegisterInfo &MRI,
class AArch64PostLegalizerCombinerImpl : public Combiner {
protected:
+ std::unique_ptr<MachineIRBuilder> Opt;
+ // hides Combiner::B
+ MachineIRBuilder &B;
const CombinerHelper Helper;
const AArch64PostLegalizerCombinerImplRuleConfig &RuleConfig;
const AArch64Subtarget &STI;
@@ -473,7 +477,9 @@ AArch64PostLegalizerCombinerImpl::AArch64PostLegalizerCombinerImpl(
const AArch64Subtarget &STI, MachineDominatorTree *MDT,
const LegalizerInfo *LI)
: Combiner(MF, CInfo...
[truncated]
|
|
✅ With the latest revision this PR passed the undef deprecator. |
Author
|
Ping. |
added 5 commits
March 29, 2025 17:21
It optimizes instructions while building them. It looks at the constness and values of its parameters to optimize them. It might build copies. In some cases partial constness or values are sufficient to optimize. The only context relied on are constants and their values. The builder will never use known bits to optimize. It always relies on legality before building. It uses undefness for optimization. The CSEMIRBuilder can build maybe illegal build vectors pass the legalizer without any means to verify. It is inconsistent whether to optimize fixed-length vectors. It never optimizes scalable vectors. The win of the new approach are separation of concern and correctness. TODO: move constant folding out of CSEMIRBuilder I put the new builder only into the post legalizer combiner to keep the patch small and show the demand for a legal builder. I like the GIConstant. It keeps the builder smaller and simpler.
Author
1 similar comment
Author
dzhidzhoev
reviewed
Mar 31, 2025
Member
dzhidzhoev
left a comment
dzhidzhoev
left a comment
There was a problem hiding this comment.
What's the original issue this PR is meant to solve?
Member
There was a problem hiding this comment.
Is this change relevant to the PR?
Author
There was a problem hiding this comment.
Probably not. It seems purely due to the check script.
Author
|
We have constant-folding in the CSEMIRbuilder, but it has no access to LegalizerInfo. On which targets is it legal to build the G_BUILD_VECTOR underneath? And I cannot you use CSEMIRBuilder without constant folding. |
Author
|
To give a bit more context, my dream is to remove constant folding from the CSEMIRBuilder and have a separate builder for constant folding. Please inspect the constructor of the new builder. The proposed builder uses undef, poison, and constant folding for optimizations. It delegates on failure to optimize to the CSEMIRBuilder. The main advertisement of the new builder is that always optimizes into legal gMIR. |
Contributor
|
I don't think this is necessary at all. We have combiners for doing anything more than basic constant folding. On top of that the comments in this PR could be more coherent. Finally, I don't really think GIConstant is appropriate for long term usage. It should really be an extension of GConstant IMO and probably shouldn't exist, but that's not the main issue in this PR. |
Author
|
I removed G_ADD, G_SUB, and G_MUL from the constant folding list in CSEMIRBuilder. It caused a lot of regressions. People depend on the constant folding in the builder. GConstant can only provide helpers for scalars. How do you constant fold vectors with out much burden for the user? Edit: Exploiting undef and poison might make the program one epsilon smaller. Combiners also rely on builders. |
Contributor
Perhaps, that to me indicates we have some missing optimizations in the combiner in order to make it functionally complete. Not every usage of MIRBuilder in a pipeline can be assumed to be CSE'ing.
As I said the GIConstant issue isn't the point here. And I'd recommend taking a look at our GitHub issues to find other more important fixes to do as your first contribution. |
arsenm
reviewed
Apr 1, 2025
Contributor
arsenm
left a comment
arsenm
left a comment
There was a problem hiding this comment.
We don't really want to be doing a lot of optimizations in the MIRBuilder. I always found the DAG behavior confusing, and is a major contributor to how slow the DAG is
The CSEMIRBuilder is a compile time hack only. We should only add anything new to MIRBuilders if there is measured evidence for a compile time improvement by duplicating an optimization here, as opposed to only having it in a combiner
Author
|
The addsubmul showed that the constant folding is beneficial in compile time. Later passes have less work to do. I stated above that CSEMIRBuilder has no access to LegalizerInfo and can build illegal instructions. That's why I propose a new builder that takes LegalizerInfo and inherits from CSEMIRBuilder. |
Contributor
Did you collect data? If you have very compelling data collected from running CTMark on AArch64 we can reconsider (assuming there's no other way to get that same improvement).
This is an orthogonal issue to the optimization question. Do you have a particular problem on a downstream target such that a build_vector is illegal but an instruction which generates the same vector type is legal? It seems like an unlikely edge case to me. |
Author
|
The combiner holds higher standards when it comes to legality. You must check for legality before building. The constant folding uses an optimistic approach. It will just work. For asymmetric opcodes like G_SITOFP, G_CTLZ, and G_ICMP I am not convinced that this is the right approach. |
Contributor
Did you decide this yourself? This is your first contribution to LLVM and you're going to use this tone? |
Author
|
CSEMIRBuilder constant folds G_SITOFP and G_UITOFP. The parameters switch between integers and floats, but more importantly the bit width may change. For all cast operations, the input and output differ. I would prefer to the query the legalizer before folding. mul(undef) combines into 0. My PR checks whether it is legal to write 0 into the destination. |
Contributor
Let me repeat my earlier question. Is this a real problem in practice? |
Author
|
I am confused by the term The new builder can also build into negation. Check or just works? |
Contributor
|
"in practice" means when you actually use GlobalISel to build software, do you hit issues? Or could you conceivably hit issues that affect real targets? |
Author
|
Should we remove them for compile time? We have Legalizers for each target that precisely state what they consider legal. Sapphire Rapids and Thumb are different. I am confused about the it just works mentality. What are the issues? Fail to select? Edit: I opened a PR with a strict constant folder. |
Contributor
I actually asked you a question twice now for which you haven't provided a straight answer. Judging from the redirection with another question I'm assuming that the answer is "no". I also asked you earlier about your claimed compile time improvement and you also didn't reply to that. Did you actually see a compile time improvement or was it theoretical?
As this is your first ever PR to LLVM I understand that you are unclear about the idea that we may trade off theoretical purity for pragmatism in some cases. You may be surprised to know that this actually happens throughout the compiler. There are some optimization corner cases (e.g. pointer provenance comes to mind) where a full theoretical fix in the past would have been to disable an entire class of optimizations, but we don't do that because it comes at an unacceptable cost of performance. And that's a case where you actually can construct a test case to show the problem. Here we don't even know if this can ever be an issue on any sane target. |
Contributor
|
Closing this PR as @john-stuart2 was deemed to be an alternate account of a previously banned individual. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It optimizes instructions while building them. It looks at the constness and values of its parameters to optimize them. It might build copies. In some cases partial constness or values are sufficient to optimize. The only context relied on are constants and their values. The builder will never use known bits to optimize. It always relies on legality before building. It uses undefness for optimization.
The CSEMIRBuilder has no access to LegalizerInfo and may build illegal instructions. It is inconsistent whether to optimize fixed-length vectors. It never optimizes scalable vectors.
I put the new builder only into the post legalizer combiner to keep the patch small and show the demand for a legal builder.