[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added a comment. I think (with test updates) this will be good to go once D58188 lands. Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1158 + llvm::StructType *STy = dyn_cast(Ty); + if (STy && (STy == Loc.getElementType()) && + shou

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider added a comment. FWIW I think I've almost finished handling the padding: https://reviews.llvm.org/D58188 I haven't checked whether it works correctly with padding in custom initializers (like `struct s local = {1, 2}`), but TEST_UNINIT and TEST_BRACES are covered already. CHANGES SINCE

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added inline comments. Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979 + if (CGM.getCodeGenOpts().OptimizationLevel == 0) +return false; + if (GlobalSize <= SizeLimit) rjmccall wrote: > glider wrote: > > jfb wrote: > > > The general 64-byte heuristic

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added a comment. In D57898#1395953 , @glider wrote: > ... which happily skips the padding. I don't think padding is an issue right now. It's valid to either initialize it or not. It is slightly unfortunate to lose the information about padding (whic

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

rjmccall added inline comments. Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979 + if (CGM.getCodeGenOpts().OptimizationLevel == 0) +return false; + if (GlobalSize <= SizeLimit) glider wrote: > jfb wrote: > > The general 64-byte heuristic is fine with me.

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider added a comment. ... which happily skips the padding. It occurs to me that we need to properly handle the padding in `patternFor` before we'll be able to split the structures. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57898/new/ https://reviews.llvm.org/D57898 __

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider added a comment. Well, now we somewhat break padding initialization. E.g. for the following struct: struct s { int a; char b; long int c; }; we generate the following constant initializer with `-O0`: .L__const.foo.local: .long 2863311530 # 0xa

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider added inline comments. Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979 + if (CGM.getCodeGenOpts().OptimizationLevel == 0) +return false; + if (GlobalSize <= SizeLimit) jfb wrote: > The general 64-byte heuristic is fine with me. It's just a bit wei

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider updated this revision to Diff 186605. glider marked 4 inline comments as done. glider added a comment. Addressed the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57898/new/ https://reviews.llvm.org/D57898 Files: tools/clang/lib/CodeGen/CGDecl.cpp tools/clang/test/

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

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 c

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider added a comment. This patch shaved off 2.5% of performance slowdown on https://github.com/kamalmarhubi/linux-ipc-benchmarks/blob/master/af_inet_loopback.c on a kernel built in pattern-initialization mode. The .rodata size went down by only 12K (0.5%, since we now split only structures sm

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider updated this revision to Diff 186469. glider marked an inline comment as done. glider added a subscriber: pcc. glider added a comment. Added a helper function to decide whether we want to break the structure or not. Right now it only checks for optimization level and structure size. The co

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider marked 4 inline comments as done. glider added inline comments. Herald added a subscriber: jdoerfert. Comment at: tools/clang/test/CodeGenCXX/auto-var-init.cpp:496 // ZERO-LABEL: @test_smallpartinit_uninit() // ZERO: call void @llvm.memset{{.*}}, i8 0,

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added a comment. In D57898#1390816 , @glider wrote: > > Can you add a link to bug 40605 in the commit message? > > It's in the title now, doesn't that count? :) Oh OK I hadn't seen that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57898/n

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider added a comment. > Can you add a link to bug 40605 in the commit message? It's in the title now, doesn't that count? :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57898/new/ https://reviews.llvm.org/D57898 ___ cfe-commits mailing

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added a comment. Can you add a link to bug 40605 in the commit message? > I'm not quite sure how to show the resulting difference in code. Do you mean > we need Clang to support both modes and to compare the resulting assembly? I only meant tests that show codegen, as you've now added auto-

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider added a comment. Re: case when STy != Loc.getElementType() - this is already covered by other Clang tests. I'm still unsure about the heuristic here. I believe that for auto-initialization we want to be quite aggressive with these splits (unlike for regular constant stores). Perhaps we

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

glider updated this revision to Diff 185990. glider retitled this revision from "[RFC] Split constant structures generated by -ftrivial-auto-var-init when emitting initializators" to "CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializator