vitalybuka marked 4 inline comments as done. vitalybuka added inline comments.
================ Comment at: clang/lib/CodeGen/CGDecl.cpp:865 +/// initializer with equal or fewer than NumStores scalar stores. +static bool canEmitStoresForInitAfterMemset(CodeGenModule &CGM, + llvm::Constant *Init, ---------------- jfb wrote: > You should update the return value to `Optional<Value*>` where the non-`None` > return is the bytewise value. Why? We already know value which was used for memset, now we just counting additional store/memsets ================ Comment at: clang/lib/CodeGen/CGDecl.cpp:873 return true; - if (isa<llvm::ConstantInt>(Init) || isa<llvm::ConstantFP>(Init) || + if (BWInit || isa<llvm::ConstantInt>(Init) || isa<llvm::ConstantFP>(Init) || isa<llvm::ConstantVector>(Init) || isa<llvm::BlockAddress>(Init) || ---------------- jfb wrote: > I'm not sure I understand what this bit does. If something isn't a bytewise > value then we now automatically assume it can be stored to with a single > store. That's not necessarily true. I think you need to drop `BWInit ||` from > here, and conditionalize this entire branch with `if (BWInit)` instead (so, > an `&&` on it all). If value is bytewise, we will emit BWInit ================ Comment at: clang/lib/CodeGen/CGDecl.cpp:888 return true; } ---------------- jfb wrote: > The recursion now allows different values for `BWInit` on each sub-element. yes ================ Comment at: clang/lib/CodeGen/CGDecl.cpp:895 + return true; + } + ---------------- jfb wrote: > What does this new loop do? It'll only ever go around the loop once. it counts number of stores used by elements similar look we had in canEmitInitWithFewStoresAfterBZero Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64382/new/ https://reviews.llvm.org/D64382 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits