[PATCH] [MC] Align fragments when -mc-relax-all flag is used

[Petr Hosek]

REPOSITORY
  rL LLVM

================
Comment at: lib/MC/MCAssembler.cpp:589
@@ -588,2 +588,3 @@
   // bundle padding directly into fragments when the instructions are emitted
-  // inside the streamer.
+  // inside the streamer. However, we still need to ensure that fragments are
+  // bundle aligned.
----------------
mseaborn wrote:
> How about adding more context?  i.e.:
> 
> """
> Without -mc-relax-all, we emit one instruction per fragment.  These fragments don't contain no-op bundle padding, so the fragments make no assumption about their alignment.
> 
> With -mc-relax-all, we emit multiple instructions per fragment, with no-op bundle padding inside fragments, which means these fragments potentially assume they are bundle-aligned.  (At least, a fragment that is larger than the bundle size will assume it is bundle-aligned.)  This means that if we end up with multiple fragments, we must emit bundle padding between fragments.
> 
> ".align N" is an example of a directive that introduces multiple fragments.  It might be better to add a special case to handle ".align N" by emitting within-fragment padding (which would produce less padding when N is less than the bundle size), but for now we don't.
> """
Done.

================
Comment at: test/MC/X86/AlignedBundling/misaligned-bundle-group.s:17
@@ +16,3 @@
+# CHECK-RELAX:      2f: nopw %cs:(%eax,%eax)
+        movl $0x1, (%esp)
+        .bundle_unlock
----------------
mseaborn wrote:
> How about using "call" as a more realistic example, since that is what's used with align_to_end?
Done.

================
Comment at: test/MC/X86/AlignedBundling/misaligned-bundle.s:13
@@ +12,3 @@
+# CHECK-RELAX:      1f: nop
+        movl $0x1, (%esp)
+        movl $0x1, (%esp)
----------------
mseaborn wrote:
> It's quite difficult to tell, by reading this test, what it's intended to be testing for.
> 
> For a start, how about commenting how many bytes these instructions are meant to be?
> e.g.
> movl $0x1, (%esp) // 7 bytes [or 5 bytes?]
> 
> How about checking for these instructions and their offsets in the output too?  The same applies to the other test.
Done.

http://reviews.llvm.org/D10044

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/