public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Bug with GCC's handling of lifetimes of implicit-lifetime types
@ 2022-12-10 17:41 Gavin Ray
  2022-12-10 18:35 ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Gavin Ray @ 2022-12-10 17:41 UTC (permalink / raw)
  To: gcc

[-- Attachment #1: Type: text/plain, Size: 841 bytes --]

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, 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"

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-10 17:41 Bug with GCC's handling of lifetimes of implicit-lifetime types Gavin Ray
@ 2022-12-10 18:35 ` Jonathan Wakely
  2022-12-10 18:43   ` Andrew Pinski
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2022-12-10 18:35 UTC (permalink / raw)
  To: Gavin Ray; +Cc: gcc

On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc <gcc@gcc.gnu.org> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-10 18:35 ` Jonathan Wakely
@ 2022-12-10 18:43   ` Andrew Pinski
  2022-12-10 21:48     ` Gavin Ray
  2022-12-11  9:11     ` Richard Biener
  0 siblings, 2 replies; 10+ messages in thread
From: Andrew Pinski @ 2022-12-10 18:43 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Gavin Ray, gcc

On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
<gcc@gcc.gnu.org> wrote:
>
> On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc <gcc@gcc.gnu.org> 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.

Thanks,
Andrew Pinski

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-10 18:43   ` Andrew Pinski
@ 2022-12-10 21:48     ` Gavin Ray
  2022-12-11  9:11     ` Richard Biener
  1 sibling, 0 replies; 10+ messages in thread
From: Gavin Ray @ 2022-12-10 21:48 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: Jonathan Wakely, gcc

[-- Attachment #1: Type: text/plain, Size: 3533 bytes --]

Ahh alright, thanks Jonathan & Andrew, appreciate the replies

@Jonathan
> ... "Without a proper implementation of start_lifetime_as (which GCC
doesn't provide yet)"

I mailed the author of that proposal yesterday after learning about it
(it's very useful!) and they told me as much
Had written them to ask about a naive implementation I tried to see if I
understood it correctly:
- P2590R2: "Explicit lifetime management" (start_lifetime_as)
implementation (github.com)
<https://gist.github.com/GavinRay97/b69cb6ebab6a0e13d2cfe74a6ed7cdc6>


>
>
>
> *"Your code copies bytes around. The compiler *might* optimise that away
> but I'mnot sure. The idea of start_lifetime_as is that it would compile
> down to noinstructions - it would always be a no-op at runtime. My
> understanding is thatit cannot be implemented by the user in C++, the
> implementation would have to bea "magic" function using compiler
> intrinsics."*


About the memmove thing, based on a Godbolt link I found from Google -- I
think it's relying on UB to initialize storage + lifetime?
- Compiler Explorer (godbolt.org) <https://godbolt.org/z/VuRtNC>

But I don't pretend to understand the technicalities of the above, ha.

@Andrew

Would you be willing to attempt to explain the linked issue to someone not
familiar with the details of C++'s object storage + lifetime model?
Seems like an interesting thing but I don't have the technical
background/context to understand the full discussion happening there.






On Sat, Dec 10, 2022 at 1:44 PM Andrew Pinski <pinskia@gmail.com> wrote:

> On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
> <gcc@gcc.gnu.org> wrote:
> >
> > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc <gcc@gcc.gnu.org> 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.
>
> Thanks,
> Andrew Pinski
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-10 18:43   ` Andrew Pinski
  2022-12-10 21:48     ` Gavin Ray
@ 2022-12-11  9:11     ` Richard Biener
  2022-12-11 12:02       ` Jonathan Wakely
  1 sibling, 1 reply; 10+ messages in thread
From: Richard Biener @ 2022-12-11  9:11 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: Jonathan Wakely, Gavin Ray, gcc

On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc <gcc@gcc.gnu.org> wrote:
>
> On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
> <gcc@gcc.gnu.org> wrote:
> >
> > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc <gcc@gcc.gnu.org> 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 <typename T>
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 <typename T>
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-11  9:11     ` Richard Biener
@ 2022-12-11 12:02       ` Jonathan Wakely
  2022-12-11 13:31         ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2022-12-11 12:02 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew Pinski, Gavin Ray, gcc

[-- Attachment #1: Type: text/plain, Size: 2936 bytes --]

On Sun, 11 Dec 2022, 09:12 Richard Biener, <richard.guenther@gmail.com>
wrote:

> On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc <gcc@gcc.gnu.org>
> wrote:
> >
> > On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
> > <gcc@gcc.gnu.org> wrote:
> > >
> > > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc <gcc@gcc.gnu.org>
> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-11 12:02       ` Jonathan Wakely
@ 2022-12-11 13:31         ` Richard Biener
  2022-12-11 13:38           ` Gavin Ray
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2022-12-11 13:31 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Andrew Pinski, Gavin Ray, gcc

On Sun, Dec 11, 2022 at 1:02 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
>
>
> On Sun, 11 Dec 2022, 09:12 Richard Biener, <richard.guenther@gmail.com> wrote:
>>
>> On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc <gcc@gcc.gnu.org> wrote:
>> >
>> > On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
>> > <gcc@gcc.gnu.org> wrote:
>> > >
>> > > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc <gcc@gcc.gnu.org> 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.

>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-11 13:31         ` Richard Biener
@ 2022-12-11 13:38           ` Gavin Ray
  2022-12-11 13:44             ` Gavin Ray
  0 siblings, 1 reply; 10+ messages in thread
From: Gavin Ray @ 2022-12-11 13:38 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jonathan Wakely, Andrew Pinski, gcc

[-- Attachment #1: Type: text/plain, Size: 4055 bytes --]

@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<page, NUM_PAGES> pages_span(pages, NUM_PAGES);


On Sun, Dec 11, 2022 at 8:31 AM Richard Biener <richard.guenther@gmail.com>
wrote:

> On Sun, Dec 11, 2022 at 1:02 PM Jonathan Wakely <jwakely.gcc@gmail.com>
> wrote:
> >
> >
> >
> > On Sun, 11 Dec 2022, 09:12 Richard Biener, <richard.guenther@gmail.com>
> wrote:
> >>
> >> On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc <gcc@gcc.gnu.org>
> wrote:
> >> >
> >> > On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
> >> > <gcc@gcc.gnu.org> wrote:
> >> > >
> >> > > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc <gcc@gcc.gnu.org>
> 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.
>
> >
> >
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-11 13:38           ` Gavin Ray
@ 2022-12-11 13:44             ` Gavin Ray
  2022-12-12 11:56               ` Jonathan Wakely
  0 siblings, 1 reply; 10+ messages in thread
From: Gavin Ray @ 2022-12-11 13:44 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jonathan Wakely, Andrew Pinski, gcc

[-- Attachment #1: Type: text/plain, Size: 4337 bytes --]

Whoops, the last line should be pages_span(storage, ...)

On Sun, Dec 11, 2022 at 8:38 AM Gavin Ray <ray.gavin97@gmail.com> 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<page, NUM_PAGES> pages_span(pages, NUM_PAGES);
>
>
> On Sun, Dec 11, 2022 at 8:31 AM Richard Biener <richard.guenther@gmail.com>
> wrote:
>
>> On Sun, Dec 11, 2022 at 1:02 PM Jonathan Wakely <jwakely.gcc@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > On Sun, 11 Dec 2022, 09:12 Richard Biener, <richard.guenther@gmail.com>
>> wrote:
>> >>
>> >> On Sat, Dec 10, 2022 at 7:45 PM Andrew Pinski via Gcc <gcc@gcc.gnu.org>
>> wrote:
>> >> >
>> >> > On Sat, Dec 10, 2022 at 10:36 AM Jonathan Wakely via Gcc
>> >> > <gcc@gcc.gnu.org> wrote:
>> >> > >
>> >> > > On Sat, 10 Dec 2022 at 17:42, Gavin Ray via Gcc <gcc@gcc.gnu.org>
>> 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.
>>
>> >
>> >
>>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Bug with GCC's handling of lifetimes of implicit-lifetime types
  2022-12-11 13:44             ` Gavin Ray
@ 2022-12-12 11:56               ` Jonathan Wakely
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Wakely @ 2022-12-12 11:56 UTC (permalink / raw)
  To: Gavin Ray; +Cc: Richard Biener, Andrew Pinski, gcc

On Sun, 11 Dec 2022 at 13:44, Gavin Ray wrote:
>
> Whoops, the last line should be pages_span(storage, ...)

That won't even compile, so there's your main difference.

That seems even more off-topic on this list than "somebody told me gcc
is broken, because of [unrelated code example with UB]".

A creates 'pages' objects in the storage, B doesn't compile, but even
if it did, it's just saying "look at this storage as though it
contained this many objects of type 'pages'" which doesn't create any
objects, and would be a lie.


>
> On Sun, Dec 11, 2022 at 8:38 AM Gavin Ray <ray.gavin97@gmail.com> 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<page, NUM_PAGES> pages_span(pages, NUM_PAGES);

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-12-12 11:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-10 17:41 Bug with GCC's handling of lifetimes of implicit-lifetime types Gavin Ray
2022-12-10 18:35 ` Jonathan Wakely
2022-12-10 18:43   ` Andrew Pinski
2022-12-10 21:48     ` Gavin Ray
2022-12-11  9:11     ` Richard Biener
2022-12-11 12:02       ` Jonathan Wakely
2022-12-11 13:31         ` Richard Biener
2022-12-11 13:38           ` Gavin Ray
2022-12-11 13:44             ` Gavin Ray
2022-12-12 11:56               ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).