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

Nicholas Guy via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 3 06:37:00 PST 2021


NickGuy added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:141
+  /// Zero to represent no maximum.
+  /// Values greater than Alignment^2 are understood to be functionally useless,
+  /// and are equivalent to a value of Zero.
----------------
dmgreen wrote:
> 1<<Alignment or 2^Alignment.  It may be better to use the wording from elsewhere and say "If the alignment cannot be reached in this many bytes, no bytes are emitted."
Right, Alignment^2 is not the same as 2^Alignment... Fixed


================
Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:143
+  /// and are equivalent to a value of Zero.
+  uint8_t MaxBytesForAlignment = 0;
 
----------------
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.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2465
   } else
     OutStreamer->emitValueToAlignment(Alignment.value());
 }
----------------
dmgreen wrote:
> What about this code path?
I haven't seen anything take this path, but I've now added MaxBytesToEmit to this call too, following the logic that if MaxBytesToEmit is provided, it should be used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114590



More information about the llvm-commits mailing list