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

[Rafael Espíndola via llvm-commits]

llc crashes on the attached file.

Cheers,
Rafael


On 3 December 2015 at 23:27, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: AArch64Disassembler.ll
Type: application/octet-stream
Size: 220469 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151204/04bf4da6/attachment-0001.obj>