[llvm-commits] [llvm] r45843 - in /llvm/trunk: include/llvm/Target/TargetInstrDesc.h include/llvm/Target/TargetInstrInfo.h lib/CodeGen/MachineLICM.cpp lib/CodeGen/MachineSink.cpp lib/Target/X86/X86InstrInfo.cpp lib/Target/X86/X86InstrInfo.h utils/TableGen/InstrInfoEmitter.cpp

Chris Lattner sabre at nondot.org
Thu Jan 10 15:08:24 PST 2008


Author: lattner
Date: Thu Jan 10 17:08:24 2008
New Revision: 45843

URL: http://llvm.org/viewvc/llvm-project?rev=45843&view=rev
Log:
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.

Modified:
    llvm/trunk/include/llvm/Target/TargetInstrDesc.h
    llvm/trunk/include/llvm/Target/TargetInstrInfo.h
    llvm/trunk/lib/CodeGen/MachineLICM.cpp
    llvm/trunk/lib/CodeGen/MachineSink.cpp
    llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
    llvm/trunk/lib/Target/X86/X86InstrInfo.h
    llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp

Modified: llvm/trunk/include/llvm/Target/TargetInstrDesc.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetInstrDesc.h?rev=45843&r1=45842&r2=45843&view=diff

==============================================================================
--- llvm/trunk/include/llvm/Target/TargetInstrDesc.h (original)
+++ llvm/trunk/include/llvm/Target/TargetInstrDesc.h Thu Jan 10 17:08:24 2008
@@ -92,8 +92,7 @@
     SimpleLoad,
     MayLoad,
     MayStore,
-    NeverHasSideEffects,
-    MayHaveSideEffects,
+    UnmodeledSideEffects,
     Commutable,
     ConvertibleTo3Addr,
     UsesCustomDAGSchedInserter,
@@ -326,37 +325,21 @@
     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().
-  ///
-  /// Examples of other side effects would be calling a function, modifying
-  /// 'invisible' machine state like a control register, etc.
-  ///
-  /// If some instances of this instruction are side-effect free but others are
-  /// not, the hasConditionalSideEffects() property should return true, not this
-  /// one.
-  ///
-  /// 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);
+  /// 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:
+  ///
+  ///  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.
+  ///
+  /// Examples of side effects would be modifying 'invisible' machine state like
+  /// a control register, flushing a cache, modifying a register invisible to
+  /// LLVM, etc.
+  ///
+  bool hasUnmodeledSideEffects() const {
+    return Flags & (1 << TID::UnmodeledSideEffects);
   }
   
   //===--------------------------------------------------------------------===//

Modified: llvm/trunk/include/llvm/Target/TargetInstrInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetInstrInfo.h?rev=45843&r1=45842&r2=45843&view=diff

==============================================================================
--- llvm/trunk/include/llvm/Target/TargetInstrInfo.h (original)
+++ llvm/trunk/include/llvm/Target/TargetInstrInfo.h Thu Jan 10 17:08:24 2008
@@ -69,15 +69,6 @@
            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 @@
     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 @@
     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

Modified: llvm/trunk/lib/CodeGen/MachineLICM.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineLICM.cpp?rev=45843&r1=45842&r2=45843&view=diff

==============================================================================
--- llvm/trunk/lib/CodeGen/MachineLICM.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineLICM.cpp Thu Jan 10 17:08:24 2008
@@ -223,6 +223,26 @@
 /// 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 @@
           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 @@
       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;
 }

Modified: llvm/trunk/lib/CodeGen/MachineSink.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineSink.cpp?rev=45843&r1=45842&r2=45843&view=diff

==============================================================================
--- llvm/trunk/lib/CodeGen/MachineSink.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineSink.cpp Thu Jan 10 17:08:24 2008
@@ -133,19 +133,21 @@
   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

Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=45843&r1=45842&r2=45843&view=diff

==============================================================================
--- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Thu Jan 10 17:08:24 2008
@@ -763,16 +763,19 @@
   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 @@
   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 @@
     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;
 }
 

Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.h?rev=45843&r1=45842&r2=45843&view=diff

==============================================================================
--- llvm/trunk/lib/Target/X86/X86InstrInfo.h (original)
+++ llvm/trunk/lib/Target/X86/X86InstrInfo.h Thu Jan 10 17:08:24 2008
@@ -255,7 +255,7 @@
   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

Modified: llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp?rev=45843&r1=45842&r2=45843&view=diff

==============================================================================
--- llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp (original)
+++ llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp Thu Jan 10 17:08:24 2008
@@ -357,8 +357,7 @@
   if (Inst.usesCustomDAGSchedInserter)
     OS << "|(1<<TID::UsesCustomDAGSchedInserter)";
   if (Inst.isVariadic)         OS << "|(1<<TID::Variadic)";
-  if (Inst.mayHaveSideEffects) OS << "|(1<<TID::MayHaveSideEffects)";
-  if (!HasSideEffects)         OS << "|(1<<TID::NeverHasSideEffects)";
+  if (HasSideEffects)          OS << "|(1<<TID::UnmodeledSideEffects)";
   OS << ", 0";
 
   // Emit all of the target-specific flags...





More information about the llvm-commits mailing list