[llvm] r254694 - [BranchFolding] Merge MMOs during tail merge

[Rafael Espíndola via llvm-commits]

I reverted it because it broke bootstrap.

I  have delta running and will email a testcase once I have it.

Cheers,
Rafael


On 3 December 2015 at 21:29, Junmo Park via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: flyingforyou
> Date: Thu Dec  3 20:29:25 2015
> New Revision: 254694
>
> URL: http://llvm.org/viewvc/llvm-project?rev=254694&view=rev
> Log:
> [BranchFolding] Merge MMOs during tail merge
>
> Summary:
> If we remove the MMOs from Load/Store instructions,
> they are treated as volatile. This makes other optimization passes unhappy.
> eg. Load/Store Optimization
>
> So, it looks better to merge, not remove.
>
> Reviewers: gberry, mcrosier
>
> Subscribers: gberry, llvm-commits
>
> Differential Revision: http://reviews.llvm.org/D14797
>
>
> Modified:
>     llvm/trunk/lib/CodeGen/BranchFolding.cpp
>
> Modified: llvm/trunk/lib/CodeGen/BranchFolding.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BranchFolding.cpp?rev=254694&r1=254693&r2=254694&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/BranchFolding.cpp (original)
> +++ llvm/trunk/lib/CodeGen/BranchFolding.cpp Thu Dec  3 20:29:25 2015
> @@ -744,24 +744,35 @@ bool BranchFolder::CreateCommonTailOnlyB
>    return true;
>  }
>
> -static bool hasIdenticalMMOs(const MachineInstr *MI1, const MachineInstr *MI2) {
> +// Add MI1's MMOs to MI2's MMOs while excluding any duplicates. The MI scheduler
> +// currently doesn't handle multiple MMOs, so duplicates would likely pessimize
> +// the scheduler.
> +static void mergeMMOs(MachineInstr *MI1, MachineInstr *MI2) {
>    auto I1 = MI1->memoperands_begin(), E1 = MI1->memoperands_end();
>    auto I2 = MI2->memoperands_begin(), E2 = MI2->memoperands_end();
> -  if ((E1 - I1) != (E2 - I2))
> -    return false;
> -  for (; I1 != E1; ++I1, ++I2) {
> -    if (**I1 != **I2)
> -      return false;
> +  MachineFunction *MF = MI1->getParent()->getParent();
> +
> +  // Mostly, MI1's MMO count is 1 or zero. So we don't have to use
> +  // SmallSet.
> +  for (; I1 != E1; ++I1) {
> +    bool IsDupMMO = false;
> +    for (I2 = MI2->memoperands_begin(); I2 != E2; ++I2) {
> +      if (**I1 == **I2) {
> +        IsDupMMO = true;
> +        break;
> +      }
> +    }
> +    if (IsDupMMO == false) {
> +      MI2->addMemOperand(*MF, *I1);
> +      E2 = MI2->memoperands_end();
> +    }
>    }
> -  return true;
>  }
>
>  static void
> -removeMMOsFromMemoryOperations(MachineBasicBlock::iterator MBBIStartPos,
> -                               MachineBasicBlock &MBBCommon) {
> -  // Remove MMOs from memory operations in the common block
> -  // when they do not match the ones from the block being tail-merged.
> -  // This ensures later passes conservatively compute dependencies.
> +mergeMMOsFromMemoryOperations(MachineBasicBlock::iterator MBBIStartPos,
> +                              MachineBasicBlock &MBBCommon) {
> +  // Merge MMOs from memory operations in the common block
>    MachineBasicBlock *MBB = MBBIStartPos->getParent();
>    // Note CommonTailLen does not necessarily matches the size of
>    // the common BB nor all its instructions because of debug
> @@ -792,8 +803,7 @@ removeMMOsFromMemoryOperations(MachineBa
>      assert(MBBICommon->isIdenticalTo(&*MBBI) && "Expected matching MIIs!");
>
>      if (MBBICommon->mayLoad() || MBBICommon->mayStore())
> -      if (!hasIdenticalMMOs(&*MBBI, &*MBBICommon))
> -        MBBICommon->clearMemRefs();
> +      mergeMMOs(&*MBBI, &*MBBICommon);
>
>      ++MBBI;
>      ++MBBICommon;
> @@ -913,8 +923,8 @@ bool BranchFolder::TryTailMergeBlocks(Ma
>          continue;
>        DEBUG(dbgs() << "BB#" << SameTails[i].getBlock()->getNumber()
>                     << (i == e-1 ? "" : ", "));
> -      // Remove MMOs from memory operations as needed.
> -      removeMMOsFromMemoryOperations(SameTails[i].getTailStartPos(), *MBB);
> +      // Merge MMOs from memory operations as needed.
> +      mergeMMOsFromMemoryOperations(SameTails[i].getTailStartPos(), *MBB);
>        // Hack the end off BB i, making it jump to BB commonTailIndex instead.
>        ReplaceTailWithBranchTo(SameTails[i].getTailStartPos(), MBB);
>        // BB i is no longer a predecessor of SuccBB; remove it from the worklist.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits