-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Potential lack of keyword final #23
Comments
As far as I can see the current version has 'override' on that line, maybe you refere to an older version?
The keyword override in itself doesn't add or remove the need for a vtable: that is cause by the 'virtual' in the supercalss (in this case whlib::pin_out).
Wouter van Ooijen
0638150444 - HL15 4.060
…________________________________
From: Xenoamor <[email protected]>
Sent: Sunday, June 30, 2019 2:07 PM
To: wovo/hwlib
Cc: Subscribed
Subject: [wovo/hwlib] Potential lack of keyword final (#23)
https://github.com/wovo/hwlib/blob/23d687944e7ce90c99c9c2b20f113cce3948c3f4/library/pins/hwlib-pin-all.hpp#L63
Looking at this line should the keyword final not be used? If this is committed then a vtable would need to be created and the generated code would bloat.
I've made an example of this on godbolt<https://godbolt.org/z/S7DlGk>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#23?email_source=notifications&email_token=ACE643BT7IKJ6GU7L7K4HXLP5COXHA5CNFSM4H4MNGZKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G4PUJCQ>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACE643ENA34W273RFDFJG7LP5COXHANCNFSM4H4MNGZA>.
|
Hi Wouter, I think you might have misunderstood me. It is correct to have the keyword override but it would be beneficial to also have the keyword final. I'd also like to add that I saw your talk on youtube from a recommendation on Reddit and would like to thank you for it! It's been a great help in selling me on moving to C++ |
OK, that makes sense. I didn't use final anywhere (yet), but I think there are no disadvantages. But In that case I should probably make the class final.
I'll give it a try (soon).
Wouter van Ooijen
0638150444 - HL15 4.060
…________________________________
From: Xenoamor <[email protected]>
Sent: Sunday, June 30, 2019 9:40 PM
To: wovo/hwlib
Cc: Wouter van Ooijen; Comment
Subject: Re: [wovo/hwlib] Potential lack of keyword final (#23)
Hi Wouter, I think you might have misunderstood me. It is correct to have the keyword override but it would be beneficial to also have the keyword final.
If you do then the compiler can de-virtualise it<https://marcofoco.com/the-power-of-devirtualization/> and remove the overheads associated with it
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub<#23?email_source=notifications&email_token=ACE643ADZCMAVMDSBK5ESATP5ED4XA5CNFSM4H4MNGZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY4SMGI#issuecomment-507061785>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACE643GA6XZYM5OE2JQ6OT3P5ED4XANCNFSM4H4MNGZA>.
|
Awesome, it'll be interesting to know if it becomes as fast as a templated version. I imagine link time optimisation might be needed for that though |
I think even LTO won't optimize as agressively as a template-based version, especially not when multiple ports are used that are passed to multiple functions. IIRC a single port, passed to a single function, is indeed optimized. Tried that, a few years ago.
And with the templated version (godafoss) things are so much easier because not 'inbetween' objects are needed.
Wouter van Ooijen
0638150444 - HL15 4.060
…________________________________
From: Xenoamor <[email protected]>
Sent: Sunday, June 30, 2019 9:58 PM
To: wovo/hwlib
Cc: Wouter van Ooijen; Comment
Subject: Re: [wovo/hwlib] Potential lack of keyword final (#23)
Awesome, it'll be interesting to know if it becomes as fast as a templated version. I imagine link time optimisation might be needed for that though
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub<#23?email_source=notifications&email_token=ACE643FEGRM75XRXU3I7T43P5EF6TA5CNFSM4H4MNGZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY4SVRY#issuecomment-507062983>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACE643E67VNUPD7SCOBRISLP5EF6TANCNFSM4H4MNGZA>.
|
Yes I see this now, I've played around with it in godbolt and you only get the savings from I'll have a look at godafoss when I get some free time! |
hwlib/library/pins/hwlib-pin-all.hpp
Line 63 in 23d6879
Looking at this line should the keyword final not be used? If this is omitted then a vtable would need to be created and the generated code would bloat.
I've made an example of this on godbolt
The text was updated successfully, but these errors were encountered: