[AMD Public Use] Hey, Jason perhaps you mentioned this somewhere but what is the reason for such a strong aversion to the template approach? It seems to solve the issue nicely with what seems to be a minor change in syntax. gem5 is C++, so we should allow users to write C++.
Tony From: Jason Lowe-Power via gem5-dev <[email protected]> Sent: Friday, September 18, 2020 8:05 AM To: Gabe Black <[email protected]> Cc: gem5 Developer List <[email protected]>; Jason Lowe-Power <[email protected]> Subject: [gem5-dev] Re: A few quick thoughts [CAUTION: External Email] There is another option to keep the function-like syntax, but get the constexpr via templates: A preprocessor macro: #define bits(val, first, last) bits<first,last>(val) The major downside is that we can't overload preprocessor macros. We'd have to have two name bits_const() and bits(), which I also don't like. Personally, I strongly don't want to lose the function-like syntax for the bits function. I won't *block* the change, but I'll push back against the template syntax. Cheers, Jason On Fri, Sep 18, 2020 at 5:02 AM Gabe Black <[email protected]<mailto:[email protected]>> wrote: I spent some more time digging into 2, and while I didn't find anything that directly stated that you aren't allowed to do that, without explicit support I think it flies in the face of how C++ templates, types, etc. work to the point where if you *did* find a way to do it, it would almost certainly be a bug or an oversight in the standard somewhere. So unless they do adopt one of those standards I saw proposed and we wait a good number of years for it to be implemented in all the compilers we support (one said it targeted C++23) I think templates, while slightly gross, are really the only way to make it work. Gabe On Thu, Sep 17, 2020 at 2:53 PM Jason Lowe-Power <[email protected]<mailto:[email protected]>> wrote: On Thu, Sep 17, 2020 at 2:48 PM Gabe Black via gem5-dev <[email protected]<mailto:[email protected]>> wrote: 1. Sounds good, I'll hopefully have some time to put together a CL in no too long (weekend?). 2. I 5ries to figure out a way to do it without the template that wasn't really gross a somewhat fragile and wasn't able to, but that would definitely be preferable. I'll keep thinking about it, but the internet didn't seem to have any ideas either. Unfortunately using constexpr won't work like that Jason, although I wish it did and found a couple unadopted (as far as I know) standards proposals to that effect. Yeah, that's what I found, too :). 3. Sounds good. Likely this weekend? Gabe On Thu, Sep 17, 2020, 1:15 PM Bobby Bruce via gem5-dev <[email protected]<mailto:[email protected]>> wrote: 1) Seems fine to me. 2) I remember looking into this and I agree with Jason, it involves template magic which I'm not a huge fan of. I feel like in order to add these compile time asserts we'd be sacrificing some readability/ease-of-usability of bitfields.hh. This may just be a "me thing", but something about templates confuse me whenever I need to deal with them. 3) In truth, our minimum supported Clang version is 3.9 in practise (We even state on our website's building documentation that we support Clang 3.9 to 9: http://www.gem5.org/documentation/general_docs/building<https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.gem5.org%2Fdocumentation%2Fgeneral_docs%2Fbuilding&data=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571740119&sdata=6hkyKwsWGm%2BY67faMlI2NvDQR21oA6h0L2fDRekCD7o%3D&reserved=0>). I didn't realize we still have "3.1" hardcoded in the SConscript and would be happy for this to be bumped up to 3.9. Our compiler tests do not test with versions of clang before 3.9, so, at present, we aren't doing much to help those using versions older than 3.9. I'd love to bump up to c++14 also. While I'm sure there are plenty of good reasons, I personally would like to use C++14's deprecation attribute for if/when we start deprecating gem5 C++ APIs: https://en.cppreference.com/w/cpp/language/attributes/deprecated<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fcpp%2Flanguage%2Fattributes%2Fdeprecated&data=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571750113&sdata=dazAk2W0t1y04k%2BlD2Eu3acXTZ5YjrGHlLQNMM7PG10%3D&reserved=0> We already do use the deprecated attribute (see https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#55<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgem5.googlesource.com%2Fpublic%2Fgem5%2F%2B%2Frefs%2Fheads%2Fdevelop%2Fsrc%2Fbase%2Fcompiler.hh%2355&data=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571770101&sdata=ZeptqljKIK1C8oAbFd%2Frfz0eRmONiIk1kp4%2BomWoenk%3D&reserved=0>). We should be able to get rid of this: https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#93<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgem5.googlesource.com%2Fpublic%2Fgem5%2F%2B%2Frefs%2Fheads%2Fdevelop%2Fsrc%2Fbase%2Fcompiler.hh%2393&data=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571770101&sdata=ufibvsE%2FQ7NY7IupAGJATGRxV6NWGvUCdPbiWX47tX8%3D&reserved=0> And maybe this: https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#69<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgem5.googlesource.com%2Fpublic%2Fgem5%2F%2B%2Frefs%2Fheads%2Fdevelop%2Fsrc%2Fbase%2Fcompiler.hh%2369&data=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571780090&sdata=WoB63rD6sFKr81ufEMRQH0YEMLipELBGmApFrW9uoDw%3D&reserved=0> -- Dr. Bobby R. Bruce Room 2235, Kemper Hall, UC Davis Davis, CA, 95616 web: https://www.bobbybruce.net<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.bobbybruce.net%2F&data=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571780090&sdata=2B7Shdd7ko8cltS6slVYXSC84F3DCOkfcm04SaLxD80%3D&reserved=0> On Thu, Sep 17, 2020 at 8:25 AM Jason Lowe-Power via gem5-dev <[email protected]<mailto:[email protected]>> wrote: Hey Gabe, On Thu, Sep 17, 2020 at 4:46 AM Gabe Black via gem5-dev <[email protected]<mailto:[email protected]>> wrote: 1. Use __builtin_expect() for panic, fatal, etc. Preexisting library functions like assert probably already have this, but our versions don't and have similar behavior patterns. This should improve performance. Seems like a good idea. It shouldn't hurt anything. 2. Create template versions of the bits, etc functions in bitfields.hh which use static_assert to verify that the bounds are in the right order. Unless the bounds change at runtime (probably very uncommon in practice) I like the idea of static asserts, but don't like the change in the syntax away from a simple function call. Would it be possible to instead use a constexpr function parameter? What we would really like is template <class T> inline T bits(T val, constexpr int first, constexpr int last) { int nbits = first - last + 1; static_assert((first - last) >= 0); return (val >> last) & mask(nbits); } However, after spending 15-30 minutes researching it doesn't seem like this is easy to do today. Gabe... you seem to know much more template magic. Maybe there's a way? bits(foo, 2, 1) => bits<2, 1>(foo) Then we get the free compile time checking of bounds most of the time without big overhead otherwise. Something like this: template <int first, int last, typename T> constexpr T bits(T val) { static_assert(first > last); return bits(val, first, last); } 3. Our new min gcc is version 5 which supports c++14. Our min clang is 3.1 which does not, but 3.4 does. Do we want to bump the min clang version up and move from C++11 to C++14? C++17 has more compelling features, but C++14 fixed some annoyances, at least according to wikipedia: https://en.wikipedia.org/wiki/C%2B%2B14<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wikipedia.org%2Fwiki%2FC%252B%252B14&data=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571790094&sdata=q2adNzwC7u7FVbFATbG%2F%2BG3lokPQ%2BhjtcaJSHHuCnXg%3D&reserved=0> Yeah, I don't see any reason not to bump our minimum clang version. If we do go up to c++14, we can simplify compiler.hh significantly. Thanks for starting this conversation! Cheers, Jason Gabe _______________________________________________ gem5-dev mailing list -- [email protected]<mailto:[email protected]> To unsubscribe send an email to [email protected]<mailto:[email protected]> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s _______________________________________________ gem5-dev mailing list -- [email protected]<mailto:[email protected]> To unsubscribe send an email to [email protected]<mailto:[email protected]> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s _______________________________________________ gem5-dev mailing list -- [email protected]<mailto:[email protected]> To unsubscribe send an email to [email protected]<mailto:[email protected]> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s _______________________________________________ gem5-dev mailing list -- [email protected]<mailto:[email protected]> To unsubscribe send an email to [email protected]<mailto:[email protected]> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________ gem5-dev mailing list -- [email protected] To unsubscribe send an email to [email protected] %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
