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 151823858D35 for ; Wed, 13 Mar 2024 11:25:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 151823858D35 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 151823858D35 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=1710329148; cv=none; b=q94SBZhGwasnVFlQ25Qt10w/Ed3XbXNT8zIEZ7VAQ96mdCvsE2IJ6B+yPyk15lU3vLRZZwo1d9nKb4tagYJ+A0ug8Lm2p6e8UEq9YivxGhZr9rHL5DaqMT719SN7exJqtqvLXyZXPmKD/h3d/9IfZsXFVZsg5cqORoAyyaNhqMU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710329148; c=relaxed/simple; bh=IBaiRx+Qwa8e+QJxan8TB3A+BiWRtuuVt+TUqwllGa4=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=iH/z7TgrY7YZiJZfTMf8RijmixlwoBCMFBQ5lkS6lCflXYso1y9M0uxGV/1J+d9Zun5WqXdJ2Oo8pePN+xwGUfm76AH/iRTeJsQOHJ2J5BjOncjYOB0kTX3FlkmoM++I9dG+n2/+5w9jqrT5J3Ijj+EDmlt90fyDWWQtPFWEtBI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710329144; 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=VvrK25yRsTKKAfXVmpTT+VjmpGlK429hVo/+JqPxaS0=; b=F5tJRHy2+Oj6VeqBlrL83uzTcsfCQFkkwg9DIrQZDrskvnTi7vqHo78tbasmmWuij09Tt4 bjJhDJXWozooVf7mKgB4lJdJQtri1D7yh1vO0Sf2HK+LjiXlPJS6S11971kGG8H9niYQMG 5OhRsKZtZHGIFUwNH4fJQliDwHGjpTo= 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-650-1w52_488PwiTC7i0wCp65w-1; Wed, 13 Mar 2024 07:25:43 -0400 X-MC-Unique: 1w52_488PwiTC7i0wCp65w-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (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 359BE858DB3; Wed, 13 Mar 2024 11:25:43 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.45.225.36]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E7FFD492BC6; Wed, 13 Mar 2024 11:25:42 +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 42DBPf2Z753322 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 13 Mar 2024 12:25:41 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 42DBPe3J753321; Wed, 13 Mar 2024 12:25:40 +0100 Date: Wed, 13 Mar 2024 12:25:40 +0100 From: Jakub Jelinek To: Jan Hubicka Cc: Richard Biener , gcc-patches@gcc.gnu.org Subject: Re: Patch ping Re: [PATCH] icf: Reset SSA_NAME_{PTR,RANGE}_INFO in successfully merged functions [PR113907] Message-ID: Reply-To: Jakub Jelinek References: <65o73751-9998-87n9-3q22-28772o21sq19@fhfr.qr> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 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.9 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_H4,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 Wed, Mar 13, 2024 at 12:18:45PM +0100, Jan Hubicka wrote: > > On Wed, Mar 13, 2024 at 10:55:07AM +0100, Jan Hubicka wrote: > > > > > So the ipa_jump_func are I think the only thing that actually can differ > > > > > on the ICF merging candidates from value range POV. > > > > > > > > I agree. Btw, I would have approved the original patch in this > > > > thread that wipes SSA_NAME_INFO in merged bodies to mimic what LTO > > > > effectively does right now. That also looks most sensible to > > > > backport. > > > > > > > > But I'll defer to Honza in the end (but also want to point out we > > > > need something suitable for backporting). > > > > > > My main worry here is that I tried to relax matching of IL metadata in > > > the past and it triggers interesting problems. (I implemented TBAA > > > merging and one needs to match additional things in summaries and loop > > > structures) > > > > The point of the patch is that it emulates what happens with LTO (though, > > does that only for successful ICF merges), because LTO streaming ignores > > SSA_NAME_{RANGE,PTR}_INFO. > > So, because with LTO all you have in the IL is the m_vr in jump_tables, > > pure/const analysis results, whatever else might be derived on the side > > from the range info, you need to punt or union all that information anyway, > > otherwise it will misbehave with LTO. > > So punting on SSA_NAME_{RANGE,PTR}_INFO differences instead of throwing it > > away means that non-LTO will get fewer ICF merges than LTO unnecessarily, > > it doesn't improve anything for the code correctness at least for the LTO > > case. > > We have wrong code with LTO, too. I know. > The problem is that IPA passes (and > not only that, loop analysis too) does analysis at compile time (with > value numbers in) and streams the info separately. And that is desirable, because otherwise it simply couldn't derive any ranges. > Removal of value ranges > (either by LTO or by your patch) happens between computing these > summaries and using them, so this can be used to trigger wrong code, > sadly. Yes. But with LTO, I don't see how the IPA ICF comparison whether two functions are the same or not could be done with the SSA_NAME_{RANGE,PTR}_INFO in, otherwise it could only ICF merge functions from the same TUs. So the comparison IMHO (and the assert checks in my patch prove that) is done when the SSA_NAME_{RANGE,PTR}_INFO aren't in anymore. So, one just needs to compare and punt or union whatever is or could be influenced in the IPA streamed data from the ranges etc. And because one has to do it for LTO, doing it for non-LTO should be sufficient too. Jakub