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 7B0073857820 for ; Tue, 5 Apr 2022 09:15:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7B0073857820 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-244-ArVdkIejNQ2B2-DBxMQ7YA-1; Tue, 05 Apr 2022 05:15:43 -0400 X-MC-Unique: ArVdkIejNQ2B2-DBxMQ7YA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id F1E621C01E81; Tue, 5 Apr 2022 09:15:42 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.195.172]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B5019145F964; Tue, 5 Apr 2022 09:15:42 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 2359Fe7r3914782 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 5 Apr 2022 11:15:40 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 2359FdGM3914781; Tue, 5 Apr 2022 11:15:39 +0200 Date: Tue, 5 Apr 2022 11:15:39 +0200 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] match.pd: Punt on optimizing sqrt with incorrect arg type [PR105150] Message-ID: Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 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=-4.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Apr 2022 09:15:45 -0000 On Tue, Apr 05, 2022 at 10:50:01AM +0200, Richard Biener wrote: > > In the following testcase sqrt is declared as unprototyped function > > and so it depends on what type has the argument passed to it. > > If an argument of incorrect type is passed, the sqrt comparison > > simplification can create invalid GIMPLE. > > > > The patch fixes it by punting if there is a mismatch of types. > > As I didn't want to reindent 133 lines, the first hunk contains an > > ugly hack with if (false). If you prefer reindentation, I can do that > > too. > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > So why does this get pas gimple_call_combined_fn ()? Are those > internal functions at the point we match them? Otherwise we > invoke gimple_builtin_call_types_compatible_p on them. If > they are internal functions why did we rewrite the calls to them > when the types do not match? If gimple_builtin_call_types_compatible_p > is fine then why did the frontend assign BUILT_IN_SQRT to the > function decls? For the unprototyped functions like double sqrt (); they can be called in a valid way or they might not, so I think having DECL_BUILT_IN_CLASS other than BUILT_IN_NORMAL is right. The problem is that we don't do proper checking. In generic-match.cc, we just call get_call_combined_fn which performs no checking of the arguments whatsoever. In gimple-match.cc, we call gimple_call_combined_fn, which performs bad checking of the arguments. When not an internal function (which doesn't need argument checking, the compiler guarantees it is right), the former does just: tree fndecl = get_callee_fndecl (call); if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); In GIMPLE, we call: && gimple_builtin_call_types_compatible_p (stmt, fndecl) but that is insufficient, that verifies whether the arguments passed to GIMPLE_CALL match the fndecl argument types. But that fndecl may very well be the user declaration, so doesn't have to match exactly the builtin as predeclared by builtins.def etc. (though, there is the cotcha that say for FILE * we just use void * etc.). So e.g. in tree-ssa-strlen.cc we use additional: tree callee = gimple_call_fndecl (stmt); tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); if (decl && decl != callee && !gimple_builtin_call_types_compatible_p (stmt, decl)) return false; For this particular testcase, seems the problem is just the generic folding, if I add && GIMPLE to the sqrt comparison foldings, it doesn't ICE anymore, so just adding generic_builtin_call_types_compatible_p (basically copy gimple_builtin_call_types_compatible_p) might be enough. Though a big question is what it should do, using useless_type_conversion_p might be needed so that it can deal e.g. with the FILE * vs. void * differences (and various others), on the other side, what e.g. the exact sqrt comparison folding requires to be much more strict, as it turns (cmp (sq @0) REAL_CST@1) to (cmp @0 @1) and similarly for (cmp (sq @0) (sq @1)) and that requires exact type match, no? So one possibility would be to use useless_type_conversion_p but actually not write (cmp @0 @1) in such cases, but (cmp (convert:op1type @0) @1) or so? Jakub