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. 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.
> I don't have a strong opinion on variable naming, but this:
> ```
> 974 static bool shouldSplitStructStore(CodeGenModule &CGM,
> 975 uint64_t GlobalByteSize) {
> 976 // Don't break structures that occupy more than one cacheline.
> 977 uint64_t ByteSizeLimit = 64;
> 978 if (CGM.getCodeGenOpts().OptimizationLevel == 0)
> 979 return false;
> 980 if (GlobalByteSize <= ByteSizeLimit)
> 981 return true;
> 982 return false;
> ```
> looks a bit more heavyweight than the previous function, in which we also
> have `GlobalSize` and `SizeLimit`.
What are the cases we actually want to make sure we emit as separate stores?
Basically just really small types that happen to be wrapped up in several
layers of struct for abstraction purposes, right? Maybe this heuristic should
primarily consider element counts (and types) and use overall sizes at the top
level.
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
