[MC] Handle layout change due to relaxation of another section

[reames]

Summary

This is an alternate fix for the test case from https://reviews.llvm.org/D76114.

The (rather fundamental) issue revealed by this test case is that our relaxation implementation was not correctly adjusting the size and offsets of fragments in one section based on changes in size of another if the layout order of the two happened to be such that the former was visited before the later. To trigger this, you need one section which refers to labels in another, but the fragments involved aren't relaxable. (If they were, that would trigger invalidation.) Any of fill, align, or the like can trip this.

An alternate approach would be to make all fragments with expression computable sizes relaxable. This would require either a) storing the size in the fragment, or b) peaking ahead to compute the previously computed size from the offset of the next fragment.

I'm curious, which do folks think is less ugly?

p.s. This is mostly posted for discussion purposes. Jain, you're welcome to take any of this code and incorporate into a refresh on the original review if desired.

[jcai19]

Thanks! I actually had almost identical implementation at first :), but then I realized I needed to avoid "stats::FragmentLayouts" to be increased in MCAsmLayout::layoutFragment when adjusting the offset of valid fragments and changed my implementation to https://reviews.llvm.org/D76114?id=250118. Also, one thing I am not sure is if marking fragments invalid will always trigger relaxation if needed in the following iteration of the while loop?

[jcai19]

Never mind my last comment. I think I misunderstood the meaning of stats::FragmentLayouts. I have updated https://reviews.llvm.org/D76114 accordingly. Thanks.

[skan]

I prefer the current solution. Making all fragments with expression computable sizes relaxable is much more ugly.

llvm/lib/MC/MCAssembler.cpp

for (unsigned i = 0, e = Layout.getSectionOrder().size(); i != e; ++i) {
     Sec->setLayoutOrder(i);
 
     unsigned FragmentIndex = 0;
     for (MCFragment &Frag : *Sec)
       Frag.setLayoutOrder(FragmentIndex++);
   }
 
   // Layout until everything fits.
-  while (layoutOnce(Layout))
+  while (layoutOnce(Layout)) {
     if (getContext().hadError())
       return;
 
+    // Size of fragments in one section can depend on the size of fragments in
+    // another.  If any fragment has changed size, we have to re-layout (and
+    // as a result possibly further relax) all.
+    for (MCSection &Sec : *this)
+      Layout.invalidateFragmentsFrom(&*Sec.begin());
+  }
+    
   DEBUG_WITH_TYPE("mc-dump", {
       errs() << "assembler backend - post-relaxation\n--\n";
       dump(); });
 
   // Finalize the layout, including fragment lowering.
   finishLayout(Layout);
 
   DEBUG_WITH_TYPE("mc-dump", {

llvm/test/MC/X86/relax-offset.s

+# RUN: llvm-mc -filetype=obj -triple=i386 %s | llvm-objdump - --headers | FileCheck %s
+
+  # CHECK: .text1        00000005 00000000 
+  # CHECK: .text2        00000005 00000000 
+
+  .section .text1
+ .skip after-before,0x0
+.Lint80_keep_stack:
+
+  .section .text2
+before:
+ jmp .Lint80_keep_stack
+after:
+