From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x129.google.com (mail-lf1-x129.google.com [IPv6:2a00:1450:4864:20::129]) by sourceware.org (Postfix) with ESMTPS id 5F1603858D1E for ; Fri, 20 Oct 2023 07:04:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5F1603858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5F1603858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::129 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697785500; cv=none; b=CIfX13M3ibjyktUGjDIuUpuKawn7c0zGUUg2IS5lOiQLvwqpCf2AcX7ms7xAXIZGHyx5/ra+qP+nsbZNias2R3aemgIFowhovvuZICOeFwBjVVl2PnuM/eKflZvdw+SMA5rA6jFrif7V5JgIUFgBpBXW8NbU1FHpcbW3jm+V4ns= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1697785500; c=relaxed/simple; bh=o8zt4vm57aqn0wMWw75PNRp1nyoyhoEgxdaZdABM6os=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=mJ9At8i9cQLzRflZCiL3zddue3OP37WdzB1ZyrtRuu4QXm15p7+0ZCT46Gk+ycalRVnCCuiym2axA9sFJKX1xzviqaL44n71GhCVZL/WB7Ml0HEduyEpluTK6HmmElTKkEd52q+Z58RSl/FYthhu7nkG/NbMLJF+x61u+fH0r58= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x129.google.com with SMTP id 2adb3069b0e04-507bd64814fso636410e87.1 for ; Fri, 20 Oct 2023 00:04:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1697785496; x=1698390296; darn=gcc.gnu.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:cc:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=/ZqypdHtXbUCfZ8Ja9jjxRFQyEjnWkCGI+nL0QIyr7c=; b=aE0v6LpIkQLDzJgJ9Vhc4DCvkkf6NTgeYN/ME2NJMQKDe7ityF1s5q5r2WT1ltW8pZ MIYXJIWXEtzpVk36AqUnKooDNCdoVPaNFirTFhilqJbOK1MsdskkENl7rlhEteroX8fZ ZxKMsg+1sXkaRogMdzTgCAgspK9uWocj8DqD/+oW9h+frF8O3r3uKc6mumbmfR3/Xa+E ThZ5Q6gDi64N/tLsaF6S0TMZmF6KaTw7En/Pnypn/A5LSENL6T/XYn0sqZqqLH5g+GxE 624CaQX+ZqEI5/hhl0x1mOvq2OydXeZ7vAb/Zwz9IJ5nnMoaUJn6sn5/R2ZjF/nf1Ypl ZVog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697785496; x=1698390296; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:cc:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=/ZqypdHtXbUCfZ8Ja9jjxRFQyEjnWkCGI+nL0QIyr7c=; b=aJGTkftpz+tcqr+frYAvhe7RTWuJL8Wnf9lKKZq5cSCUMagcUxehDL336onRMUbfRJ wMODMdCYnbPvQmgKL5MbFWcVLzwd5OCDKnSWkysmHjaxsrQTOADMdBGhzA06Hv7b3Mel ARVTantCAr1ZzeRoJBVY+vl7hIwkCwIHl6yoouDSqt74Xu+b5BOtIodn36y/ttfoesO5 ZxktHARZ9NThjQSV/hbgrICgx6yOJtYhswjGrQwm5B4nt8ianO3GelZeH8KQ7Z/6HX5z xzdikH1huGFmqde8oBajv6BupLzMxU/14fS0OPyRR8c/0L4DkOMvxmB0ySEIJZLXwi8a 7wbA== X-Gm-Message-State: AOJu0YxoYArYDGH/NhGQqj7CFdRknOBP3/kbPaLRiMmiHi3CYoFNCWjm tkal9baSgdBiVHdTnGsEXNk= X-Google-Smtp-Source: AGHT+IHeadO0npDahRVyhgrzHjHDeT8D1EpaODT9bWVPy9JKv0kpiuPzVrMRVJslb5IpY5t7Ayzp4A== X-Received: by 2002:ac2:5ec9:0:b0:507:95ea:1e72 with SMTP id d9-20020ac25ec9000000b0050795ea1e72mr618560lfq.22.1697785496177; Fri, 20 Oct 2023 00:04:56 -0700 (PDT) Received: from [192.168.1.23] (ip-046-223-203-173.um13.pools.vodafone-ip.de. [46.223.203.173]) by smtp.gmail.com with ESMTPSA id a16-20020adffad0000000b0032d9f32b96csm1012409wrs.62.2023.10.20.00.04.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 20 Oct 2023 00:04:55 -0700 (PDT) Message-ID: <6cf3deca-2edc-419a-a66c-b987324e3da9@gmail.com> Date: Fri, 20 Oct 2023 09:04:55 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: rdapp.gcc@gmail.com Subject: Re: [PATCH v3 1/4] ifcvt: handle sequences that clobber flags in noce_convert_multiple_sets To: Manolis Tsamis , gcc-patches@gcc.gnu.org, Philipp Tomsich , Jakub Jelinek , richard.sandiford@arm.com References: <20230830101400.1539313-1-manolis.tsamis@vrull.eu> <20230830101400.1539313-2-manolis.tsamis@vrull.eu> Content-Language: en-US From: Robin Dapp In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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 List-Id: > Sorry for the slow review. TBH I was hoping someone else would pick > it up, since (a) I'm not very familiar with this code, and (b) I don't > really agree with the way that the current code works. I'm not sure the > current dependency checking is safe, so I'm nervous about adding even > more cases to it. And it feels like the different ifcvt techniques are > not now well distinguished, so that they're beginning to overlap and > compete with one another. None of that is your fault, of course. :) I might be to blame, at least partially :) The idea back then was to do it here because (1) it can handle cases the other part cannot and (2) its costing is based on sequence cost rather than just counting instructions. > This condition looks odd. A SET_SRC is never null. But more importantly, > continuing means "assume the best", and I don't think we should assume > the best for (say) an insn with two parallel sets. IIRC I didn't consider two parallel sets at all :/ > But I'm not sure which cases this code is trying to catch. Is it trying > to catch cases where seq2 "spontaneously" uses registers that happen to > overlap with cond? If so, then when does that happen? And if it does > happen, wouldn't the sequence also have to set the registers first? In order for sequence costing to be superior to just counting "conditional" instructions we need to make sure that as few redundant instructions as possible are present in the costed sequences. (redundant as in "will be removed in a subsequent pass"). So, originally, the backend would always be passed a comparison (set cc (cmp a b)). On e.g. s390 this would result in at least two redundant instructions per conditional move so costing was always wrong. When passing the backend the comparison "result" e.g. (cmp cc 0) there is no redundant instruction. Now, passing the comparison is useful as well when we want to turn a conditional move into a min/max in the backend. This, however, overwrites the initial condition result and any subsequent iterations must not pass the result but the comparison itself to the backend. In my first approach I hadn't considered several special cases which, of course, complicated things further down the line. All that said - maybe trying hard to get rid of later-redundant insns is not a good approach after all? A lot of complexity would go away if we counted emitted conditional instructions just like the other ifcvt part. Maybe a middle ground would be to cost the initial sequence as well as if + jmp and compare this against insn_cost of only the created conditional insns? In that case we might need to tighten/audit our preconditions in order not to accidentally allow cases that result in strictly worse code. But maybe that's an easier problem than what we are currently solving? Regards Robin