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 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.
Yes, small types.
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
