[globalisel][combiner] Make the CombinerChangeObserver a MachineFunction::Delegate
Summary:
This allows us to register it with the MachineFunction delegate and be
notified automatically about erasure and creation of instructions. However,
we still need explicit notification for modifications such as those caused
by setReg() or replaceRegWith().
There is a catch with this though. The notification for creation is
delivered before any operands can be added. While appropriate for
scheduling combiner work. This is unfortunate for debug output since an
opcode by itself doesn't provide sufficient information on what happened.
As a result, the work list remembers the instructions (when debug output is
requested) and emits a more complete dump later.
Another nit is that the MachineFunction::Delegate provides const pointers
which is inconvenient since we want to use it to schedule future
modification. To resolve this GISelWorkList now has an optional pointer to
the MachineFunction which describes the scope of the work it is permitted
to schedule. If a given MachineInstr* is in this function then it is
permitted to schedule work to be performed on the MachineInstr's. An
alternative to this would be to remove the const from the
MachineFunction::Delegate interface, however delegates are not permitted
to modify the MachineInstr's they receive.
In addition to this, the observer has three interface changes.
* erasedInstr() is now erasingInstr() to indicate it is about to be erased
but still exists at the moment.
* changingInstr() and changedInstr() have been added to report changes
before and after they are made. This allows us to trace the changes
in the debug output.
* As a convenience changingAllUsesOfReg() and
finishedChangingAllUsesOfReg() will report changingInstr() and
changedInstr() for each use of a given register. This is primarily useful
for changes caused by MachineRegisterInfo::replaceRegWith()
With this in place, both combine rules have been updated to report their
changes to the observer.
Finally, make some cosmetic changes to the debug output and make Combiner
and CombinerHelp
Reviewers: aditya_nandakumar, bogner, volkan, rtereshin, javed.absar
Reviewed By: aditya_nandakumar
Subscribers: mgorny, rovka, kristof.beyls, llvm-commits
Differential Revision: https://reviews.llvm.org/D52947
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index 26b1cd5..6e9ac01 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -1,4 +1,4 @@
-//== llvm/CodeGen/GlobalISel/CombinerHelper.h -------------- -*- C++ -*-==//
+//===-- llvm/CodeGen/GlobalISel/CombinerHelper.h --------------*- C++ -*-===//
//
// The LLVM Compiler Infrastructure
//
@@ -24,17 +24,24 @@
class MachineIRBuilder;
class MachineRegisterInfo;
class MachineInstr;
+class MachineOperand;
class CombinerHelper {
MachineIRBuilder &Builder;
MachineRegisterInfo &MRI;
GISelChangeObserver &Observer;
- void scheduleForVisit(MachineInstr &MI);
-
public:
CombinerHelper(GISelChangeObserver &Observer, MachineIRBuilder &B);
+ /// MachineRegisterInfo::replaceRegWith() and inform the observer of the changes
+ void replaceRegWith(MachineRegisterInfo &MRI, unsigned FromReg, unsigned ToReg) const;
+
+ /// Replace a single register operand with a new register and inform the
+ /// observer of the changes.
+ void replaceRegOpWith(MachineRegisterInfo &MRI, MachineOperand &FromRegOp,
+ unsigned ToReg) const;
+
/// If \p MI is COPY, try to combine it.
/// Returns true if MI changed.
bool tryCombineCopy(MachineInstr &MI);
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerInfo.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerInfo.h
index d73aec9..d21aa3f 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerInfo.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerInfo.h
@@ -43,6 +43,17 @@
/// illegal ops that are created.
bool LegalizeIllegalOps; // TODO: Make use of this.
const LegalizerInfo *LInfo;
+
+ /// Attempt to combine instructions using MI as the root.
+ ///
+ /// Use Observer to report the creation, modification, and erasure of
+ /// instructions. GISelChangeObserver will automatically report certain
+ /// kinds of operations. These operations are:
+ /// * Instructions that are newly inserted into the MachineFunction
+ /// * Instructions that are erased from the MachineFunction.
+ ///
+ /// However, it is important to report instruction modification and this is
+ /// not automatic.
virtual bool combine(GISelChangeObserver &Observer, MachineInstr &MI,
MachineIRBuilder &B) const = 0;
};
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
index 6e86e3b..d21c733 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
@@ -1,5 +1,4 @@
-//== ----- llvm/CodeGen/GlobalISel/GISelChangeObserver.h ---------------------
-//== //
+//===----- llvm/CodeGen/GlobalISel/GISelChangeObserver.h ------------------===//
//
// The LLVM Compiler Infrastructure
//
@@ -15,25 +14,41 @@
#ifndef LLVM_CODEGEN_GLOBALISEL_GISELCHANGEOBSERVER_H
#define LLVM_CODEGEN_GLOBALISEL_GISELCHANGEOBSERVER_H
+#include "llvm/ADT/SmallPtrSet.h"
+
namespace llvm {
+class MachineInstr;
+class MachineRegisterInfo;
+
/// Abstract class that contains various methods for clients to notify about
/// changes. This should be the preferred way for APIs to notify changes.
/// Typically calling erasingInstr/createdInstr multiple times should not affect
/// the result. The observer would likely need to check if it was already
/// notified earlier (consider using GISelWorkList).
-class MachineInstr;
class GISelChangeObserver {
+ SmallPtrSet<MachineInstr *, 4> ChangingAllUsesOfReg;
+
public:
virtual ~GISelChangeObserver() {}
/// An instruction is about to be erased.
- virtual void erasingInstr(MachineInstr &MI) = 0;
+ virtual void erasingInstr(const MachineInstr &MI) = 0;
/// An instruction was created and inserted into the function.
- virtual void createdInstr(MachineInstr &MI) = 0;
+ virtual void createdInstr(const MachineInstr &MI) = 0;
/// This instruction is about to be mutated in some way.
- virtual void changingInstr(MachineInstr &MI) = 0;
+ virtual void changingInstr(const MachineInstr &MI) = 0;
/// This instruction was mutated in some way.
- virtual void changedInstr(MachineInstr &MI) = 0;
+ virtual void changedInstr(const MachineInstr &MI) = 0;
+
+ /// All the instructions using the given register are being changed.
+ /// For convenience, finishedChangingAllUsesOfReg() will report the completion
+ /// of the changes. The use list may change between this call and
+ /// finishedChangingAllUsesOfReg().
+ void changingAllUsesOfReg(const MachineRegisterInfo &MRI, unsigned Reg);
+ /// All instructions reported as changing by changingAllUsesOfReg() have
+ /// finished being changed.
+ void finishedChangingAllUsesOfReg();
+
};
} // namespace llvm
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
index 167905d..ad41990 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
@@ -18,32 +18,60 @@
class MachineInstr;
-// Worklist which mostly works similar to InstCombineWorkList, but on MachineInstrs.
-// The main difference with something like a SetVector is that erasing an element doesn't
-// move all elements over one place - instead just nulls out the element of the vector.
-// FIXME: Does it make sense to factor out common code with the instcombinerWorkList?
+// Worklist which mostly works similar to InstCombineWorkList, but on
+// MachineInstrs. The main difference with something like a SetVector is that
+// erasing an element doesn't move all elements over one place - instead just
+// nulls out the element of the vector.
+//
+// This worklist operates on instructions within a particular function. This is
+// important for acquiring the rights to modify/replace instructions a
+// GISelChangeObserver reports as the observer doesn't have the right to make
+// changes to the instructions it sees so we use our access to the
+// MachineFunction to establish that it's ok to add a given instruction to the
+// worklist.
+//
+// FIXME: Does it make sense to factor out common code with the
+// instcombinerWorkList?
template<unsigned N>
class GISelWorkList {
- SmallVector<MachineInstr*, N> Worklist;
- DenseMap<MachineInstr*, unsigned> WorklistMap;
+ MachineFunction *MF;
+ SmallVector<MachineInstr *, N> Worklist;
+ DenseMap<MachineInstr *, unsigned> WorklistMap;
public:
- GISelWorkList() = default;
+ GISelWorkList(MachineFunction *MF) : MF(MF) {}
bool empty() const { return WorklistMap.empty(); }
unsigned size() const { return WorklistMap.size(); }
- /// Add - Add the specified instruction to the worklist if it isn't already
- /// in it.
+ /// Add the specified instruction to the worklist if it isn't already in it.
void insert(MachineInstr *I) {
- if (WorklistMap.try_emplace(I, Worklist.size()).second) {
- Worklist.push_back(I);
+ // It would be safe to add this instruction to the worklist regardless but
+ // for consistency with the const version, check that the instruction we're
+ // adding would have been accepted if we were given a const pointer instead.
+ insert(const_cast<const MachineInstr *>(I));
+ }
+
+ void insert(const MachineInstr *I) {
+ // Confirm we'd be able to find the non-const pointer we want to schedule if
+ // we wanted to. We have the right to schedule work that may modify any
+ // instruction in MF.
+ assert(I->getParent() && "Expected parent BB");
+ assert(I->getParent()->getParent() && "Expected parent function");
+ assert((!MF || I->getParent()->getParent() == MF) &&
+ "Expected parent function to be current function or not given");
+
+ // But don't actually do the search since we can derive it from the const
+ // pointer.
+ MachineInstr *NonConstI = const_cast<MachineInstr *>(I);
+ if (WorklistMap.try_emplace(NonConstI, Worklist.size()).second) {
+ Worklist.push_back(NonConstI);
}
}
- /// Remove - remove I from the worklist if it exists.
- void remove(MachineInstr *I) {
+ /// Remove I from the worklist if it exists.
+ void remove(const MachineInstr *I) {
auto It = WorklistMap.find(I);
if (It == WorklistMap.end()) return; // Not in worklist.
diff --git a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
index 4c1da375..5f13692 100644
--- a/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
+++ b/llvm/lib/CodeGen/GlobalISel/CMakeLists.txt
@@ -3,6 +3,7 @@
GlobalISel.cpp
Combiner.cpp
CombinerHelper.cpp
+ GISelChangeObserver.cpp
IRTranslator.cpp
InstructionSelect.cpp
InstructionSelector.cpp
diff --git a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
index a2f220a..90fd54e 100644
--- a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
@@ -1,4 +1,4 @@
-//===-- lib/CodeGen/GlobalISel/GICombiner.cpp -----------------------===//
+//===-- lib/CodeGen/GlobalISel/Combiner.cpp -------------------------------===//
//
// The LLVM Compiler Infrastructure
//
@@ -29,36 +29,62 @@
namespace {
/// This class acts as the glue the joins the CombinerHelper to the overall
/// Combine algorithm. The CombinerHelper is intended to report the
-/// modifications it makes to the MIR to the CombinerChangeObserver and the
+/// modifications it makes to the MIR to the GISelChangeObserver and the
/// observer subclass will act on these events. In this case, instruction
/// erasure will cancel any future visits to the erased instruction and
/// instruction creation will schedule that instruction for a future visit.
/// Other Combiner implementations may require more complex behaviour from
-/// their CombinerChangeObserver subclass.
-class WorkListMaintainer : public GISelChangeObserver {
+/// their GISelChangeObserver subclass.
+class WorkListMaintainer : public GISelChangeObserver,
+ public MachineFunction::Delegate {
using WorkListTy = GISelWorkList<512>;
+ MachineFunction &MF;
WorkListTy &WorkList;
+ /// The instructions that have been created but we want to report once they
+ /// have their operands. This is only maintained if debug output is requested.
+ SmallPtrSet<const MachineInstr *, 4> CreatedInstrs;
public:
- WorkListMaintainer(WorkListTy &WorkList) : WorkList(WorkList) {}
- virtual ~WorkListMaintainer() {}
+ WorkListMaintainer(MachineFunction &MF, WorkListTy &WorkList)
+ : GISelChangeObserver(), MF(MF), WorkList(WorkList) {
+ MF.setDelegate(this);
+ }
+ virtual ~WorkListMaintainer() {
+ MF.resetDelegate(this);
+ }
- void erasingInstr(MachineInstr &MI) override {
+ void erasingInstr(const MachineInstr &MI) override {
LLVM_DEBUG(dbgs() << "Erased: " << MI << "\n");
WorkList.remove(&MI);
}
- void createdInstr(MachineInstr &MI) override {
- LLVM_DEBUG(dbgs() << "Created: " << MI << "\n");
+ void createdInstr(const MachineInstr &MI) override {
+ LLVM_DEBUG(dbgs() << "Creating: " << MI << "\n");
+ WorkList.insert(&MI);
+ LLVM_DEBUG(CreatedInstrs.insert(&MI));
+ }
+ void changingInstr(const MachineInstr &MI) override {
+ LLVM_DEBUG(dbgs() << "Changing: " << MI << "\n");
WorkList.insert(&MI);
}
- void changingInstr(MachineInstr &MI) override {
- LLVM_DEBUG(dbgs() << "Changing: " << MI << "\n");
- WorkList.remove(&MI);
- }
- // Currently changed conservatively assumes erased.
- void changedInstr(MachineInstr &MI) override {
+ void changedInstr(const MachineInstr &MI) override {
LLVM_DEBUG(dbgs() << "Changed: " << MI << "\n");
- WorkList.remove(&MI);
+ WorkList.insert(&MI);
+ }
+
+ void reportFullyCreatedInstrs() {
+ LLVM_DEBUG(for (const auto *MI
+ : CreatedInstrs) {
+ dbgs() << "Created: ";
+ MI->print(dbgs());
+ });
+ LLVM_DEBUG(CreatedInstrs.clear());
+ }
+
+ void MF_HandleInsertion(const MachineInstr &MI) override {
+ createdInstr(MI);
+ }
+ void MF_HandleRemoval(const MachineInstr &MI) override {
+ erasingInstr(MI);
}
};
}
@@ -90,8 +116,8 @@
// insert with list bottom up, so while we pop_back_val, we'll traverse top
// down RPOT.
Changed = false;
- GISelWorkList<512> WorkList;
- WorkListMaintainer Observer(WorkList);
+ GISelWorkList<512> WorkList(&MF);
+ WorkListMaintainer Observer(MF, WorkList);
for (MachineBasicBlock *MBB : post_order(&MF)) {
if (MBB->empty())
continue;
@@ -110,8 +136,9 @@
// Main Loop. Process the instructions here.
while (!WorkList.empty()) {
MachineInstr *CurrInst = WorkList.pop_back_val();
- LLVM_DEBUG(dbgs() << "Try combining " << *CurrInst << "\n";);
+ LLVM_DEBUG(dbgs() << "\nTry combining " << *CurrInst;);
Changed |= CInfo.combine(Observer, *CurrInst, Builder);
+ Observer.reportFullyCreatedInstrs();
}
MFChanged |= Changed;
} while (Changed);
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 6018c59..b1c5670 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -1,4 +1,4 @@
-//== ---lib/CodeGen/GlobalISel/GICombinerHelper.cpp --------------------- == //
+//===-- lib/CodeGen/GlobalISel/GICombinerHelper.cpp -----------------------===//
//
// The LLVM Compiler Infrastructure
//
@@ -15,7 +15,7 @@
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/TargetInstrInfo.h"
-#define DEBUG_TYPE "gi-combine"
+#define DEBUG_TYPE "gi-combiner"
using namespace llvm;
@@ -23,8 +23,27 @@
MachineIRBuilder &B)
: Builder(B), MRI(Builder.getMF().getRegInfo()), Observer(Observer) {}
-void CombinerHelper::scheduleForVisit(MachineInstr &MI) {
- Observer.createdInstr(MI);
+void CombinerHelper::replaceRegWith(MachineRegisterInfo &MRI, unsigned FromReg,
+ unsigned ToReg) const {
+ Observer.changingAllUsesOfReg(MRI, FromReg);
+
+ if (MRI.constrainRegAttrs(ToReg, FromReg))
+ MRI.replaceRegWith(FromReg, ToReg);
+ else
+ Builder.buildCopy(ToReg, FromReg);
+
+ Observer.finishedChangingAllUsesOfReg();
+}
+
+void CombinerHelper::replaceRegOpWith(MachineRegisterInfo &MRI,
+ MachineOperand &FromRegOp,
+ unsigned ToReg) const {
+ assert(FromRegOp.getParent() && "Expected an operand in an MI");
+ Observer.changingInstr(*FromRegOp.getParent());
+
+ FromRegOp.setReg(ToReg);
+
+ Observer.changedInstr(*FromRegOp.getParent());
}
bool CombinerHelper::tryCombineCopy(MachineInstr &MI) {
@@ -38,7 +57,7 @@
// a(sx) = COPY b(sx) -> Replace all uses of a with b.
if (DstTy.isValid() && SrcTy.isValid() && DstTy == SrcTy) {
MI.eraseFromParent();
- MRI.replaceRegWith(DstReg, SrcReg);
+ replaceRegWith(MRI, DstReg, SrcReg);
return true;
}
return false;
@@ -191,8 +210,11 @@
// type since by definition the result of an extend is larger.
assert(Preferred.Ty != LoadValueTy && "Extending to same type?");
+ LLVM_DEBUG(dbgs() << "Preferred use is: " << *Preferred.MI);
+
// Rewrite the load to the chosen extending load.
unsigned ChosenDstReg = Preferred.MI->getOperand(0).getReg();
+ Observer.changingInstr(MI);
MI.setDesc(
Builder.getTII().get(Preferred.ExtendOpcode == TargetOpcode::G_SEXT
? TargetOpcode::G_SEXTLOAD
@@ -211,7 +233,7 @@
if (UseMI->getOpcode() == Preferred.ExtendOpcode ||
UseMI->getOpcode() == TargetOpcode::G_ANYEXT) {
unsigned UseDstReg = UseMI->getOperand(0).getReg();
- unsigned UseSrcReg = UseMI->getOperand(1).getReg();
+ MachineOperand &UseSrcMO = UseMI->getOperand(1);
const LLT &UseDstTy = MRI.getType(UseDstReg);
if (UseDstReg != ChosenDstReg) {
if (Preferred.Ty == UseDstTy) {
@@ -224,7 +246,7 @@
// rewrites to:
// %2:_(s32) = G_SEXTLOAD ...
// ... = ... %2(s32)
- MRI.replaceRegWith(UseDstReg, ChosenDstReg);
+ replaceRegWith(MRI, UseDstReg, ChosenDstReg);
ScheduleForErase.push_back(UseMO.getParent());
} else if (Preferred.Ty.getSizeInBits() < UseDstTy.getSizeInBits()) {
// If the preferred size is smaller, then keep the extend but extend
@@ -237,7 +259,7 @@
// %2:_(s32) = G_SEXTLOAD ...
// %3:_(s64) = G_ANYEXT %2:_(s32)
// ... = ... %3(s64)
- MRI.replaceRegWith(UseSrcReg, ChosenDstReg);
+ replaceRegOpWith(MRI, UseSrcMO, ChosenDstReg);
} else {
// If the preferred size is large, then insert a truncate. For
// example:
@@ -284,7 +306,9 @@
MachineInstr *PreviouslyEmitted = EmittedInsns.lookup(InsertIntoBB);
if (PreviouslyEmitted) {
+ Observer.changingInstr(*UseMO->getParent());
UseMO->setReg(PreviouslyEmitted->getOperand(0).getReg());
+ Observer.changedInstr(*UseMO->getParent());
continue;
}
@@ -292,14 +316,14 @@
unsigned NewDstReg = MRI.cloneVirtualRegister(MI.getOperand(0).getReg());
MachineInstr *NewMI = Builder.buildTrunc(NewDstReg, ChosenDstReg);
EmittedInsns[InsertIntoBB] = NewMI;
- UseMO->setReg(NewDstReg);
- Observer.createdInstr(*NewMI);
+ replaceRegOpWith(MRI, *UseMO, NewDstReg);
}
for (auto &EraseMI : ScheduleForErase) {
Observer.erasingInstr(*EraseMI);
EraseMI->eraseFromParent();
}
MI.getOperand(0).setReg(ChosenDstReg);
+ Observer.changedInstr(MI);
return true;
}
diff --git a/llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp b/llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp
new file mode 100644
index 0000000..993a919
--- /dev/null
+++ b/llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp
@@ -0,0 +1,31 @@
+//===-- lib/CodeGen/GlobalISel/GISelChangeObserver.cpp --------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file constains common code to combine machine functions at generic
+// level.
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
+
+using namespace llvm;
+
+void GISelChangeObserver::changingAllUsesOfReg(
+ const MachineRegisterInfo &MRI, unsigned Reg) {
+ for (auto &ChangingMI : MRI.use_instructions(Reg)) {
+ changingInstr(ChangingMI);
+ ChangingAllUsesOfReg.insert(&ChangingMI);
+ }
+}
+
+void GISelChangeObserver::finishedChangingAllUsesOfReg() {
+ for (auto *ChangedMI : ChangingAllUsesOfReg)
+ changedInstr(*ChangedMI);
+}
+
diff --git a/llvm/lib/CodeGen/GlobalISel/Legalizer.cpp b/llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
index 74d592b..df2dcac 100644
--- a/llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
@@ -81,7 +81,7 @@
LegalizerWorkListManager(InstListTy &Insts, ArtifactListTy &Arts)
: InstList(Insts), ArtifactList(Arts) {}
- void createdInstr(MachineInstr &MI) override {
+ void createdInstr(const MachineInstr &MI) override {
// Only legalize pre-isel generic instructions.
// Legalization process could generate Target specific pseudo
// instructions with generic types. Don't record them
@@ -94,17 +94,17 @@
LLVM_DEBUG(dbgs() << ".. .. New MI: " << MI);
}
- void erasingInstr(MachineInstr &MI) override {
+ void erasingInstr(const MachineInstr &MI) override {
LLVM_DEBUG(dbgs() << ".. .. Erasing: " << MI);
InstList.remove(&MI);
ArtifactList.remove(&MI);
}
- void changingInstr(MachineInstr &MI) override {
+ void changingInstr(const MachineInstr &MI) override {
LLVM_DEBUG(dbgs() << ".. .. Changing MI: " << MI);
}
- void changedInstr(MachineInstr &MI) override {
+ void changedInstr(const MachineInstr &MI) override {
// When insts change, we want to revisit them to legalize them again.
// We'll consider them the same as created.
LLVM_DEBUG(dbgs() << ".. .. Changed MI: " << MI);
@@ -126,8 +126,8 @@
MachineRegisterInfo &MRI = MF.getRegInfo();
// Populate Insts
- InstListTy InstList;
- ArtifactListTy ArtifactList;
+ InstListTy InstList(&MF);
+ ArtifactListTy ArtifactList(&MF);
ReversePostOrderTraversal<MachineFunction *> RPOT(&MF);
// Perform legalization bottom up so we can DCE as we legalize.
// Traverse BB in RPOT and within each basic block, add insts top down,
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir
index 8a78ed9..4601808 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-cornercases.mir
@@ -1,4 +1,7 @@
# RUN: llc -O0 -run-pass=aarch64-prelegalizer-combiner -global-isel %s -o - | FileCheck %s
+# RUN: llc -O0 -run-pass=aarch64-prelegalizer-combiner -global-isel %s -o - \
+# RUN: -debug-only=aarch64-prelegalizer-combiner,gi-combiner 2>&1 >/dev/null \
+# RUN: | FileCheck %s --check-prefix=CHECK-WORKLIST
--- |
target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
@@ -102,6 +105,23 @@
; CHECK: $w1 = COPY [[T6]](s32)
$w0 = COPY %3
$w1 = COPY %9
+
+# Check that we report the correct modifications to the observer. This acts as
+# a test of the debug output and a test.
+#
+# CHECK-WORKLIST-LABEL: Generic MI Combiner for: multiple_copies
+# CHECK-WORKLIST: Try combining [[IN0:%[0-9]+]]:_(s8) = G_LOAD [[IN1:%[0-9]+]]:_(p0) :: (load 1 from %ir.addr)
+# CHECK-WORKLIST: Preferred use is: [[IN2:%[0-9]+]]:_(s32) = G_SEXT [[IN0]]:_(s8)
+# CHECK-WORKLIST-DAG: Changing: [[IN0]]:_(s8) = G_LOAD [[IN1]]:_(p0) :: (load 1 from %ir.addr)
+# CHECK-WORKLIST-DAG: Changing: [[IN3:%[0-9]+]]:_(s8) = G_ADD [[IN0]]:_, [[IN4:%[0-9]+]]:_
+# CHECK-WORKLIST-DAG: Changed: [[IN3]]:_(s8) = G_ADD [[NEW1:%[0-9]+]]:_, [[IN4]]:_
+# CHECK-WORKLIST-DAG: Changing: [[IN5:%[0-9]+]]:_(s8) = G_SUB [[IN0]]:_, [[IN6:%[0-9]+]]:_
+# CHECK-WORKLIST-DAG: Changed: [[IN5]]:_(s8) = G_SUB [[NEW2:%[0-9]+]]:_, [[IN6]]:_
+# CHECK-WORKLIST-DAG: Erased: [[IN2]]:_(s32) = G_SEXT [[IN0]]:_(s8)
+# CHECK-WORKLIST-DAG: Changed: [[IN2]]:_(s32) = G_SEXTLOAD [[IN1]]:_(p0) :: (load 1 from %ir.addr)
+# CHECK-WORKLIST-DAG: Created: [[NEW1]]:_(s8) = G_TRUNC [[IN2]]:_(s32)
+# CHECK-WORKLIST-DAG: Created: [[NEW2]]:_(s8) = G_TRUNC [[IN2]]:_(s32)
+# CHECK-WORKLIST: Try combining
...
---
@@ -157,37 +177,52 @@
%0:_(p0) = COPY $x0
%1:_(s32) = COPY $w1
%2:_(s8) = G_LOAD %0 :: (load 1 from %ir.addr)
- %3:_(s32) = G_SEXT %2
- %4:_(s32) = G_CONSTANT i32 1
- %5:_(s1) = G_ICMP intpred(ne), %1:_(s32), %4:_
- G_BRCOND %5:_(s1), %bb.1
+ %3:_(s32) = G_CONSTANT i32 1
+ %4:_(s1) = G_ICMP intpred(ne), %1:_(s32), %3:_
+ G_BRCOND %4:_(s1), %bb.1
G_BR %bb.2.else
bb.1.if:
; CHECK: bb.1.if:
successors: %bb.3(0x80000000)
- %10:_(s8) = G_CONSTANT i8 1
+ %5:_(s8) = G_CONSTANT i8 1
; CHECK: [[T1:%[0-9]+]]:_(s8) = G_TRUNC [[T0]](s32)
- %6:_(s8) = G_ADD %2, %10
+ %6:_(s8) = G_ADD %2, %5
; CHECK: [[T2:%[0-9]+]]:_(s8) = G_ADD [[T1]], {{.*}}
G_BR %bb.3.exit
bb.2.else:
; CHECK: bb.2.else:
successors: %bb.3(0x80000000)
- %11:_(s8) = G_CONSTANT i8 1
+ %7:_(s8) = G_CONSTANT i8 1
; CHECK: [[T3:%[0-9]+]]:_(s8) = G_TRUNC [[T0]](s32)
- %7:_(s8) = G_SUB %2, %11
+ %8:_(s8) = G_SUB %2, %7
; CHECK: [[T4:%[0-9]+]]:_(s8) = G_SUB [[T3]], {{.*}}
G_BR %bb.3.exit
bb.3.exit:
; CHECK: bb.3.exit:
- %8:_(s8) = G_PHI %6:_(s8), %bb.1, %7:_(s8), %bb.2
+ %9:_(s8) = G_PHI %6:_(s8), %bb.1, %8:_(s8), %bb.2
; CHECK: [[T5:%[0-9]+]]:_(s8) = G_PHI [[T2]](s8), %bb.1, [[T4]](s8)
- %9:_(s32) = G_ZEXT %8
+ %10:_(s32) = G_SEXT %2
+ %11:_(s32) = G_ZEXT %9
; CHECK: [[T6:%[0-9]+]]:_(s32) = G_ZEXT [[T5]](s8)
; CHECK: $w0 = COPY [[T0]](s32)
; CHECK: $w1 = COPY [[T6]](s32)
- $w0 = COPY %3
- $w1 = COPY %9
+ $w0 = COPY %10
+ $w1 = COPY %11
+# CHECK-WORKLIST-LABEL: Generic MI Combiner for: sink_to_phi_nondominating
+# CHECK-WORKLIST: Try combining [[IN0:%[0-9]+]]:_(s8) = G_LOAD [[IN1:%[0-9]+]]:_(p0) :: (load 1 from %ir.addr)
+# CHECK-WORKLIST: Preferred use is: [[IN2:%[0-9]+]]:_(s32) = G_SEXT [[IN0]]:_(s8)
+# CHECK-WORKLIST-DAG: Changing: [[IN0]]:_(s8) = G_LOAD [[IN1]]:_(p0) :: (load 1 from %ir.addr)
+# CHECK-WORKLIST-DAG: Creating: G_TRUNC
+# CHECK-WORKLIST-DAG: Changing: [[IN3:%[0-9]+]]:_(s8) = G_ADD [[IN0]]:_, [[IN4:%[0-9]+]]:_
+# CHECK-WORKLIST-DAG: Changed: [[IN3]]:_(s8) = G_ADD [[OUT1:%[0-9]+]]:_, [[IN4]]:_
+# CHECK-WORKLIST-DAG: Creating: G_TRUNC
+# CHECK-WORKLIST-DAG: Changing: [[IN5:%[0-9]+]]:_(s8) = G_SUB [[IN0]]:_, [[IN6:%[0-9]+]]:_
+# CHECK-WORKLIST-DAG: Changed: [[IN5]]:_(s8) = G_SUB [[OUT2:%[0-9]+]]:_, [[IN6]]:_
+# CHECK-WORKLIST-DAG: Erased: [[IN2]]:_(s32) = G_SEXT [[IN0]]:_(s8)
+# CHECK-WORKLIST-DAG: Changed: [[IN2]]:_(s32) = G_SEXTLOAD [[IN1]]:_(p0) :: (load 1 from %ir.addr)
+# CHECK-WORKLIST-DAG: Created: [[OUT1]]:_(s8) = G_TRUNC [[IN2]]:_(s32)
+# CHECK-WORKLIST-DAG: Created: [[OUT2]]:_(s8) = G_TRUNC [[IN2]]:_(s32)
+# CHECK-WORKLIST: Try combining
...
---
diff --git a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
index 38fb58e..ee84aef 100644
--- a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
@@ -13,10 +13,10 @@
class DummyGISelObserver : public GISelChangeObserver {
public:
- void changingInstr(MachineInstr &MI) override {}
- void changedInstr(MachineInstr &MI) override {}
- void createdInstr(MachineInstr &MI) override {}
- void erasingInstr(MachineInstr &MI) override {}
+ void changingInstr(const MachineInstr &MI) override {}
+ void changedInstr(const MachineInstr &MI) override {}
+ void createdInstr(const MachineInstr &MI) override {}
+ void erasingInstr(const MachineInstr &MI) override {}
};
// Test CTTZ expansion when CTTZ_ZERO_UNDEF is legal or custom,
