[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I'm not a great reviewer for coroutines as I don't quite get them, so a vastly better commit/review message would be GREATLY appreciated.

Code seems fine, but I'd love for someone better at coroutines to take a look too.



================
Comment at: clang/lib/CodeGen/CGCoroutine.cpp:688
+    auto &Context = getContext();
+    auto SizeTy = Context.getSizeType();
+    auto T = Builder.getIntNTy(Context.getTypeSize(SizeTy));
----------------
Don't use 'auto' unless the RHS has the type in it clearly.  SO this should be QualType I think?  I also see you've broken that coding standard rule above.


================
Comment at: clang/lib/CodeGen/CGCoroutine.cpp:689
+    auto SizeTy = Context.getSizeType();
+    auto T = Builder.getIntNTy(Context.getTypeSize(SizeTy));
+    llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::coro_align, T);
----------------
Same here.  ALSO, even when using 'auto', we need ot keep the '*' around.


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

https://reviews.llvm.org/D133341