[PATCH] D114590: [CodeGen] Emit alignment "Max Skip" operand for align directives

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Can you add a test to show that the alignment when emitting an object file is correct too?

> I've partially folded D114879 <https://reviews.llvm.org/D114879> into this, I want to keep the AArch64-specific stuff separate though.

OK. I'm not sure I understand why. It is easier to review patches that are complete and do something specific :)



================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:143
+  /// and are equivalent to a value of Zero.
+  uint8_t MaxBytesForAlignment = 0;
 
----------------
NickGuy wrote:
> courbet wrote:
> > Is there any reason for the inconsistency between `unsigned` in `emitAlignment` and `uint8_t` here ? I don't think this struct is particularly memory-constrained, so I would just use `unsigned` here.
> No particular reason beyond memory saving, if that's not a concern here then `unsigned` makes more sense.
Using unsigned sounds good.

The "Values greater than..." sentence is already covered by the "no bytes are emitted" part above, and can be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114590