From 629db5d8e5a63a2fe533caab0bf019b62a2ab5be Mon Sep 17 00:00:00 2001 From: Daniel Sanders Date: Fri, 14 Dec 2018 17:50:14 +0000 Subject: [PATCH] [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 llvm-svn: 349167 --- .../llvm/CodeGen/GlobalISel/CombinerHelper.h | 13 +++-- .../include/llvm/CodeGen/GlobalISel/CombinerInfo.h | 11 ++++ .../llvm/CodeGen/GlobalISel/GISelChangeObserver.h | 29 +++++++--- .../llvm/CodeGen/GlobalISel/GISelWorkList.h | 54 ++++++++++++++----- llvm/lib/CodeGen/GlobalISel/CMakeLists.txt | 1 + llvm/lib/CodeGen/GlobalISel/Combiner.cpp | 61 ++++++++++++++++------ llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | 44 ++++++++++++---- .../lib/CodeGen/GlobalISel/GISelChangeObserver.cpp | 31 +++++++++++ llvm/lib/CodeGen/GlobalISel/Legalizer.cpp | 12 ++--- ...galizercombiner-extending-loads-cornercases.mir | 59 ++++++++++++++++----- .../CodeGen/GlobalISel/LegalizerHelperTest.cpp | 8 +-- 11 files changed, 251 insertions(+), 72 deletions(-) create mode 100644 llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp 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 GISelChangeObserver; 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 @@ public: /// 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 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 @@ namespace llvm { 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 class GISelWorkList { - SmallVector Worklist; - DenseMap WorklistMap; + MachineFunction *MF; + SmallVector Worklist; + DenseMap 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(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(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 @@ add_llvm_library(LLVMGlobalISel 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 @@ using namespace llvm; 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 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(MachineInstr &MI) override { + void changingInstr(const MachineInstr &MI) override { LLVM_DEBUG(dbgs() << "Changing: " << MI << "\n"); - WorkList.remove(&MI); + WorkList.insert(&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 @@ bool Combiner::combineMachineInstrs(MachineFunction &MF) { // 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 @@ bool Combiner::combineMachineInstrs(MachineFunction &MF) { // 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 @@ CombinerHelper::CombinerHelper(GISelChangeObserver &Observer, 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 @@ bool CombinerHelper::tryCombineCopy(MachineInstr &MI) { // 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 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) { // 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 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) { 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 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) { // 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 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) { // %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 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) { 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 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) { 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 @@ public: 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 @@ public: 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 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) { MachineRegisterInfo &MRI = MF.getRegInfo(); // Populate Insts - InstListTy InstList; - ArtifactListTy ArtifactList; + InstListTy InstList(&MF); + ArtifactListTy ArtifactList(&MF); ReversePostOrderTraversal 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 @@ body: | ; 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 @@ body: | %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 @@ namespace { 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, -- 2.7.4