Message ID | 2FC986DA-9B67-476D-9593-49F1B2BFA2A9@oracle.com |
---|---|
State | New |
Headers | show |
Series | Where in C++ module streaming to handle a new bitfield added in "tree_decl_common" | expand |
On 8/2/22 10:44, Qing Zhao wrote: > Hi, Nathan, > > I am adding a new bitfield “decl_not_flexarray” in “tree_decl_common” (gcc/tree-core.h) for the new gcc feature -fstrict-flex-arrays. > > ==== > diff --git a/gcc/tree-core.h b/gcc/tree-core.h > index ea9f281f1cc..458c6e6ceea 100644 > --- a/gcc/tree-core.h > +++ b/gcc/tree-core.h > @@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common { > TYPE_WARN_IF_NOT_ALIGN. */ > unsigned int warn_if_not_align : 6; > > - /* 14 bits unused. */ > + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ > + unsigned int decl_not_flexarray : 1; Is it possible to invert the meaning here -- set the flag if it /IS/ a flexible array? negated flags can be confusing, and I see your patch sets it to '!is_flexible_array (...)' anyway? > + > + /* 13 bits unused. */ > > /* UID for points-to sets, stable over copying from inlining. */ > unsigned int pt_uid; > ==== > > (Please refer to the following for details: > > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598556.html > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598965.html > ) > > Richard mentioned the following: > > "I've not seen it so you are probably missing it - the bit has to be > streamed in tree-streamer-{in,out}.cc to be usable from LTO. Possibly > C++ module streaming also needs to handle it.” > > I have figured out that where to add the handling of the bit in “tree-streamer-{in, out}.cc, > However, it’s quite difficult for me to locate where should I add the handling of this new bit in > C++ module streaming, could you please help me on this? > add it in to trees_{in,out}::core_bools. You could elide streaming for non-FIELD_DECL decls. Hope that helps. nathan > Thanks a lot for your help. > > Qing
On Mon, Aug 15, 2022 at 3:29 PM Nathan Sidwell via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On 8/2/22 10:44, Qing Zhao wrote: > > Hi, Nathan, > > > > I am adding a new bitfield “decl_not_flexarray” in “tree_decl_common” (gcc/tree-core.h) for the new gcc feature -fstrict-flex-arrays. > > > > ==== > > diff --git a/gcc/tree-core.h b/gcc/tree-core.h > > index ea9f281f1cc..458c6e6ceea 100644 > > --- a/gcc/tree-core.h > > +++ b/gcc/tree-core.h > > @@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common { > > TYPE_WARN_IF_NOT_ALIGN. */ > > unsigned int warn_if_not_align : 6; > > > > - /* 14 bits unused. */ > > + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ > > + unsigned int decl_not_flexarray : 1; > > Is it possible to invert the meaning here -- set the flag if it /IS/ a > flexible array? negated flags can be confusing, and I see your patch > sets it to '!is_flexible_array (...)' anyway? The issue is it's consumed by the middle-end but set by a single (or two) frontends and the conservative setting is having the bit not set. That works nicely together with touching just the frontends that want stricter behavior than currently ... > > + > > + /* 13 bits unused. */ > > > > /* UID for points-to sets, stable over copying from inlining. */ > > unsigned int pt_uid; > > ==== > > > > (Please refer to the following for details: > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598556.html > > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598965.html > > > > > ) > > > > Richard mentioned the following: > > > > "I've not seen it so you are probably missing it - the bit has to be > > streamed in tree-streamer-{in,out}.cc to be usable from LTO. Possibly > > C++ module streaming also needs to handle it.” > > > > I have figured out that where to add the handling of the bit in “tree-streamer-{in, out}.cc, > > However, it’s quite difficult for me to locate where should I add the handling of this new bit in > > C++ module streaming, could you please help me on this? > > > > > add it in to trees_{in,out}::core_bools. You could elide streaming for > non-FIELD_DECL decls. > > Hope that helps. > > nathan > > > > > Thanks a lot for your help. > > > > Qing > > > -- > Nathan Sidwell
On 8/15/22 10:03, Richard Biener wrote: > On Mon, Aug 15, 2022 at 3:29 PM Nathan Sidwell via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> On 8/2/22 10:44, Qing Zhao wrote: >>> Hi, Nathan, >>> >>> I am adding a new bitfield “decl_not_flexarray” in “tree_decl_common” (gcc/tree-core.h) for the new gcc feature -fstrict-flex-arrays. >>> >>> ==== >>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h >>> index ea9f281f1cc..458c6e6ceea 100644 >>> --- a/gcc/tree-core.h >>> +++ b/gcc/tree-core.h >>> @@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common { >>> TYPE_WARN_IF_NOT_ALIGN. */ >>> unsigned int warn_if_not_align : 6; >>> >>> - /* 14 bits unused. */ >>> + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ >>> + unsigned int decl_not_flexarray : 1; >> >> Is it possible to invert the meaning here -- set the flag if it /IS/ a >> flexible array? negated flags can be confusing, and I see your patch >> sets it to '!is_flexible_array (...)' anyway? > > The issue is it's consumed by the middle-end but set by a single (or two) > frontends and the conservative setting is having the bit not set. That works > nicely together with touching just the frontends that want stricter behavior > than currently ... Makes sense, but is the comment incomplete? I'm guessing this flag is for FIELD_DECLs /of array type/, and not just any old FIELD_DECL? After all a field of type int is not a flexible array, but presumably doesn't need this flag setting? nathan
On Tue, Aug 16, 2022 at 2:16 PM Nathan Sidwell <nathan@acm.org> wrote: > > On 8/15/22 10:03, Richard Biener wrote: > > On Mon, Aug 15, 2022 at 3:29 PM Nathan Sidwell via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > >> > >> On 8/2/22 10:44, Qing Zhao wrote: > >>> Hi, Nathan, > >>> > >>> I am adding a new bitfield “decl_not_flexarray” in “tree_decl_common” (gcc/tree-core.h) for the new gcc feature -fstrict-flex-arrays. > >>> > >>> ==== > >>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h > >>> index ea9f281f1cc..458c6e6ceea 100644 > >>> --- a/gcc/tree-core.h > >>> +++ b/gcc/tree-core.h > >>> @@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common { > >>> TYPE_WARN_IF_NOT_ALIGN. */ > >>> unsigned int warn_if_not_align : 6; > >>> > >>> - /* 14 bits unused. */ > >>> + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ > >>> + unsigned int decl_not_flexarray : 1; > >> > >> Is it possible to invert the meaning here -- set the flag if it /IS/ a > >> flexible array? negated flags can be confusing, and I see your patch > >> sets it to '!is_flexible_array (...)' anyway? > > > > The issue is it's consumed by the middle-end but set by a single (or two) > > frontends and the conservative setting is having the bit not set. That works > > nicely together with touching just the frontends that want stricter behavior > > than currently ... > > Makes sense, but is the comment incomplete? I'm guessing this flag is > for FIELD_DECLs /of array type/, and not just any old FIELD_DECL? After > all a field of type int is not a flexible array, but presumably doesn't > need this flag setting? Yes, the docs should be more complete in tree.h on the actual DECL_NOT_FLEXARRAY definition. Richard. > nathan > > -- > Nathan Sidwell
> On Aug 15, 2022, at 9:28 AM, Nathan Sidwell <nathan@acm.org> wrote: > > On 8/2/22 10:44, Qing Zhao wrote: >> Hi, Nathan, >> I am adding a new bitfield “decl_not_flexarray” in “tree_decl_common” (gcc/tree-core.h) for the new gcc feature -fstrict-flex-arrays. >> ==== >> diff --git a/gcc/tree-core.h b/gcc/tree-core.h >> index ea9f281f1cc..458c6e6ceea 100644 >> --- a/gcc/tree-core.h >> +++ b/gcc/tree-core.h >> @@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common { >> TYPE_WARN_IF_NOT_ALIGN. */ >> unsigned int warn_if_not_align : 6; >> - /* 14 bits unused. */ >> + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ >> + unsigned int decl_not_flexarray : 1; > > Is it possible to invert the meaning here -- set the flag if it /IS/ a flexible array? negated flags can be confusing, and I see your patch sets it to '!is_flexible_array (...)' anyway? > >> + >> + /* 13 bits unused. */ >> /* UID for points-to sets, stable over copying from inlining. */ >> unsigned int pt_uid; >> ==== >> (Please refer to the following for details: >> https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598556.html >> https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598965.html > > > >> ) >> Richard mentioned the following: >> "I've not seen it so you are probably missing it - the bit has to be >> streamed in tree-streamer-{in,out}.cc to be usable from LTO. Possibly >> C++ module streaming also needs to handle it.” >> I have figured out that where to add the handling of the bit in “tree-streamer-{in, out}.cc, >> However, it’s quite difficult for me to locate where should I add the handling of this new bit in >> C++ module streaming, could you please help me on this? > > > add it in to trees_{in,out}::core_bools. You could elide streaming for non-FIELD_DECL decls. Got it. Thanks a lot. Qing > > Hope that helps. > > nathan > > > >> Thanks a lot for your help. >> Qing > > > -- > Nathan Sidwell
> On Aug 16, 2022, at 8:37 AM, Richard Biener <richard.guenther@gmail.com> wrote: > > On Tue, Aug 16, 2022 at 2:16 PM Nathan Sidwell <nathan@acm.org> wrote: >> >> On 8/15/22 10:03, Richard Biener wrote: >>> On Mon, Aug 15, 2022 at 3:29 PM Nathan Sidwell via Gcc-patches >>> <gcc-patches@gcc.gnu.org> wrote: >>>> >>>> On 8/2/22 10:44, Qing Zhao wrote: >>>>> Hi, Nathan, >>>>> >>>>> I am adding a new bitfield “decl_not_flexarray” in “tree_decl_common” (gcc/tree-core.h) for the new gcc feature -fstrict-flex-arrays. >>>>> >>>>> ==== >>>>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h >>>>> index ea9f281f1cc..458c6e6ceea 100644 >>>>> --- a/gcc/tree-core.h >>>>> +++ b/gcc/tree-core.h >>>>> @@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common { >>>>> TYPE_WARN_IF_NOT_ALIGN. */ >>>>> unsigned int warn_if_not_align : 6; >>>>> >>>>> - /* 14 bits unused. */ >>>>> + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ >>>>> + unsigned int decl_not_flexarray : 1; >>>> >>>> Is it possible to invert the meaning here -- set the flag if it /IS/ a >>>> flexible array? negated flags can be confusing, and I see your patch >>>> sets it to '!is_flexible_array (...)' anyway? >>> >>> The issue is it's consumed by the middle-end but set by a single (or two) >>> frontends and the conservative setting is having the bit not set. That works >>> nicely together with touching just the frontends that want stricter behavior >>> than currently ... >> >> Makes sense, but is the comment incomplete? I'm guessing this flag is >> for FIELD_DECLs /of array type/, and not just any old FIELD_DECL? After >> all a field of type int is not a flexible array, but presumably doesn't >> need this flag setting? > > Yes, the docs should be more complete in tree.h on the actual DECL_NOT_FLEXARRAY > definition. Okay, will add more comments in tree.h to make the DECL_NOT_FLEXARRAY more complete. thanks. Qing > > Richard. > >> nathan >> >> -- >> Nathan Sidwell
diff --git a/gcc/tree-core.h b/gcc/tree-core.h index ea9f281f1cc..458c6e6ceea 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common { TYPE_WARN_IF_NOT_ALIGN. */ unsigned int warn_if_not_align : 6; - /* 14 bits unused. */ + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ + unsigned int decl_not_flexarray : 1; + + /* 13 bits unused. */ /* UID for points-to sets, stable over copying from inlining. */ unsigned int pt_uid;