From patchwork Fri May 10 09:23:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 1933768 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=auTXWE8i; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4VbNkm5Kmwz213s for ; Fri, 10 May 2024 19:24:47 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BD17F38460B2 for ; Fri, 10 May 2024 09:24:45 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-44.mimecast.com (us-smtp-delivery-44.mimecast.com [205.139.111.44]) by sourceware.org (Postfix) with ESMTPS id 8E1F4384CBA1 for ; Fri, 10 May 2024 09:24:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8E1F4384CBA1 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=localhost.redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8E1F4384CBA1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=205.139.111.44 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715333068; cv=none; b=tsM0iooOoDO8hz0en7fLNUHiMfI2Lrqv8CC4WCaZ53307HkY7te9HylXht405smo3xrej+jrMPenvdlD8l1DbjCVUgUNcrdkAZSNc5ZK9MCuXR/nuKpcLcICdga6opBHUAMgD+8o5RlgQS/W51AD1GOWmQFdXUI9hnP4tPrnOhw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715333068; c=relaxed/simple; bh=uFpHrZ85tTZ0CE+QJ8Fu9ydn1x8J1IH1Sa4SBpvJVK0=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=n+qIJjgcwMOoA/lVc1mjwwAWfTFqk//HE502lkJvf8QQiOd50Hch4YUeRfi5JST2URwFWr2bgi78mBofNn3bEKeplLYKSU80/XVGqtPf+fVlqdVBPHTNQATW3iyudZBqfSVAQZuBpUUwKCrOioH999DcHB2Ua7iaFPM+f+ZckwQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1715333066; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=kSSd0SmKwXynQ02o1qxXL8009sm/dwpUBwp41Ier5ag=; b=auTXWE8i6hd57nlnZKTURxX+JPXsk+k7EWWurQ3Cg7qdt3oFWpA+c3UuAAoqbmei6fMdUN PtXFBey3llJdwaJaIkssP69esPLSk2B+Q8HIZvpKBHQnbuZiKY9/CIh82c3/PXmGlUxobS hf8w7K0QxLLY8TzpbmOS8TykIaqza/k= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-556-j0wdLivTOFujoNqNJqIP9A-1; Fri, 10 May 2024 05:24:24 -0400 X-MC-Unique: j0wdLivTOFujoNqNJqIP9A-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (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 1B9A829AB3FF; Fri, 10 May 2024 09:24:24 +0000 (UTC) Received: from abulafia.quesejoda.com (unknown [10.39.193.10]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BC0E849B184; Fri, 10 May 2024 09:24:23 +0000 (UTC) Received: from abulafia.quesejoda.com (localhost [127.0.0.1]) by abulafia.quesejoda.com (8.18.1/8.17.1) with ESMTPS id 44A9OMCt432705 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 10 May 2024 11:24:22 +0200 Received: (from aldyh@localhost) by abulafia.quesejoda.com (8.18.1/8.18.1/Submit) id 44A9OLBX432704; Fri, 10 May 2024 11:24:21 +0200 From: Aldy Hernandez To: GCC patches Cc: Martin Jambor , Andrew MacLeod , Aldy Hernandez Subject: [PATCH] Adjust range type of calls into fold_range for IPA passes [PR114985] Date: Fri, 10 May 2024 11:23:48 +0200 Message-ID: <20240510092417.432674-1-aldyh@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NO_DNS_FOR_FROM, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org There are various calls into fold_range() that have the wrong type associated with the range temporary used to hold the result. This used to work, because we could store either integers or pointers in a Value_Range, but is no longer the case with prange's. Now you must explicitly state which type of range the temporary will hold before storing into it. You can change this at a later time with set_type(), but you must always have a type before using the temporary, and it must match what fold_range() returns. This patch adjusts the IPA code to restore the previous functionality, so I can re-enable the prange code, but I do question whether the previous code was correct. I have added appropriate comments to help the maintainers, but someone with more knowledge should revamp this going forward. The basic problem is that pointer comparisons return a boolean, but the IPA code is initializing the resulting range as a pointer. This wasn't a problem, because fold_range() would previously happily force the range into an integer one, and everything would work. But now we must initialize the range to an integer before calling into fold_range. The thing is, that the failing case sets the result back into a pointer, which is just weird but existing behavior. I have documented this in the code. if (!handler || !op_res.supports_type_p (vr_type) || !handler.fold_range (op_res, vr_type, srcvr, op_vr)) /* For comparison operators, the type here may be different than the range type used in fold_range above. For example, vr_type may be a pointer, whereas the type returned by fold_range will always be a boolean. This shouldn't cause any problems, as the set_varying below will happily change the type of the range in op_res, and then the cast operation in ipa_vr_operation_and_type_effects will ultimately leave things in the desired type, but it is confusing. Perhaps the original intent was to use the type of op_res here? */ op_res.set_varying (vr_type); BTW, this is not to say that the original gimple IR was wrong, but that IPA is setting the range type of the result of fold_range() to the type of the operands, which does not necessarily match in the case of a comparison. I am just restoring previous behavior here, but I do question whether it was right to begin with. Testing currently in progress on x86-64 and ppc64le with prange enabled. OK pending tests? gcc/ChangeLog: PR tree-optimization/114985 * ipa-cp.cc (ipa_value_range_from_jfunc): Adjust type of op_res. (propagate_vr_across_jump_function): Same. * ipa-fnsummary.cc (evaluate_conditions_for_known_args): Adjust type for res. * ipa-prop.h (ipa_type_for_fold_range): New. --- gcc/ipa-cp.cc | 18 ++++++++++++++++-- gcc/ipa-fnsummary.cc | 6 +++++- gcc/ipa-prop.h | 13 +++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc index 5781f50c854..3c395632364 100644 --- a/gcc/ipa-cp.cc +++ b/gcc/ipa-cp.cc @@ -1730,7 +1730,7 @@ ipa_value_range_from_jfunc (vrange &vr, } else { - Value_Range op_res (vr_type); + Value_Range op_res (ipa_type_for_fold_range (operation, vr_type)); Value_Range res (vr_type); tree op = ipa_get_jf_pass_through_operand (jfunc); Value_Range op_vr (TREE_TYPE (op)); @@ -1741,6 +1741,19 @@ ipa_value_range_from_jfunc (vrange &vr, if (!handler || !op_res.supports_type_p (vr_type) || !handler.fold_range (op_res, vr_type, srcvr, op_vr)) + /* For comparison operators, the type here may be + different than the range type used in fold_range above. + For example, vr_type may be a pointer, whereas the type + returned by fold_range will always be a boolean. + + This shouldn't cause any problems, as the set_varying + below will happily change the type of the range in + op_res, and then the cast operation in + ipa_vr_operation_and_type_effects will ultimately leave + things in the desired type, but it is confusing. + + Perhaps the original intent was to use the type of + op_res here? */ op_res.set_varying (vr_type); if (ipa_vr_operation_and_type_effects (res, @@ -2540,7 +2553,7 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, { tree op = ipa_get_jf_pass_through_operand (jfunc); Value_Range op_vr (TREE_TYPE (op)); - Value_Range op_res (param_type); + Value_Range op_res (ipa_type_for_fold_range (operation, param_type)); range_op_handler handler (operation); ipa_range_set_and_normalize (op_vr, op); @@ -2549,6 +2562,7 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc, || !ipa_supports_p (operand_type) || !handler.fold_range (op_res, operand_type, src_lats->m_value_range.m_vr, op_vr)) + /* See note in ipa_value_range_from_jfunc. */ op_res.set_varying (param_type); ipa_vr_operation_and_type_effects (vr, diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc index 07a853f78e3..4deba2394f5 100644 --- a/gcc/ipa-fnsummary.cc +++ b/gcc/ipa-fnsummary.cc @@ -502,7 +502,8 @@ evaluate_conditions_for_known_args (struct cgraph_node *node, if (vr.varying_p () || vr.undefined_p ()) break; - Value_Range res (op->type); + Value_Range res (ipa_type_for_fold_range (op->code, + op->type)); if (!op->val[0]) { Value_Range varying (op->type); @@ -511,6 +512,7 @@ evaluate_conditions_for_known_args (struct cgraph_node *node, if (!handler || !res.supports_type_p (op->type) || !handler.fold_range (res, op->type, vr, varying)) + /* See note in ipa_value_from_jfunc. */ res.set_varying (op->type); } else if (!op->val[1]) @@ -525,9 +527,11 @@ evaluate_conditions_for_known_args (struct cgraph_node *node, || !handler.fold_range (res, op->type, op->index ? op0 : vr, op->index ? vr : op0)) + /* See note in ipa_value_from_jfunc. */ res.set_varying (op->type); } else + /* See note in ipa_value_from_jfunc. */ res.set_varying (op->type); vr = res; } diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 93d1b87b1f7..8493dd19b92 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -1277,6 +1277,19 @@ ipa_range_set_and_normalize (vrange &r, tree val) r.set (val, val); } +/* Return the resulting type for a fold_range() operation for OP and + TYPE. */ + +inline tree +ipa_type_for_fold_range (tree_code op, tree type) +{ + /* Comparisons return boolean regardless of their input operands. */ + if (TREE_CODE_CLASS (op) == tcc_comparison) + return boolean_type_node; + + return type; +} + bool ipa_return_value_range (Value_Range &range, tree decl); void ipa_record_return_value_range (Value_Range val); bool ipa_jump_functions_equivalent_p (ipa_jump_func *jf1, ipa_jump_func *jf2);