[PATCH] D115370: [NFC] Replace some deprecated getAlignment() calls with getAlign()

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 8 13:46:46 PST 2021


aeubanks added inline comments.


================
Comment at: llvm/lib/Analysis/MemoryBuiltins.cpp:595
 
-APInt ObjectSizeOffsetVisitor::align(APInt Size, uint64_t Alignment) {
+APInt ObjectSizeOffsetVisitor::align(APInt Size, MaybeAlign Alignment) {
   if (Options.RoundToAlign && Alignment)
----------------
arsenm wrote:
> Don't see why this wouldn't just be Alignment (really I don't know why we have MaybeAlign at all)
background: http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
an alignment of 0 (unknown) is different from 1 (no alignment), but actually https://reviews.llvm.org/D64790 seems to specify that perhaps we can treat 0 as 1

but anyway, for now there's a check that `Alignment` is not 0, so I'll keep that for now


================
Comment at: llvm/lib/IR/AsmWriter.cpp:4243
+    if (MaybeAlign A = AI->getAlign()) {
+      Out << ", align " << A->value();
     }
----------------
arsenm wrote:
> probably should add a direct raw_ostream overload?
I wouldn't expect a raw_ostream overload for MaybeAlign to print a comma, just `align #`, but if we want to conditionally emit the comma then we already have to check if MaybeAlign is not null, so I don't see much point in it. and it matches nearby code anyway


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115370



More information about the llvm-commits mailing list