[PATCH] D94891: [Coroutine] Remain alignment information when merging frame variables

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:608-612
+      bool Alignable = [&]() -> bool {
+        auto *LargestAlloca = *AllocaSet.begin();
+        return LargestAlloca->getAlign().value() % Alloca->getAlign().value() ==
+               0;
+      }();
----------------
jmorse wrote:
> This doesn't need to be a lambda, it's only called once
I wonder if the logic could be more clear when we wrap the code in lambda.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:1123
         Builder.CreateInBoundsGEP(FrameTy, FramePtr, Indices));
-    if (isa<AllocaInst>(Orig)) {
+    if (auto *AI = dyn_cast<AllocaInst>(Orig)) {
       // If the type of GEP is not equal to the type of AllocaInst, it implies
----------------
lxfind wrote:
> Is this change unintentional? It seems to be AI and Orig is the same thing. What's the reason to use AI below instead of just Orig?
Oh, yes, it is unintentional change. I would revert this change.


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

https://reviews.llvm.org/D94891