On 8/10/23 14:06, Javier Martinez wrote:
Thanks for the comments, Jason.
v2: + Fix formatting, remove unnecessarily warning.
On Tue, Aug 8, 2023 at 10:28 PM Jason Merrill <ja...@redhat.com
<mailto:ja...@redhat.com>> wrote:
> Seems reasonable, but how do you expect this to be used?
We have large applications where some large classes are known to be used
only at startup. Marking every member function as cold is impractical so
we would mark those classes as cold.
Sure, though I think of the cold attribute as being primarily to mark
unlikely paths in otherwise hot code; is this a
if (!initialized)
do_initialization();
kind of situation?
> I think doing this here will miss lazily-declared special member
> functions; I wonder if you want to copy the attribute in grokclassfn
> instead?
I think they are being handled correctly with the current patch.
Considered the following program:
class __attribute((cold)) Foo {
public:
int m_x, m_y;
auto operator<=>(const Foo&) const = default;
};
int main(void) {
Foo a{1,1};
Foo b{1,2};
std::set<Foo> s; // OK
s.insert(a); // OK
std::cout << std::boolalpha
<< (a == b) << ' '
<< (a != b) << ' '
<< (a < b) << ' '
<< (a <= b) << ' '
<< (a > b) << ' '
<< (a >= b) << ' '
<< std::endl;
}
Per the rules for any operator<=> overload, a defaulted <=> overload
will also allow the type to be compared with <, <=, >, and >=. If
operator<=> is defaulted and operator== is not declared at all, then
operator== is implicitly defaulted.
The GIMPLE dump shows:
> __attribute__((cold))
> struct strong_ordering Foo::operator<=> (const struct Foo * const
this, const struct Foo & D.64195)
> __attribute__((cold))
> bool Foo::operator== (const struct Foo * const this, const struct Foo
& D.64206)
I think this makes sense as add_implicitly_declared_members is called
before my injection via finish_struct_1 -> check_bases_and_members.
Yes, but that's because the implicit op== isn't declared lazily like
some other special member functions (CLASSTYPE_LAZY_*/lazily_declare_fn)
which can happen after the class is complete.
---
I would like some comment on the implications of:
- { "cold", 0, 0, true, false, false, false,
+ { "cold", 0, 0, false, false, false, false,
As I am now introducing a new warning that I cannot explain in an old test:
testsuite/g++.dg/Wmissing-attributes.C:55:22: warning: 'hot' attribute
ignored [-Wattributes]
> template <>
> void*
> ATTR ((hot)) ATTR ((alloc_size (1))) ATTR ((malloc))
> missing_all<char>(int); // T = char, same as above
I think this is because attributes in that position appertain to the
return type rather than the declaration. At attribs.cc:753, if an
attribute has decl_required true and we try to apply it to a return
type, we pass it along to the declaration instead; if you change the
true to false we will try to apply it to void* directly and fail.
I think it would work to check for (flags & (ATTR_FLAG_FUNCTION_NEXT |
ATTR_FLAG_DECL_NEXT)) and return without warning in that case. You'd
still set *no_add_attr.
BTW the patch got garbled this time, probably safer to make it an
attachment if you're using gmail.
Jason
