[gem5-dev] Re: A few quick thoughts
I agree with all of this.
I'm not sure it's worth the change right now. A quick grep of "bits(" shows
that there's many cases (maybe 5-10%?) where there are variables used for
the bits. I suspect that most of these are constexpr, but it's impossible
to tell without checking each one. Just for ease of this change, I think
that we should keep the same function name in both cases.
Approximately all places bits() is called with only numbers as the 2nd and
3rd arguments.
> grep -E "bits\(.*, *[0-9]{1,2}, *[0-9]{1,2}\)" src -r | wc -l
2271
Approximately all places bits() is called with 3 arguments.
> grep -E "bits\(.*,.*,.*\)" src -r | wc -l
2589
Cheers,
Jason
On Fri, Sep 18, 2020 at 8:49 PM Gabe Black wrote:
> Well, I *think* having not extensively surveyed the code, that the vast
> majority of the time the bits that are being extracted are constant, so the
> templated form would be the default, and you'd just need to use the current
> form in the few cases where it's not fixed. As somebody suggested, we could
> use a macro to make it look a little more uniform and keep the arguments in
> the same order. Perhaps:
>
> #define bits(val, first, last) _bits_template(val)
>
> and call the full flexibility function bitsv (for variable). Most of the
> time you'd use the normal version, except when it yells at you and forces
> you to use bitsv.
>
> This is not ideal in I think three ways.
> 1. Macros are not namespaced. MyFavoritClass::bits would be problematic
> but is totally fine today.
> 2. Slightly more complexity than today. The complexity is *mostly*
> obscured, which can be a good or bad thing depending on circumstances.
> 3. If somebody does copy paste coding and doesn't know *why* something is
> bitsv vs bits, they're more likely to use the wrong one than if they were
> starting from scratch.
>
> Importantly, if somebody *does* use the wrong thing, the only way they
> could do that would be to use the variable form unnecessarily. The other
> way around would definitely be a compiler error. If they do that, then we
> would be no worse off than we are today, we would just lose a little bit of
> the error checking which we have none of now.
>
> I don't think this is a slam dunk obvious right thing to do, but I think
> it has some merits and is worth considering. If in the future somebody does
> either figure out some crazy clever way to do this without the templates,
> or the standard evolves to the point where we can use something more
> elegant, then it should be relatively easy to move to that instead.
>
> Gabe
>
> On Fri, Sep 18, 2020 at 8:35 AM Jason Lowe-Power
> wrote:
>
>> Sorry for the spam...
>>
>> One last thing: We have to keep both bits(val, first, last) *and*
>> bits(val) because sometimes first and last are *not*
>> constexpr. If they were *always* constexpr, this would be much simpler (I
>> think).
>>
>> Cheers,
>> Jason
>>
>> On Fri, Sep 18, 2020 at 8:33 AM Jason Lowe-Power
>> wrote:
>>
>>> I don't like the following change
>>>
>>> bits(val, first, last)
>>>
>>> would now be
>>>
>>> bits(val).
>>>
>>> IMO, it's confusing to put function *parameters* as template arguments.
>>>
>>> This would mean that when you use the bits() function, you'll have to
>>> think about "are these constexpr that I'm using, and, if so, should I use
>>> bits() or bits<>()?"
>>>
>>> We can go through and manually change the current code, but for new
>>> code, this will be yet another thing that we'd have to catch during code
>>> review.
>>>
>>> Just my two cents. I won't block anything, I just think that readability
>>> is more important than a little* performance.
>>>
>>> *It depends on how much performance difference assert() vs
>>> static_assert() is in this case.
>>>
>>> Cheers,
>>> Jason
>>>
>>> On Fri, Sep 18, 2020 at 8:29 AM Gutierrez, Anthony <
>>> anthony.gutier...@amd.com> wrote:
>>>
>>>> [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
>>>> *Sent
[gem5-dev] Re: A few quick thoughts
Well, I *think* having not extensively surveyed the code, that the vast majority of the time the bits that are being extracted are constant, so the templated form would be the default, and you'd just need to use the current form in the few cases where it's not fixed. As somebody suggested, we could use a macro to make it look a little more uniform and keep the arguments in the same order. Perhaps: #define bits(val, first, last) _bits_template(val) and call the full flexibility function bitsv (for variable). Most of the time you'd use the normal version, except when it yells at you and forces you to use bitsv. This is not ideal in I think three ways. 1. Macros are not namespaced. MyFavoritClass::bits would be problematic but is totally fine today. 2. Slightly more complexity than today. The complexity is *mostly* obscured, which can be a good or bad thing depending on circumstances. 3. If somebody does copy paste coding and doesn't know *why* something is bitsv vs bits, they're more likely to use the wrong one than if they were starting from scratch. Importantly, if somebody *does* use the wrong thing, the only way they could do that would be to use the variable form unnecessarily. The other way around would definitely be a compiler error. If they do that, then we would be no worse off than we are today, we would just lose a little bit of the error checking which we have none of now. I don't think this is a slam dunk obvious right thing to do, but I think it has some merits and is worth considering. If in the future somebody does either figure out some crazy clever way to do this without the templates, or the standard evolves to the point where we can use something more elegant, then it should be relatively easy to move to that instead. Gabe On Fri, Sep 18, 2020 at 8:35 AM Jason Lowe-Power wrote: > Sorry for the spam... > > One last thing: We have to keep both bits(val, first, last) *and* > bits(val) because sometimes first and last are *not* > constexpr. If they were *always* constexpr, this would be much simpler (I > think). > > Cheers, > Jason > > On Fri, Sep 18, 2020 at 8:33 AM Jason Lowe-Power > wrote: > >> I don't like the following change >> >> bits(val, first, last) >> >> would now be >> >> bits(val). >> >> IMO, it's confusing to put function *parameters* as template arguments. >> >> This would mean that when you use the bits() function, you'll have to >> think about "are these constexpr that I'm using, and, if so, should I use >> bits() or bits<>()?" >> >> We can go through and manually change the current code, but for new code, >> this will be yet another thing that we'd have to catch during code review. >> >> Just my two cents. I won't block anything, I just think that readability >> is more important than a little* performance. >> >> *It depends on how much performance difference assert() vs >> static_assert() is in this case. >> >> Cheers, >> Jason >> >> On Fri, Sep 18, 2020 at 8:29 AM Gutierrez, Anthony < >> anthony.gutier...@amd.com> wrote: >> >>> [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 >>> *Sent:* Friday, September 18, 2020 8:05 AM >>> *To:* Gabe Black >>> *Cc:* gem5 Developer List ; Jason Lowe-Power < >>> ja...@lowepower.com> >>> *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(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
[gem5-dev] Re: A few quick thoughts
Sorry for the spam... One last thing: We have to keep both bits(val, first, last) *and* bits(val) because sometimes first and last are *not* constexpr. If they were *always* constexpr, this would be much simpler (I think). Cheers, Jason On Fri, Sep 18, 2020 at 8:33 AM Jason Lowe-Power wrote: > I don't like the following change > > bits(val, first, last) > > would now be > > bits(val). > > IMO, it's confusing to put function *parameters* as template arguments. > > This would mean that when you use the bits() function, you'll have to > think about "are these constexpr that I'm using, and, if so, should I use > bits() or bits<>()?" > > We can go through and manually change the current code, but for new code, > this will be yet another thing that we'd have to catch during code review. > > Just my two cents. I won't block anything, I just think that readability > is more important than a little* performance. > > *It depends on how much performance difference assert() vs static_assert() > is in this case. > > Cheers, > Jason > > On Fri, Sep 18, 2020 at 8:29 AM Gutierrez, Anthony < > anthony.gutier...@amd.com> wrote: > >> [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 >> *Sent:* Friday, September 18, 2020 8:05 AM >> *To:* Gabe Black >> *Cc:* gem5 Developer List ; Jason Lowe-Power < >> ja...@lowepower.com> >> *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(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 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 >> wrote: >> >> >> >> >> >> On Thu, Sep 17, 2020 at 2:48 PM Gabe Black via gem5-dev < >> gem5-dev@gem5.org> 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 >> 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
[gem5-dev] Re: A few quick thoughts
I don't like the following change bits(val, first, last) would now be bits(val). IMO, it's confusing to put function *parameters* as template arguments. This would mean that when you use the bits() function, you'll have to think about "are these constexpr that I'm using, and, if so, should I use bits() or bits<>()?" We can go through and manually change the current code, but for new code, this will be yet another thing that we'd have to catch during code review. Just my two cents. I won't block anything, I just think that readability is more important than a little* performance. *It depends on how much performance difference assert() vs static_assert() is in this case. Cheers, Jason On Fri, Sep 18, 2020 at 8:29 AM Gutierrez, Anthony < anthony.gutier...@amd.com> wrote: > [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 > *Sent:* Friday, September 18, 2020 8:05 AM > *To:* Gabe Black > *Cc:* gem5 Developer List ; Jason Lowe-Power < > ja...@lowepower.com> > *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(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 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 > wrote: > > > > > > On Thu, Sep 17, 2020 at 2:48 PM Gabe Black via gem5-dev > 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 > 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=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571740119=6hkyKwsWGm%2BY67faMlI2NvDQR21oA6h0L2fDRekCD7o%3D=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
[gem5-dev] Re: A few quick thoughts
[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 Sent: Friday, September 18, 2020 8:05 AM To: Gabe Black Cc: gem5 Developer List ; Jason Lowe-Power 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(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 mailto:gabebl...@google.com>> 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 mailto:ja...@lowepower.com>> wrote: On Thu, Sep 17, 2020 at 2:48 PM Gabe Black via gem5-dev mailto:gem5-dev@gem5.org>> 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 mailto:gem5-dev@gem5.org>> 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=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571740119=6hkyKwsWGm%2BY67faMlI2NvDQR21oA6h0L2fDRekCD7o%3D=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=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571750113=dazAk2W0t1y04k%2BlD2Eu3acXTZ5YjrGHlLQNMM7PG10%3D=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=02%7C01%7Canthony.gutierrez%40amd.com%7C8dda55bf862b42f3f16408d85be45700%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637360383571770101=ZeptqljKIK1C8oAbFd%2Frfz0eRmONiIk1kp4%2BomWoenk%3D=0>). We should be able to get rid of this: https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.h
[gem5-dev] Re: A few quick thoughts
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(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 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 > wrote: > >> >> >> On Thu, Sep 17, 2020 at 2:48 PM Gabe Black via gem5-dev < >> gem5-dev@gem5.org> 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 < >>> gem5-dev@gem5.org> 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). 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 >>> >> We already do use the deprecated attribute (see >> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#55 >> ). >> >> We should be able to get rid of this: >> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#93 >> And maybe this: >> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#69 >> >> >>> -- Dr. Bobby R. Bruce Room 2235, Kemper Hall, UC Davis Davis, CA, 95616 web: https://www.bobbybruce.net On Thu, Sep 17, 2020 at 8:25 AM Jason Lowe-Power via gem5-dev < gem5-dev@gem5.org> wrote: > Hey Gabe, > > On Thu, Sep 17, 2020 at 4:46 AM Gabe Black via gem5-dev < > gem5-dev@gem5.org> 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 > inline > T > bits(T val, *constexpr *int
[gem5-dev] Re: A few quick thoughts
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
wrote:
>
>
> On Thu, Sep 17, 2020 at 2:48 PM Gabe Black via gem5-dev
> 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
>> 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). 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
>>>
>>
> We already do use the deprecated attribute (see
> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#55
> ).
>
> We should be able to get rid of this:
> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#93
> And maybe this:
> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#69
>
>
>>
>>> --
>>> Dr. Bobby R. Bruce
>>> Room 2235,
>>> Kemper Hall, UC Davis
>>> Davis,
>>> CA, 95616
>>>
>>> web: https://www.bobbybruce.net
>>>
>>>
>>> On Thu, Sep 17, 2020 at 8:25 AM Jason Lowe-Power via gem5-dev <
>>> gem5-dev@gem5.org> wrote:
>>>
Hey Gabe,
On Thu, Sep 17, 2020 at 4:46 AM Gabe Black via gem5-dev <
gem5-dev@gem5.org> 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
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
> constexpr T
> bits(T val)
> {
> static_assert(first > last);
> return bits(val, first, last);
> }
>
>
[gem5-dev] Re: A few quick thoughts
On Thu, Sep 17, 2020 at 2:48 PM Gabe Black via gem5-dev
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
> 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). 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
>>
>
We already do use the deprecated attribute (see
https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#55
).
We should be able to get rid of this:
https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#93
And maybe this:
https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/base/compiler.hh#69
>
>> --
>> Dr. Bobby R. Bruce
>> Room 2235,
>> Kemper Hall, UC Davis
>> Davis,
>> CA, 95616
>>
>> web: https://www.bobbybruce.net
>>
>>
>> On Thu, Sep 17, 2020 at 8:25 AM Jason Lowe-Power via gem5-dev <
>> gem5-dev@gem5.org> wrote:
>>
>>> Hey Gabe,
>>>
>>> On Thu, Sep 17, 2020 at 4:46 AM Gabe Black via gem5-dev <
>>> gem5-dev@gem5.org> 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
>>> 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
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
>>>
>>> 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 -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Re: A few quick thoughts
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.
3. Sounds good. Likely this weekend?
Gabe
On Thu, Sep 17, 2020, 1:15 PM Bobby Bruce via gem5-dev
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). 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
>
> --
> Dr. Bobby R. Bruce
> Room 2235,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
>
> On Thu, Sep 17, 2020 at 8:25 AM Jason Lowe-Power via gem5-dev <
> gem5-dev@gem5.org> wrote:
>
>> Hey Gabe,
>>
>> On Thu, Sep 17, 2020 at 4:46 AM Gabe Black via gem5-dev <
>> gem5-dev@gem5.org> 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
>> 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
>>> 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
>>>
>>
>> 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 -- gem5-dev@gem5.org
>>> To unsubscribe send an email to gem5-dev-le...@gem5.org
>>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>>
>> ___
>> gem5-dev mailing list -- gem5-dev@gem5.org
>> To unsubscribe send an email to gem5-dev-le...@gem5.org
>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>
> ___
> gem5-dev mailing list -- gem5-dev@gem5.org
> To unsubscribe send an email to gem5-dev-le...@gem5.org
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
[gem5-dev] Re: A few quick thoughts
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). 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
--
Dr. Bobby R. Bruce
Room 2235,
Kemper Hall, UC Davis
Davis,
CA, 95616
web: https://www.bobbybruce.net
On Thu, Sep 17, 2020 at 8:25 AM Jason Lowe-Power via gem5-dev <
gem5-dev@gem5.org> wrote:
> Hey Gabe,
>
> On Thu, Sep 17, 2020 at 4:46 AM Gabe Black via gem5-dev
> 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
> 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
>> 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
>>
>
> 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 -- gem5-dev@gem5.org
>> To unsubscribe send an email to gem5-dev-le...@gem5.org
>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>
> ___
> gem5-dev mailing list -- gem5-dev@gem5.org
> To unsubscribe send an email to gem5-dev-le...@gem5.org
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Re: A few quick thoughts
Hey Gabe,
On Thu, Sep 17, 2020 at 4:46 AM Gabe Black via gem5-dev
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
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
> 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
>
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 -- gem5-dev@gem5.org
> To unsubscribe send an email to gem5-dev-le...@gem5.org
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
