* [C++ Patch] Remove quick fix for c++/85553
@ 2018-10-17 19:38 Paolo Carlini
2018-10-17 19:39 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Carlini @ 2018-10-17 19:38 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 408 bytes --]
Hi,
as you probably remember, very close to the release of 8.1.0 we noticed
that my fix for c++/70808 was causing c++/85553, which Jakub promptly
fixed. However, we later found out that the real problem was a latent
issue in convert, which I fixed in r259966. Thus, I think that in
current trunk we can revert Jakub's quick fix, now redundant. Tested
x86_64-linux.
Thanks! Paolo.
//////////////////
[-- Attachment #2: CL --]
[-- Type: text/plain, Size: 190 bytes --]
2018-10-17 Paolo Carlini <paolo.carlini@oracle.com>
* init.c (build_zero_init_1): Remove special casing for
NULLPTR_TYPE_P (type), introduced by r259728 and made
redundant by r259966.
[-- Attachment #3: patchlet --]
[-- Type: text/plain, Size: 702 bytes --]
Index: init.c
===================================================================
--- init.c (revision 265241)
+++ init.c (working copy)
@@ -180,10 +180,8 @@ build_zero_init_1 (tree type, tree nelts, bool sta
items with static storage duration that are not otherwise
initialized are initialized to zero. */
;
- else if (TYPE_PTR_OR_PTRMEM_P (type))
+ else if (TYPE_PTR_OR_PTRMEM_P (type) || NULLPTR_TYPE_P (type))
init = fold (convert (type, nullptr_node));
- else if (NULLPTR_TYPE_P (type))
- init = build_int_cst (type, 0);
else if (SCALAR_TYPE_P (type))
init = fold (convert (type, integer_zero_node));
else if (RECORD_OR_UNION_CODE_P (TREE_CODE (type)))
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [C++ Patch] Remove quick fix for c++/85553
2018-10-17 19:38 [C++ Patch] Remove quick fix for c++/85553 Paolo Carlini
@ 2018-10-17 19:39 ` Jakub Jelinek
2018-10-17 21:26 ` Paolo Carlini
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2018-10-17 19:39 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches, Jason Merrill
On Wed, Oct 17, 2018 at 07:20:53PM +0200, Paolo Carlini wrote:
> Hi,
>
> as you probably remember, very close to the release of 8.1.0 we noticed that
> my fix for c++/70808 was causing c++/85553, which Jakub promptly fixed.
> However, we later found out that the real problem was a latent issue in
> convert, which I fixed in r259966. Thus, I think that in current trunk we
> can revert Jakub's quick fix, now redundant. Tested x86_64-linux.
Is there some desirable diagnostics you expect from the convert?
If not, build_int_cst is certainly cheaper.
> 2018-10-17 Paolo Carlini <paolo.carlini@oracle.com>
>
> * init.c (build_zero_init_1): Remove special casing for
> NULLPTR_TYPE_P (type), introduced by r259728 and made
> redundant by r259966.
> Index: init.c
> ===================================================================
> --- init.c (revision 265241)
> +++ init.c (working copy)
> @@ -180,10 +180,8 @@ build_zero_init_1 (tree type, tree nelts, bool sta
> items with static storage duration that are not otherwise
> initialized are initialized to zero. */
> ;
> - else if (TYPE_PTR_OR_PTRMEM_P (type))
> + else if (TYPE_PTR_OR_PTRMEM_P (type) || NULLPTR_TYPE_P (type))
> init = fold (convert (type, nullptr_node));
> - else if (NULLPTR_TYPE_P (type))
> - init = build_int_cst (type, 0);
> else if (SCALAR_TYPE_P (type))
> init = fold (convert (type, integer_zero_node));
> else if (RECORD_OR_UNION_CODE_P (TREE_CODE (type)))
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [C++ Patch] Remove quick fix for c++/85553
2018-10-17 19:39 ` Jakub Jelinek
@ 2018-10-17 21:26 ` Paolo Carlini
2018-10-24 20:02 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Carlini @ 2018-10-17 21:26 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]
Hi Jakub,
On 17/10/18 19:42, Jakub Jelinek wrote:
> On Wed, Oct 17, 2018 at 07:20:53PM +0200, Paolo Carlini wrote:
>> Hi,
>>
>> as you probably remember, very close to the release of 8.1.0 we noticed that
>> my fix for c++/70808 was causing c++/85553, which Jakub promptly fixed.
>> However, we later found out that the real problem was a latent issue in
>> convert, which I fixed in r259966. Thus, I think that in current trunk we
>> can revert Jakub's quick fix, now redundant. Tested x86_64-linux.
> Is there some desirable diagnostics you expect from the convert?
> If not, build_int_cst is certainly cheaper.
No, no diagnostics. But I'm still a bit nervous about the various way we
handle those zero-initializations for pointer-ish types. Which is, if
you wish, what fooled me in the first place when I simply assumed that
convert was able to cope with the easy nullptr_node case. Say we do
something like the below, can you see something wrong with it? Only
lightly tested so far but appears to work fine at least on x86_64...
Thanks, Paolo.
///////////////////
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 764 bytes --]
Index: init.c
===================================================================
--- init.c (revision 265241)
+++ init.c (working copy)
@@ -180,10 +180,10 @@ build_zero_init_1 (tree type, tree nelts, bool sta
items with static storage duration that are not otherwise
initialized are initialized to zero. */
;
- else if (TYPE_PTR_OR_PTRMEM_P (type))
+ else if (TYPE_PTR_P (type) || NULLPTR_TYPE_P (type))
+ init = build_int_cst (type, 0);
+ else if (TYPE_PTRMEM_P (type))
init = fold (convert (type, nullptr_node));
- else if (NULLPTR_TYPE_P (type))
- init = build_int_cst (type, 0);
else if (SCALAR_TYPE_P (type))
init = fold (convert (type, integer_zero_node));
else if (RECORD_OR_UNION_CODE_P (TREE_CODE (type)))
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [C++ Patch] Remove quick fix for c++/85553
2018-10-17 21:26 ` Paolo Carlini
@ 2018-10-24 20:02 ` Jason Merrill
0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2018-10-24 20:02 UTC (permalink / raw)
To: Paolo Carlini, Jakub Jelinek; +Cc: gcc-patches
On 10/17/18 3:38 PM, Paolo Carlini wrote:
> Hi Jakub,
>
> On 17/10/18 19:42, Jakub Jelinek wrote:
>> On Wed, Oct 17, 2018 at 07:20:53PM +0200, Paolo Carlini wrote:
>>> Hi,
>>>
>>> as you probably remember, very close to the release of 8.1.0 we
>>> noticed that
>>> my fix for c++/70808 was causing c++/85553, which Jakub promptly fixed.
>>> However, we later found out that the real problem was a latent issue in
>>> convert, which I fixed in r259966. Thus, I think that in current
>>> trunk we
>>> can revert Jakub's quick fix, now redundant. Tested x86_64-linux.
>> Is there some desirable diagnostics you expect from the convert?
>> If not, build_int_cst is certainly cheaper.
>
> No, no diagnostics. But I'm still a bit nervous about the various way we
> handle those zero-initializations for pointer-ish types. Which is, if
> you wish, what fooled me in the first place when I simply assumed that
> convert was able to cope with the easy nullptr_node case. Say we do
> something like the below, can you see something wrong with it? Only
> lightly tested so far but appears to work fine at least on x86_64...
This is fine.
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-24 19:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17 19:38 [C++ Patch] Remove quick fix for c++/85553 Paolo Carlini
2018-10-17 19:39 ` Jakub Jelinek
2018-10-17 21:26 ` Paolo Carlini
2018-10-24 20:02 ` Jason Merrill
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).