[PATCH] D70157: Align branches within 32-Byte boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I find another deficiency (infinite loop) with the current approach.

Say, there is a `je 0` (0x0F 0x84 0x00 0x00 0x00 0x00) at byte 0x90. (0x90+6)%32 == 0, so it ends on a 32-byte boundary.
MF.getMaxPrefixSize() is 4, so the size of MCMachineDependentFragment may vary from 0 to 4.
If there are other MCMachineDependentFragment's in the section, some may shrink while some may expand.
In some cases the following loop will not converge

  bool MCAssembler::layoutOnce(MCAsmLayout &Layout) {
    ++stats::RelaxationSteps;
  
    bool WasRelaxed = false;
    for (iterator it = begin(), ie = end(); it != ie; ++it) {
      MCSection &Sec = *it;
      while (layoutSectionOnce(Layout, Sec)) ///
        WasRelaxed = true;
    }
  
    return WasRelaxed;
  }
  
  // In MCAssembler::layoutSectionOnce,
    case MCFragment::FT_MachineDependent:
      RelaxedFrag =
          relaxMachineDependent(Layout, *cast<MCMachineDependentFragment>(I));
      break;

To give a concrete example, `clang++ -fsanitize=memory compiler-rt/test/msan/cxa_atexit.cpp -mbranches-within-32B-boundaries` does not converge. You may also try dtor-*.cpp in that directory.

A simple iterative algorithm is not guaranteed to converge. We probably can solve the layout problem with a dynamic programming algorithm:

f[i][j] = the minimum cost that layouts the first i instructions with j extra bytes (via NOPs or prefixes)
or
g[i][j] = the minimum inserted bytes that layouts the first i instructions with cost j

I am not clear which one is better. A simple greedy approach is to set an upper limit on the number of iterations when the section contains at least one MCMachineDependentFragment, i.e.

  bool HasMCMachineDependentFragment = false;
  int count = 5; // arbitrary. Please find an appropriate value.
  while (layoutSectionOnce(Layout, Sec, HasMCMachineDependentFragment) && count > 0) {
    WasRelaxed = true;
    if (HasMCMachineDependentFragment)
      count--;
  }


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70157/new/

https://reviews.llvm.org/D70157