glider marked 2 inline comments as done.
glider added inline comments.
================
Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1143
+ const llvm::StructLayout *Layout =
+ CGM.getDataLayout().getStructLayout(cast<llvm::StructType>(Ty));
+ for (unsigned i = 0; i != constant->getNumOperands(); i++) {
----------------
jfb wrote:
> I think you need a heuristic here, where you don't emit multiple stores if
> `getNumOperands` is greater than some number. In that case you'd keep doing
> memcpy instead. Note that a struct containing sub-structs (or arrays) should
> also count the number of sub-structs (so just checking `getNumOperands` isn't
> sufficient).
>
> Other parts of this code chose 6 stores...
Do we ever need to not split these stores? Can't the MemCpyOpt pass take care
of them later on?
================
Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1765
+ emitStoresForConstant(CGM, D, Loc, isVolatile, Builder, constant,
+ /*forInit*/ false);
}
----------------
jfb wrote:
> Can you explain why this case needs to be different? IIUC it's because the
> types can differ which makes the struct's elements not match up the ones from
> the constant in the loop above? I think the code above should automatically
> handle that case instead of passing in a flag here.
>
> You also definitely need to test the case where that happens :)
I just thought we shouldn't break the constants that we didn't create with
-ftrivial-var-auto-init.
I'll take a closer look though (indeed, this crashes on one of the structs from
the Linux kernel)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57898/new/
https://reviews.llvm.org/D57898
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
