[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Do you have open questions on whether some callsites passing "false" here, should be switched to true? Given what's here, I would say that it definitely does not makes sense to add this parameter everywhere.

So, for getting something committed: please send a new review which makes only the required changes, splitting by logical part of the code. E.g. one change to fix the new/delete alignment, one for the global-variable alignment, and so on if there are others.



================
Comment at: clang/lib/CodeGen/CGAtomic.cpp:814
+  std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(
+      AtomicTy, true /* NeedsPreferredAlignment */);
   uint64_t Size = sizeChars.getQuantity();
----------------
This is wrong.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:252
+      std::tie(RetVal.Size, RetVal.Align) = Ctx.getTypeInfoInChars(
+          FD->getType(), true /* NeedsPreferredAlignment */);
       assert(llvm::isPowerOf2_64(RetVal.Align.getQuantity()));
----------------
Not sure if this is right, since it's looking at individual fields in the struct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86790