Commit 0a8cae10 authored by Sam Parker's avatar Sam Parker
Browse files

[ReachingDefs] Make isSafeToMove more strict. 

Test that we're not moving the instruction through instructions with
side-effects.

Differential Revision: https://reviews.llvm.org/D74058
parent 09a88120

llvm/lib/CodeGen/ReachingDefAnalysis.cpp

+12 −4
Original line number Diff line number Diff line
@@ -371,6 +371,12 @@ MachineInstr* ReachingDefAnalysis::getLocalLiveOutMIDef(MachineBasicBlock *MBB, 
  return Def < 0 ? nullptr : getInstFromId(MBB, Def);

}



static bool mayHaveSideEffects(MachineInstr &MI) {

  return MI.mayLoadOrStore() || MI.mayRaiseFPException() ||

         MI.hasUnmodeledSideEffects() || MI.isTerminator() ||

         MI.isCall() || MI.isBarrier() || MI.isBranch() || MI.isReturn();

}



// Can we safely move 'From' to just before 'To'? To satisfy this, 'From' must

// not define a register that is used by any instructions, after and including,

// 'To'. These instructions also must not redefine any of Froms operands.
@@ -392,10 +398,13 @@ bool ReachingDefAnalysis::isSafeToMove(MachineInstr *From, 
  }



  // Now walk checking that the rest of the instructions will compute the same

  // value.

  // value and that we're not overwriting anything. Don't move the instruction

  // past any memory, control-flow or other ambigious instructions.

  for (auto I = ++Iterator(From), E = Iterator(To); I != E; ++I) {

    if (mayHaveSideEffects(*I))

      return false;

    for (auto &MO : I->operands())

      if (MO.isReg() && MO.getReg() && MO.isUse() && Defs.count(MO.getReg()))

      if (MO.isReg() && MO.getReg() && Defs.count(MO.getReg()))

        return false;

  }

  return true;
@@ -430,8 +439,7 @@ ReachingDefAnalysis::isSafeToRemove(MachineInstr *MI, InstSet &Visited, 
                                    InstSet &ToRemove, InstSet &Ignore) const {

  if (Visited.count(MI) || Ignore.count(MI))

    return true;

  else if (MI->mayLoadOrStore() || MI->hasUnmodeledSideEffects() ||

           MI->isBranch() || MI->isTerminator() || MI->isReturn()) {

  else if (mayHaveSideEffects(*MI)) {

    // Unless told to ignore the instruction, don't remove anything which has

    // side effects.

    return false;

llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp

+2 −0
Original line number Diff line number Diff line
@@ -390,6 +390,8 @@ bool LowOverheadLoop::ValidateTailPredicate(MachineInstr *StartInsertPt) { 
  // The element count register maybe defined after InsertPt, in which case we

  // need to try to move either InsertPt or the def so that the [w|d]lstp can

  // use the value.

  // TODO: On failing to move an instruction, check if the count is provided by

  // a mov and whether we can use the mov operand directly.

  MachineBasicBlock *InsertBB = StartInsertPt->getParent();

  if (!RDA->isReachingDefLiveOut(StartInsertPt, NumElements)) {

    if (auto *ElemDef = RDA->getLocalLiveOutMIDef(InsertBB, NumElements)) {

llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir

+0 −2
Original line number Diff line number Diff line
@@ -3,8 +3,6 @@ 


# Test that, though the vctp operand is defined at the end of the block,

# that the correct value is used for the dlstp.

# TODO: The pass currently just bails instead of finding the correct

# value.



--- |

  define dso_local arm_aapcs_vfpcc void @start_before_elems(i32* noalias nocapture %a, i8* nocapture readonly %b, i8* nocapture readonly %c, i32 %N) local_unnamed_addr #0 {