From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24580 invoked by alias); 14 Nov 2019 09:44:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 24567 invoked by uid 89); 14 Nov 2019 09:44:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.1 spammy=HContent-Transfer-Encoding:8bit X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 14 Nov 2019 09:44:48 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 3399BAD26; Thu, 14 Nov 2019 09:44:45 +0000 (UTC) Subject: Re: [PATCH] Add if-chain to switch conversion pass. To: Michael Matz Cc: Jakub Jelinek , gcc-patches@gcc.gnu.org, kazu@gcc.gnu.org, Jan Hubicka References: <2c3db526-cac6-4eeb-4afb-12024f8d5af2@suse.cz> <20191104144851.GJ4650@tucnak> <0a9838fd-fbde-2c48-1b11-ded7d80d92b0@suse.cz> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <30dcf27d-b03d-45cd-6c62-5afca033dd87@suse.cz> Date: Thu, 14 Nov 2019 10:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg01135.txt.bz2 On 11/13/19 4:43 PM, Michael Matz wrote: > Hi, > > On Wed, 13 Nov 2019, Martin Liška wrote: > >>> Not a review, just a few questions: >> >> Hello. >> >> Thank you for it. >> >>> >>> 1) what does it do if __builtin_expect* has been used, does it preserve >>> the probabilities and if in the end decides to expand as ifs, are those >>> probabilities retained through it? >> >> No, it's currently not supported. I can consider adding that as a follow up. > > But given that you apply the transformation even to small if ladders this > looses quite some info then. Yes. Based on my experiments, the patch does not convert if-else chain with __builtin_expect* right now. > >>> 2) for the reassoc-*.c testcases, do you get identical or better code >>> with the patch? >> >> The code is the following: >> >> Before: >> Optimizing range tests a_5(D) -[10, 10] and -[26, 26] >> into (a_5(D) & -17) != 10 >> >> [local count: 1073741823]: >> _11 = a_5(D) & -17; >> _12 = _11 == 10; >> _1 = a_5(D) == 10; >> _2 = a_5(D) == 12; >> _10 = a_5(D) == 26; >> _9 = _2 | _12; >> if (_9 != 0) >> goto ; [56.44%] >> else >> goto ; [43.56%] >> >> After: >> >> [local count: 1073741824]: >> switch (a_2(D)) [50.00%], case 10: [50.00%], case 12: >> [50.00%], case 26: [50.00%]> > > And here I bet that Jakub was asking for final code, not intermediate > code. In particular the bit trick with transforming a test for > a \in {10,26} into (a&-17)==10. The switch still tests for 10,26, and in > the end it should be ensured that the same trickery is employed to > actually implement the switch. I agree that (a&-17)==10 is more elegant trick here ... > >> As seen, reassoc can generate a different range check and then it's about cmp1 >> | cmp2 | .. >> I bet the explicit gswitch is quite equal representation. > > As long as the final code is the same or better, sure. Otherwise it's a > more canonical representation with suboptimal expansion, and the latter > should be fixed otherwise it's introducing a regression. ... and yes, the gswitch expansion is not capable of such smart interval tests. We can improve that in the future. Martin > > > Ciao, > Michael. >