[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

While I understand people are eager to receive feedback on their patches, it is not helpful to ping/remind the reviewers constantly.
This does generate noise for them and can consequently also reduce their interest in a patch. The recommendation for time without
review before a "ping" is send is still one week.



================
Comment at: clang/lib/CodeGen/CGExpr.cpp:151
+  // allocas.
+  Builder.CreateStore(Init, Var);
 }
----------------
I'm not even sure this is necessarily correct. 

How do we know the new store is not inside a loop and might write the value more than once, potentially overwriting a value set later?

Aside from that (important) question, you need to update the documentation of the function. It doesn't correspond to the new semantics anymore.



================
Comment at: clang/test/CodeGenCUDA/builtins-amdgcn.cu:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 -x hip \
----------------
Please prepare a pre-commit that adds auto-generated check lines. Adding them as part of a commit makes it impossible to see the difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257