From a22edc82cab86be4cb8876da1e6e78f82bb47a3e Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Thu, 10 Jan 2008 23:08:24 +0000 Subject: [PATCH] Simplify the side effect stuff a bit more and make licm/sinking both work right according to the new flags. This removes the TII::isReallySideEffectFree predicate, and adds TII::isInvariantLoad. It removes NeverHasSideEffects+MayHaveSideEffects and adds UnmodeledSideEffects as machine instr flags. Now the clients can decide everything they need. I think isRematerializable can be implemented in terms of the flags we have now, though I will let others tackle that. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@45843 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Target/TargetInstrDesc.h | 43 ++++++++------------------- include/llvm/Target/TargetInstrInfo.h | 29 +++++++----------- lib/CodeGen/MachineLICM.cpp | 27 +++++++++++++---- lib/CodeGen/MachineSink.cpp | 24 ++++++++------- lib/Target/X86/X86InstrInfo.cpp | 23 +++++++------- lib/Target/X86/X86InstrInfo.h | 2 +- utils/TableGen/InstrInfoEmitter.cpp | 3 +- 7 files changed, 74 insertions(+), 77 deletions(-) diff --git a/include/llvm/Target/TargetInstrDesc.h b/include/llvm/Target/TargetInstrDesc.h index ded965876e9..34e295cfb63 100644 --- a/include/llvm/Target/TargetInstrDesc.h +++ b/include/llvm/Target/TargetInstrDesc.h @@ -92,8 +92,7 @@ namespace TID { SimpleLoad, MayLoad, MayStore, - NeverHasSideEffects, - MayHaveSideEffects, + UnmodeledSideEffects, Commutable, ConvertibleTo3Addr, UsesCustomDAGSchedInserter, @@ -326,37 +325,21 @@ public: return Flags & (1 << TID::MayStore); } - /// hasNoSideEffects - Return true if all instances of this instruction are - /// guaranteed to have no side effects other than: - /// 1. The register operands that are def/used by the MachineInstr. - /// 2. Registers that are implicitly def/used by the MachineInstr. - /// 3. Memory Accesses captured by mayLoad() or mayStore(). + /// hasUnmodeledSideEffects - Return true if this instruction has side + /// effects that are not modeled by other flags. This does not return true + /// for instructions whose effects are captured by: /// - /// Examples of other side effects would be calling a function, modifying - /// 'invisible' machine state like a control register, etc. + /// 1. Their operand list and implicit definition/use list. Register use/def + /// info is explicit for instructions. + /// 2. Memory accesses. Use mayLoad/mayStore. + /// 3. Calling, branching, returning: use isCall/isReturn/isBranch. /// - /// If some instances of this instruction are side-effect free but others are - /// not, the hasConditionalSideEffects() property should return true, not this - /// one. + /// Examples of side effects would be modifying 'invisible' machine state like + /// a control register, flushing a cache, modifying a register invisible to + /// LLVM, etc. /// - /// Note that you should not call this method directly, instead, call the - /// TargetInstrInfo::hasUnmodelledSideEffects method, which handles analysis - /// of the machine instruction. - bool hasNoSideEffects() const { - return Flags & (1 << TID::NeverHasSideEffects); - } - - /// hasConditionalSideEffects - Return true if some instances of this - /// instruction are guaranteed to have no side effects other than those listed - /// for hasNoSideEffects(). To determine whether a specific machineinstr has - /// side effects, the TargetInstrInfo::isReallySideEffectFree virtual method - /// is invoked to decide. - /// - /// Note that you should not call this method directly, instead, call the - /// TargetInstrInfo::hasUnmodelledSideEffects method, which handles analysis - /// of the machine instruction. - bool hasConditionalSideEffects() const { - return Flags & (1 << TID::MayHaveSideEffects); + bool hasUnmodeledSideEffects() const { + return Flags & (1 << TID::UnmodeledSideEffects); } //===--------------------------------------------------------------------===// diff --git a/include/llvm/Target/TargetInstrInfo.h b/include/llvm/Target/TargetInstrInfo.h index 6035f33794b..e62104c9211 100644 --- a/include/llvm/Target/TargetInstrInfo.h +++ b/include/llvm/Target/TargetInstrInfo.h @@ -69,15 +69,6 @@ public: isReallyTriviallyReMaterializable(MI); } - /// hasUnmodelledSideEffects - Returns true if the instruction has side - /// effects that are not captured by any operands of the instruction or other - /// flags. - bool hasUnmodelledSideEffects(MachineInstr *MI) const { - const TargetInstrDesc &TID = MI->getDesc(); - if (TID.hasNoSideEffects()) return false; - if (!TID.hasConditionalSideEffects()) return true; - return !isReallySideEffectFree(MI); // May have side effects - } protected: /// isReallyTriviallyReMaterializable - For instructions with opcodes for /// which the M_REMATERIALIZABLE flag is set, this function tests whether the @@ -91,15 +82,6 @@ protected: return true; } - /// isReallySideEffectFree - If the M_MAY_HAVE_SIDE_EFFECTS flag is set, this - /// method is called to determine if the specific instance of this - /// instruction has side effects. This is useful in cases of instructions, - /// like loads, which generally always have side effects. A load from a - /// constant pool doesn't have side effects, though. So we need to - /// differentiate it from the general case. - virtual bool isReallySideEffectFree(MachineInstr *MI) const { - return false; - } public: /// Return true if the instruction is a register to register move /// and leave the source and dest operands in the passed parameters. @@ -127,6 +109,17 @@ public: return 0; } + /// isInvariantLoad - Return true if the specified instruction (which is + /// marked mayLoad) is loading from a location whose value is invariant across + /// the function. For example, loading a value from the constant pool or from + /// from the argument area of a function if it does not change. This should + /// only return true of *all* loads the instruction does are invariant (if it + /// does multiple loads). + virtual bool isInvariantLoad(MachineInstr *MI) const { + return false; + } + + /// convertToThreeAddress - This method must be implemented by targets that /// set the M_CONVERTIBLE_TO_3_ADDR flag. When this flag is set, the target /// may be able to convert a two-address instruction into one or more true diff --git a/lib/CodeGen/MachineLICM.cpp b/lib/CodeGen/MachineLICM.cpp index 04211aba97c..92256eb3c2e 100644 --- a/lib/CodeGen/MachineLICM.cpp +++ b/lib/CodeGen/MachineLICM.cpp @@ -223,6 +223,26 @@ void MachineLICM::HoistRegion(MachineDomTreeNode *N) { /// effects that aren't captured by the operands or other flags. /// bool MachineLICM::IsLoopInvariantInst(MachineInstr &I) { + const TargetInstrDesc &TID = I.getDesc(); + + // Ignore stuff that we obviously can't hoist. + if (TID.mayStore() || TID.isCall() || TID.isReturn() || TID.isBranch() || + TID.hasUnmodeledSideEffects()) + return false; + + if (TID.mayLoad()) { + // Okay, this instruction does a load. As a refinement, allow the target + // to decide whether the loaded value is actually a constant. If so, we + // can actually use it as a load. + if (!TII->isInvariantLoad(&I)) { + // FIXME: we should be able to sink loads with no other side effects if + // there is nothing that can change memory from here until the end of + // block. This is a trivial form of alias analysis. + return false; + } + } + + DEBUG({ DOUT << "--- Checking if we can hoist " << I; if (I.getDesc().getImplicitUses()) { @@ -243,8 +263,8 @@ bool MachineLICM::IsLoopInvariantInst(MachineInstr &I) { DOUT << " -> " << MRI->getName(*ImpDefs) << "\n"; } - if (TII->hasUnmodelledSideEffects(&I)) - DOUT << " * Instruction has side effects.\n"; + //if (TII->hasUnmodelledSideEffects(&I)) + //DOUT << " * Instruction has side effects.\n"; }); // The instruction is loop invariant if all of its operands are loop-invariant @@ -268,9 +288,6 @@ bool MachineLICM::IsLoopInvariantInst(MachineInstr &I) { return false; } - // Don't hoist something that has unmodelled side effects. - if (TII->hasUnmodelledSideEffects(&I)) return false; - // If we got this far, the instruction is loop invariant! return true; } diff --git a/lib/CodeGen/MachineSink.cpp b/lib/CodeGen/MachineSink.cpp index b83d844a162..dc3e364e8a4 100644 --- a/lib/CodeGen/MachineSink.cpp +++ b/lib/CodeGen/MachineSink.cpp @@ -133,19 +133,21 @@ bool MachineSinking::SinkInstruction(MachineInstr *MI) { const TargetInstrDesc &TID = MI->getDesc(); // Ignore stuff that we obviously can't sink. - if (TID.mayStore() || TID.isCall() || TID.isReturn() || TID.isBranch()) + if (TID.mayStore() || TID.isCall() || TID.isReturn() || TID.isBranch() || + TID.hasUnmodeledSideEffects()) return false; - if (TID.mayLoad()) - return false; - - // Don't sink things with side-effects we don't understand. - if (TII->hasUnmodelledSideEffects(MI)) - return false; - - // FIXME: we should be able to sink loads with no other side effects if there - // is nothing that can change memory from here until the end of block. This - // is a trivial form of alias analysis. + if (TID.mayLoad()) { + // Okay, this instruction does a load. As a refinement, allow the target + // to decide whether the loaded value is actually a constant. If so, we + // can actually use it as a load. + if (!TII->isInvariantLoad(MI)) { + // FIXME: we should be able to sink loads with no other side effects if + // there is nothing that can change memory from here until the end of + // block. This is a trivial form of alias analysis. + return false; + } + } // FIXME: This should include support for sinking instructions within the // block they are currently in to shorten the live ranges. We often get diff --git a/lib/Target/X86/X86InstrInfo.cpp b/lib/Target/X86/X86InstrInfo.cpp index 26ab7d2d0a4..100d308c566 100644 --- a/lib/Target/X86/X86InstrInfo.cpp +++ b/lib/Target/X86/X86InstrInfo.cpp @@ -763,16 +763,19 @@ bool X86InstrInfo::isReallyTriviallyReMaterializable(MachineInstr *MI) const { return true; } -/// isReallySideEffectFree - If the M_MAY_HAVE_SIDE_EFFECTS flag is set, this -/// method is called to determine if the specific instance of this instruction -/// has side effects. This is useful in cases of instructions, like loads, which -/// generally always have side effects. A load from a constant pool doesn't have -/// side effects, though. So we need to differentiate it from the general case. -bool X86InstrInfo::isReallySideEffectFree(MachineInstr *MI) const { +/// isInvariantLoad - Return true if the specified instruction (which is marked +/// mayLoad) is loading from a location whose value is invariant across the +/// function. For example, loading a value from the constant pool or from +/// from the argument area of a function if it does not change. This should +/// only return true of *all* loads the instruction does are invariant (if it +/// does multiple loads). +bool X86InstrInfo::isInvariantLoad(MachineInstr *MI) const { + // FIXME: This should work with any X86 instruction that does a load, for + // example, all load+op instructions. switch (MI->getOpcode()) { default: break; case X86::MOV32rm: - // Loads from stubs of global addresses are side effect free. + // Loads from stubs of global addresses are invariant. if (MI->getOperand(1).isReg() && MI->getOperand(2).isImm() && MI->getOperand(3).isReg() && MI->getOperand(4).isGlobal() && @@ -794,7 +797,7 @@ bool X86InstrInfo::isReallySideEffectFree(MachineInstr *MI) const { case X86::MOVAPDrm: case X86::MMX_MOVD64rm: case X86::MMX_MOVQ64rm: - // Loads from constant pools are trivially rematerializable. + // Loads from constant pools are trivially invariant. if (MI->getOperand(1).isReg() && MI->getOperand(2).isImm() && MI->getOperand(3).isReg() && MI->getOperand(4).isCPI() && MI->getOperand(1).getReg() == 0 && @@ -815,8 +818,8 @@ bool X86InstrInfo::isReallySideEffectFree(MachineInstr *MI) const { return false; } - // All other instances of these instructions are presumed to have side - // effects. + // All other instances of these instructions are presumed to have other + // issues. return false; } diff --git a/lib/Target/X86/X86InstrInfo.h b/lib/Target/X86/X86InstrInfo.h index 27675b94b1f..68f16647467 100644 --- a/lib/Target/X86/X86InstrInfo.h +++ b/lib/Target/X86/X86InstrInfo.h @@ -255,7 +255,7 @@ public: unsigned isLoadFromStackSlot(MachineInstr *MI, int &FrameIndex) const; unsigned isStoreToStackSlot(MachineInstr *MI, int &FrameIndex) const; bool isReallyTriviallyReMaterializable(MachineInstr *MI) const; - bool isReallySideEffectFree(MachineInstr *MI) const; + bool isInvariantLoad(MachineInstr *MI) const; /// convertToThreeAddress - This method must be implemented by targets that /// set the M_CONVERTIBLE_TO_3_ADDR flag. When this flag is set, the target diff --git a/utils/TableGen/InstrInfoEmitter.cpp b/utils/TableGen/InstrInfoEmitter.cpp index ed48c330047..5bf25d17451 100644 --- a/utils/TableGen/InstrInfoEmitter.cpp +++ b/utils/TableGen/InstrInfoEmitter.cpp @@ -357,8 +357,7 @@ void InstrInfoEmitter::emitRecord(const CodeGenInstruction &Inst, unsigned Num, if (Inst.usesCustomDAGSchedInserter) OS << "|(1<