jfb added a comment. Overall this LGTM besides a few nits, and wanting input from @rjmccall.
As follow-ups (which I can take on): - Handle the case where the types don't match (because `Loc` was adjusted). - Handle small arrays (and any other cases where the struct stores get broken down into components which still contain a `memcpy`/ `memset`). ================ Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:975 +static bool shouldSplitStructStore(CodeGenModule &CGM, llvm::Constant *Init, + uint64_t GlobalSize) { + // Don't break structures that occupy more than one cacheline. ---------------- You don't use `Init` in the function. ================ Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979 + if (CGM.getCodeGenOpts().OptimizationLevel == 0) + return false; + if (GlobalSize <= SizeLimit) ---------------- The general 64-byte heuristic is fine with me. It's just a bit weird to special-case `-O0`, but we do it elsewhere too (and it keeps the current status-quo of letting the backend decide what to do). From that perspective I'm fine with it. @rjmccall do you have a strong preference either way? One small nit: can you use `ByteSize` instead of just `Size`? Makes it easier to figure out what's going on in code IMO. ================ Comment at: tools/clang/test/CodeGenCXX/auto-var-init.cpp:476 // CHECK-NEXT: call void @{{.*}}used{{.*}}%uninit) // PATTERN-LABEL: @test_empty_uninit() +// PATTERN-O0: call void @llvm.memcpy{{.*}} @__const.test_empty_uninit.uninit ---------------- The tests aren't matching labels anymore: `PATTERN-LABEL` is a dead label check now. I think forking things at `-O0` and `-O1` is fine, but I think you want a small `-O0` test (separate from this patch) which does one or two things, and then you want this test to only look at `-O1`. That way you don't need so much stuff changing in the test (and `-O0` is still tested). 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