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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
