[PATCH] D15230: [BranchFolding] Merge MMOs during tail merge

[Junmo Park via llvm-commits]

flyingforyou added a comment.

> Would you mind explaining what exactly caused the bootstrap to fail?


The error message is below.

> void llvm::MachineInstr::setMemRefs(llvm::MachineInstr::mmo_iterator, llvm::MachineInstr::mmo_iterator): Assertion `NumMemRefs == NewMemRefsEnd - NewMemRefs && "Too many memrefs"' failed.




  MachineInstr.h
  
     uint8_t NumMemRefs;                   // Information on memory references.
     mmo_iterator MemRefs;

NumMemRefs type is uint8_t. So the maximum value is 255. Previous commit doesn't care about the NewMemrefs's count. When modified clang build clang, there are several cases that sum of two MI's MMOs count is over 255.
So, it has a problem. I just set the threshold value maximum. No matter MI1, MI2's count is, it could be safe.
eg. MI1 MMOs count == 1, MI2 MMOs count == 255, it will be occurred overflow. Because Sum of two MMOs count is 256. (Product of two MMOs count is 255. This is the maximum value of NumMemRefs.)

I don't want to set threshold value to 255. 16 is also good choice. (If the threshold value is 16, most of MMOs can be merged.)

> p.s., We should always do the necessary due diligence to prevent breakages, but please don't lose sleep over it. :) We do appreciate your efforts on this patch!


I really appreciate your kind words.

Junmo.


http://reviews.llvm.org/D15230