Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc wrote:
>
> On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
> wrote:
> >
> > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc wrote:
> > >
> > > This came up when I was asking around about what the proper way was to:
> > >
> > > - Allocate aligned storage for a buffer pool/page cache
> > > - Then create pointers to "Page" structs inside of the storage memory area
> > >
> > > I thought something like this might do:
> > >
> > > struct buffer_pool
> > > {
> > > alignas(PAGE_SIZE) std::byte storage[NUM_PAGES * PAGE_SIZE];
> > > page* pages = new (storage) page[NUM_PAGES];
> > > }
> > >
> > > Someone told me that this was a valid solution but not to do it, because
> > > it
> > > wouldn't function properly on GCC
> > > They gave this as a reproduction:
> > >
> > > https://godbolt.org/z/EhzM37Gzh
> > >
> > > I'm not experienced enough with C++ to grok the connection between this
> > > repro and my code,
> >
> > Me neither. I don't think there is any connection, because I don't
> > think the repro shows what they think it shows.
> >
> > > but I figured
> > > I'd post it on the mailing list in case it was useful for others/might get
> > > fixed in the future =)
> > >
> > > They said it had to do with "handling of lifetimes of implicit-lifetime
> > > types"
> >
> > I don't think that code is a valid implementation of
> > start_lifetime_as. Without a proper implementation of
> > start_lifetime_as (which GCC doesn't provide yet), GCC does not allow
> > you to read the bytes of a float as an int, and doesn't give you the
> > bytes of 1.0f, it gives you 0.
> >
> > https://godbolt.org/z/dvncY9Pea works for GCC. But this has nothing to
> > do your code above, as far as I can see.
>
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107115#c10 for what
> is going wrong.
> Basically GCC does not have a way to express this in the IR currently
> and there are proposals there on how to do it.
I wouldn't call them "proposals" - basically the C++ language providing
holes into the TBAA system is a misdesign, it will be incredibly difficult
to implement this "hole" without sacrifying optimization which means
people will complain endlessly why std::start_lifetime_as isn't a way
to circumvent TBAA without losing optimization.
But yes, I think std::start_lifetime_as needs to be implemented in the
C++ frontend and not in the library. As a band-aid that works you can
use
template
auto start_lifetime_as(void* p) noexcept -> T* {
__asm__ volatile ("" : : "g" (p) : "memory");
return std::launder((T*)p);
}
that will a) force what is pointed to be by 'p' addressable and
b) make all aliased memory considered clobbered (including *p).
Have fun with that.
In the end we'd need something like this, for less optimization
effect maybe with a way to specify that only *(T *)p is clobbered.
template
auto start_lifetime_as(void* p) noexcept -> T* {
typedef T Tp __attribute__((may_alias));
__asm__ volatile ("" : "=m" (*(Tp *)p) : "g" (p), "m" (*(Tp *)p));
return std::launder((T*)p);
}
might work, but the typedef/casting/dereferencing might be
problematic for some 'T'. Note on the GIMPLE level asms
with memory inputs/outputs are hard barriers for everything.
Richard.
> Thanks,
> Andrew Pinski
Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
On Sun, 11 Dec 2022, 09:12 Richard Biener,
wrote:
> On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc
> wrote:
> >
> > On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
> > wrote:
> > >
> > > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc
> wrote:
> > > >
> > > > This came up when I was asking around about what the proper way was
> to:
> > > >
> > > > - Allocate aligned storage for a buffer pool/page cache
> > > > - Then create pointers to "Page" structs inside of the storage
> memory area
> > > >
> > > > I thought something like this might do:
> > > >
> > > > struct buffer_pool
> > > > {
> > > > alignas(PAGE_SIZE) std::byte storage[NUM_PAGES * PAGE_SIZE];
> > > > page* pages = new (storage) page[NUM_PAGES];
> > > > }
> > > >
> > > > Someone told me that this was a valid solution but not to do it,
> because it
> > > > wouldn't function properly on GCC
> > > > They gave this as a reproduction:
> > > >
> > > > https://godbolt.org/z/EhzM37Gzh
> > > >
> > > > I'm not experienced enough with C++ to grok the connection between
> this
> > > > repro and my code,
> > >
> > > Me neither. I don't think there is any connection, because I don't
> > > think the repro shows what they think it shows.
> > >
> > > > but I figured
> > > > I'd post it on the mailing list in case it was useful for
> others/might get
> > > > fixed in the future =)
> > > >
> > > > They said it had to do with "handling of lifetimes of
> implicit-lifetime
> > > > types"
> > >
> > > I don't think that code is a valid implementation of
> > > start_lifetime_as. Without a proper implementation of
> > > start_lifetime_as (which GCC doesn't provide yet), GCC does not allow
> > > you to read the bytes of a float as an int, and doesn't give you the
> > > bytes of 1.0f, it gives you 0.
> > >
> > > https://godbolt.org/z/dvncY9Pea works for GCC. But this has nothing to
> > > do your code above, as far as I can see.
> >
> > See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107115#c10 for what
> > is going wrong.
> > Basically GCC does not have a way to express this in the IR currently
> > and there are proposals there on how to do it.
>
> I wouldn't call them "proposals" - basically the C++ language providing
> holes into the TBAA system is a misdesign, it will be incredibly difficult
> to implement this "hole" without sacrifying optimization which means
> people will complain endlessly why std::start_lifetime_as isn't a way
> to circumvent TBAA without losing optimization.
>
People already make holes in the type system, this just lets them do it
without UB. If it's not as fast as their UB, that's ok IMHO.
>
But I don't see what start_lifetime_as has to do with the original problem
anyway. The placement new expression will start lifetimes:
page* pages = new (storage) page[NUM_PAGES];
There's no need to mess with the type system here.
[no subject]
To kill time kill you
Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
On Sun, Dec 11, 2022 at 1:02 PM Jonathan Wakely wrote:
>
>
>
> On Sun, 11 Dec 2022, 09:12 Richard Biener, wrote:
>>
>> On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc
>> wrote:
>> >
>> > On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
>> > wrote:
>> > >
>> > > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc wrote:
>> > > >
>> > > > This came up when I was asking around about what the proper way was to:
>> > > >
>> > > > - Allocate aligned storage for a buffer pool/page cache
>> > > > - Then create pointers to "Page" structs inside of the storage memory
>> > > > area
>> > > >
>> > > > I thought something like this might do:
>> > > >
>> > > > struct buffer_pool
>> > > > {
>> > > > alignas(PAGE_SIZE) std::byte storage[NUM_PAGES * PAGE_SIZE];
>> > > > page* pages = new (storage) page[NUM_PAGES];
>> > > > }
>> > > >
>> > > > Someone told me that this was a valid solution but not to do it,
>> > > > because it
>> > > > wouldn't function properly on GCC
>> > > > They gave this as a reproduction:
>> > > >
>> > > > https://godbolt.org/z/EhzM37Gzh
>> > > >
>> > > > I'm not experienced enough with C++ to grok the connection between this
>> > > > repro and my code,
>> > >
>> > > Me neither. I don't think there is any connection, because I don't
>> > > think the repro shows what they think it shows.
>> > >
>> > > > but I figured
>> > > > I'd post it on the mailing list in case it was useful for others/might
>> > > > get
>> > > > fixed in the future =)
>> > > >
>> > > > They said it had to do with "handling of lifetimes of implicit-lifetime
>> > > > types"
>> > >
>> > > I don't think that code is a valid implementation of
>> > > start_lifetime_as. Without a proper implementation of
>> > > start_lifetime_as (which GCC doesn't provide yet), GCC does not allow
>> > > you to read the bytes of a float as an int, and doesn't give you the
>> > > bytes of 1.0f, it gives you 0.
>> > >
>> > > https://godbolt.org/z/dvncY9Pea works for GCC. But this has nothing to
>> > > do your code above, as far as I can see.
>> >
>> > See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107115#c10 for what
>> > is going wrong.
>> > Basically GCC does not have a way to express this in the IR currently
>> > and there are proposals there on how to do it.
>>
>> I wouldn't call them "proposals" - basically the C++ language providing
>> holes into the TBAA system is a misdesign, it will be incredibly difficult
>> to implement this "hole" without sacrifying optimization which means
>> people will complain endlessly why std::start_lifetime_as isn't a way
>> to circumvent TBAA without losing optimization.
>
>
> People already make holes in the type system, this just lets them do it
> without UB. If it's not as fast as their UB, that's ok IMHO.
>
>
>
> But I don't see what start_lifetime_as has to do with the original problem
> anyway. The placement new expression will start lifetimes:
>
> page* pages = new (storage) page[NUM_PAGES];
>
> There's no need to mess with the type system here.
That's true, and that should work, not sure what the problem should be here.
Richard.
>
>
Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
@Richard
That's some intense code, I appreciate the code-samples and explanation,
thank you =)
@Jonathan
Maybe there was some misunderstanding?
I didn't make the connection either but I also don't know that much about
C++
It seems like that expression is valid then? Good to know =)
As a random aside if I may -- what is the difference between placement-new
of pointers in
std::byte storage, and making a std::span over the storage area?
std::byte storage[PAGE_SIZE * NUM_PAGES];
// A)
page* pages = new (storage) page[NUM_PAGES];
// B)
std::span pages_span(pages, NUM_PAGES);
On Sun, Dec 11, 2022 at 8:31 AM Richard Biener
wrote:
> On Sun, Dec 11, 2022 at 1:02 PM Jonathan Wakely
> wrote:
> >
> >
> >
> > On Sun, 11 Dec 2022, 09:12 Richard Biener,
> wrote:
> >>
> >> On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc
> wrote:
> >> >
> >> > On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
> >> > wrote:
> >> > >
> >> > > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc
> wrote:
> >> > > >
> >> > > > This came up when I was asking around about what the proper way
> was to:
> >> > > >
> >> > > > - Allocate aligned storage for a buffer pool/page cache
> >> > > > - Then create pointers to "Page" structs inside of the storage
> memory area
> >> > > >
> >> > > > I thought something like this might do:
> >> > > >
> >> > > > struct buffer_pool
> >> > > > {
> >> > > > alignas(PAGE_SIZE) std::byte storage[NUM_PAGES * PAGE_SIZE];
> >> > > > page* pages = new (storage) page[NUM_PAGES];
> >> > > > }
> >> > > >
> >> > > > Someone told me that this was a valid solution but not to do it,
> because it
> >> > > > wouldn't function properly on GCC
> >> > > > They gave this as a reproduction:
> >> > > >
> >> > > > https://godbolt.org/z/EhzM37Gzh
> >> > > >
> >> > > > I'm not experienced enough with C++ to grok the connection
> between this
> >> > > > repro and my code,
> >> > >
> >> > > Me neither. I don't think there is any connection, because I don't
> >> > > think the repro shows what they think it shows.
> >> > >
> >> > > > but I figured
> >> > > > I'd post it on the mailing list in case it was useful for
> others/might get
> >> > > > fixed in the future =)
> >> > > >
> >> > > > They said it had to do with "handling of lifetimes of
> implicit-lifetime
> >> > > > types"
> >> > >
> >> > > I don't think that code is a valid implementation of
> >> > > start_lifetime_as. Without a proper implementation of
> >> > > start_lifetime_as (which GCC doesn't provide yet), GCC does not
> allow
> >> > > you to read the bytes of a float as an int, and doesn't give you the
> >> > > bytes of 1.0f, it gives you 0.
> >> > >
> >> > > https://godbolt.org/z/dvncY9Pea works for GCC. But this has
> nothing to
> >> > > do your code above, as far as I can see.
> >> >
> >> > See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107115#c10 for what
> >> > is going wrong.
> >> > Basically GCC does not have a way to express this in the IR currently
> >> > and there are proposals there on how to do it.
> >>
> >> I wouldn't call them "proposals" - basically the C++ language providing
> >> holes into the TBAA system is a misdesign, it will be incredibly
> difficult
> >> to implement this "hole" without sacrifying optimization which means
> >> people will complain endlessly why std::start_lifetime_as isn't a way
> >> to circumvent TBAA without losing optimization.
> >
> >
> > People already make holes in the type system, this just lets them do it
> without UB. If it's not as fast as their UB, that's ok IMHO.
> >
> >
> >
> > But I don't see what start_lifetime_as has to do with the original
> problem anyway. The placement new expression will start lifetimes:
> >
> > page* pages = new (storage) page[NUM_PAGES];
> >
> > There's no need to mess with the type system here.
>
> That's true, and that should work, not sure what the problem should be
> here.
>
> Richard.
>
> >
> >
>
Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
Whoops, the last line should be pages_span(storage, ...)
On Sun, Dec 11, 2022 at 8:38 AM Gavin Ray wrote:
> @Richard
>
> That's some intense code, I appreciate the code-samples and explanation,
> thank you =)
>
> @Jonathan
>
> Maybe there was some misunderstanding?
> I didn't make the connection either but I also don't know that much about
> C++
>
> It seems like that expression is valid then? Good to know =)
>
> As a random aside if I may -- what is the difference between placement-new
> of pointers in
> std::byte storage, and making a std::span over the storage area?
>
> std::byte storage[PAGE_SIZE * NUM_PAGES];
>
> // A)
> page* pages = new (storage) page[NUM_PAGES];
> // B)
> std::span pages_span(pages, NUM_PAGES);
>
>
> On Sun, Dec 11, 2022 at 8:31 AM Richard Biener
> wrote:
>
>> On Sun, Dec 11, 2022 at 1:02 PM Jonathan Wakely
>> wrote:
>> >
>> >
>> >
>> > On Sun, 11 Dec 2022, 09:12 Richard Biener,
>> wrote:
>> >>
>> >> On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc
>> wrote:
>> >> >
>> >> > On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
>> >> > wrote:
>> >> > >
>> >> > > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc
>> wrote:
>> >> > > >
>> >> > > > This came up when I was asking around about what the proper way
>> was to:
>> >> > > >
>> >> > > > - Allocate aligned storage for a buffer pool/page cache
>> >> > > > - Then create pointers to "Page" structs inside of the storage
>> memory area
>> >> > > >
>> >> > > > I thought something like this might do:
>> >> > > >
>> >> > > > struct buffer_pool
>> >> > > > {
>> >> > > > alignas(PAGE_SIZE) std::byte storage[NUM_PAGES * PAGE_SIZE];
>> >> > > > page* pages = new (storage) page[NUM_PAGES];
>> >> > > > }
>> >> > > >
>> >> > > > Someone told me that this was a valid solution but not to do it,
>> because it
>> >> > > > wouldn't function properly on GCC
>> >> > > > They gave this as a reproduction:
>> >> > > >
>> >> > > > https://godbolt.org/z/EhzM37Gzh
>> >> > > >
>> >> > > > I'm not experienced enough with C++ to grok the connection
>> between this
>> >> > > > repro and my code,
>> >> > >
>> >> > > Me neither. I don't think there is any connection, because I don't
>> >> > > think the repro shows what they think it shows.
>> >> > >
>> >> > > > but I figured
>> >> > > > I'd post it on the mailing list in case it was useful for
>> others/might get
>> >> > > > fixed in the future =)
>> >> > > >
>> >> > > > They said it had to do with "handling of lifetimes of
>> implicit-lifetime
>> >> > > > types"
>> >> > >
>> >> > > I don't think that code is a valid implementation of
>> >> > > start_lifetime_as. Without a proper implementation of
>> >> > > start_lifetime_as (which GCC doesn't provide yet), GCC does not
>> allow
>> >> > > you to read the bytes of a float as an int, and doesn't give you
>> the
>> >> > > bytes of 1.0f, it gives you 0.
>> >> > >
>> >> > > https://godbolt.org/z/dvncY9Pea works for GCC. But this has
>> nothing to
>> >> > > do your code above, as far as I can see.
>> >> >
>> >> > See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107115#c10 for what
>> >> > is going wrong.
>> >> > Basically GCC does not have a way to express this in the IR currently
>> >> > and there are proposals there on how to do it.
>> >>
>> >> I wouldn't call them "proposals" - basically the C++ language providing
>> >> holes into the TBAA system is a misdesign, it will be incredibly
>> difficult
>> >> to implement this "hole" without sacrifying optimization which means
>> >> people will complain endlessly why std::start_lifetime_as isn't a way
>> >> to circumvent TBAA without losing optimization.
>> >
>> >
>> > People already make holes in the type system, this just lets them do it
>> without UB. If it's not as fast as their UB, that's ok IMHO.
>> >
>> >
>> >
>> > But I don't see what start_lifetime_as has to do with the original
>> problem anyway. The placement new expression will start lifetimes:
>> >
>> > page* pages = new (storage) page[NUM_PAGES];
>> >
>> > There's no need to mess with the type system here.
>>
>> That's true, and that should work, not sure what the problem should be
>> here.
>>
>> Richard.
>>
>> >
>> >
>>
>
gcc-13-20221211 is now available
Snapshot gcc-13-20221211 is now available on https://gcc.gnu.org/pub/gcc/snapshots/13-20221211/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 13 git branch with the following options: git://gcc.gnu.org/git/gcc.git branch master revision 8f72249ff4cbd0a5c701b99ee8aa1ca9d82df046 You'll find: gcc-13-20221211.tar.xz Complete GCC SHA256=9872154ce3d27677445c1d3b825a9cb14bea2de8abe1f6064afe2b5068f0ca1a SHA1=12fafe820d6246d8657dfc79d87b09313d081934 Diffs from 13-20221204 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-13 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
