From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A52D2385842F for ; Tue, 19 Dec 2023 22:57:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A52D2385842F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A52D2385842F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703026667; cv=none; b=bkd5Ta0htXeMHXBrlrdhyLRhAh4Vs1bZIL9YVAbskOcTjOnwexRCvLJ6RUXQjh4/ctoFNDWJAWsAav1tYMyQzfMkHg+tsqIy/DYFLtRdPCE07G18SERX0ZtElDXBAeZO0d4sT7dnyG/Sg1OSnJTUoGQeXci2c49kjZho5qZebpk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703026667; c=relaxed/simple; bh=Sq1itkahE/A4L/bLdfCLlS43uQH876SQQU72PCK/QJY=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=MweZaqz9XsiMYoGZMtvGTNO8taqDwFi74aepn3AwNprOusIOcCPhLCIGNGPouKi6YgY7m0gG6o6Xy0Icc4QPHQhdzX7rL5A3gR6CTi8U+4AmxGjVEfJ5eVVyn6paKF+2sgiAc0Oe631izuWa0oUGq0C30dVn5U16NdUZD8uhapY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1703026666; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=ZyG2hUSjqrq/hUDoXWba0kt5TfmTAzkgljTlK3j2/c0=; b=Ojn1MStpItSzqQRN9mNkKbePp/almfFlA1tVagyaml1dofVyz004OC3iQvDVFeQJvN7jI3 Ka+Fvd60OOmk3A+KwI9TtAYGod1vg3+hPD8ZmXPTejIE5SyW51xBwtdgAtiRgHHNXc0fn+ /S4oSk3YfChaMWt8hPqlTPuC+Dow1q4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-280-0_t_lbRqMnyu_wFNypdvAQ-1; Tue, 19 Dec 2023 17:57:44 -0500 X-MC-Unique: 0_t_lbRqMnyu_wFNypdvAQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4FCDA185A781; Tue, 19 Dec 2023 22:57:44 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.92]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 11CB71121306; Tue, 19 Dec 2023 22:57:43 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 3BJMvaPw156464 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 19 Dec 2023 23:57:37 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 3BJMvZQX156458; Tue, 19 Dec 2023 23:57:35 +0100 Date: Tue, 19 Dec 2023 23:57:35 +0100 From: Jakub Jelinek To: Siddhesh Poyarekar Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] tree-object-size: Always set computed bit for bdos [PR113012] Message-ID: Reply-To: Jakub Jelinek References: <20231218164252.1963249-1-siddhesh@gotplt.org> MIME-Version: 1.0 In-Reply-To: <20231218164252.1963249-1-siddhesh@gotplt.org> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Dec 18, 2023 at 11:42:52AM -0500, Siddhesh Poyarekar wrote: > It is always safe to set the computed bit for dynamic object sizes at > the end of collect_object_sizes_for because even in case of a dependency > loop encountered in nested calls, we have an SSA temporary to actually > finish the object size expression. The reexamine pass for dynamic > object sizes is only for propagation of unknowns and gimplification of > the size expressions, not for loop resolution as in the case of static > object sizes. > > gcc/ChangeLog: > > PR tree-optimization/113012 > * gcc.dg/ubsan/pr113012.c: New test case. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/113012 > * tree-object-size.cc (compute_builtin_object_size): Expand > comment for dynamic object sizes. > (collect_object_sizes_for): Always set COMPUTED bitmap for > dynamic object sizes. You have the gcc/ChangeLog and gcc/testsuite/ChangeLog hunks swapped, I think this wouldn't get through pre-commit hook. > --- a/gcc/tree-object-size.cc > +++ b/gcc/tree-object-size.cc > @@ -1185,10 +1185,12 @@ compute_builtin_object_size (tree ptr, int object_size_type, > osi.tos = NULL; > } > > - /* First pass: walk UD chains, compute object sizes that > - can be computed. osi.reexamine bitmap at the end will > - contain what variables were found in dependency cycles > - and therefore need to be reexamined. */ > + /* First pass: walk UD chains, compute object sizes that can be computed. > + osi.reexamine bitmap at the end will contain what variables that need This wording is weird. Perhaps ... will contain versions of SSA_NAMEs that need to be reexamined. ? Because varno seems to be SSA_NAME_VERSION. > @@ -1823,11 +1825,16 @@ collect_object_sizes_for (struct object_size_info *osi, tree var) > gcc_unreachable (); > } > > - if (! reexamine || object_sizes_unknown_p (object_size_type, varno)) > + /* Dynamic sizes use placeholder temps to return an answer, so it is always > + * safe to set COMPUTED for them. */ We don't use this style of comments, please replace the * at the start of second line with a space. > + if ((object_size_type & OST_DYNAMIC) > + || !reexamine || object_sizes_unknown_p (object_size_type, varno)) > { > bitmap_set_bit (computed[object_size_type], varno); > if (!(object_size_type & OST_DYNAMIC)) > bitmap_clear_bit (osi->reexamine, varno); > + else if (reexamine) > + bitmap_set_bit (osi->reexamine, varno); > } > else > { Otherwise LGTM, but please wait at least a few weeks before backporting to 13. Jakub