From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x532.google.com (mail-ed1-x532.google.com [IPv6:2a00:1450:4864:20::532]) by sourceware.org (Postfix) with ESMTPS id C8DC23857817 for ; Wed, 2 Jun 2021 10:12:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C8DC23857817 Received: by mail-ed1-x532.google.com with SMTP id b17so2308513ede.0 for ; Wed, 02 Jun 2021 03:12:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=W5/sCZwd/Z4QFLgXyolimtt+tasZyBLLEkq4jVl/3A0=; b=SSgnfplAaZ0yrSham/uAgJazE5TpZlfUEHt8XAAKdeXmsFBD/XoBCL+pG55411Vk2T mQqN+Ortpww8Ss98ZVExOFGvRoDLklM7IRA3LFviXiYA35zHqGIap8ECzuOoF97bzpPF cK78td5w+Lh3rkt0JRncWe0F7zrHcrROYJeu0s8hap5LuwUwg1KGgVV4SQXS2DVc4q1H WSayUeJSzGEdXUkkcRJ8F+Tvevn4gDXCgVHH4KeGizrVmmfD3VDehhZeIXbIYUM++hSb j7TfIQvHH3AffZt5hfvZkY7uYG+h1MRlHVFdpbU51FUZ0OMxl2D/YGO7eFom4wapf33U jZMA== X-Gm-Message-State: AOAM531umm+JRuUEHozXLbbOYxxnceyfkKZSvN6UCIk0VryBg4y9X8hk j60CDGKcKdvXudPPIrKQ7Zt8JHp3Fe5DL51xiC0= X-Google-Smtp-Source: ABdhPJxPlSPPmn9tHLdBbWTcQI7H/aKdET8Et6XHrkPsDXl0zefjjXuGO63nQAtozYlCurkZZalCtPei1sQu31vZQ+c= X-Received: by 2002:a05:6402:4256:: with SMTP id g22mr37110776edb.214.1622628735642; Wed, 02 Jun 2021 03:12:15 -0700 (PDT) MIME-Version: 1.0 References: <0a03d10f-522d-cf22-6c66-4e0b9c5d042f@linux.ibm.com> In-Reply-To: From: Richard Biener Date: Wed, 2 Jun 2021 12:12:04 +0200 Message-ID: Subject: Re: [RFC/PATCH 00/11] Fix up some unexpected empty split conditions To: "Kewen.Lin" Cc: Richard Sandiford , GCC Patches , Bill Schmidt , Segher Boessenkool , Jeff Law , Jakub Jelinek Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00, BODY_8BITS, 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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Wed, 02 Jun 2021 10:12:18 -0000 On Wed, Jun 2, 2021 at 12:01 PM Kewen.Lin wrote: > > on 2021/6/2 =E4=B8=8B=E5=8D=885:13, Richard Sandiford wrote: > > "Kewen.Lin" writes: > >> Hi Richard, > >> > >> on 2021/6/2 =E9=94=9F=E6=96=A4=E6=8B=B7=E9=94=9F=E6=96=A4=E6=8B=B74:11= , Richard Sandiford wrote: > >>> Kewen Lin writes: > >>>> Hi all, > >>>> > >>>> define_insn_and_split should avoid to use empty split condition > >>>> if the condition for define_insn isn't empty, otherwise it can > >>>> sometimes result in unexpected consequence, since the split > >>>> will always be done even if the insn condition doesn't hold. > >>>> > >>>> To avoid forgetting to add "&& 1" onto split condition, as > >>>> Segher suggested in thread[1], this series is to add the check > >>>> and raise an error if it catches the unexpected cases. With > >>>> this new check, we have to fix up some existing > >>>> define_insn_and_split which are detected as error. I hope all > >>>> these places are not intentional to be kept as blank. > >>> > >>> I wonder whether we should instead redefine the semantics of > >>> define_insn_and_split so that the split condition is always applied > >>> on top of the insn condition. It's rare for a define_insn_and_split > >>> to have independent insn and split conditions, so at the moment, > >>> we're making the common case hard. > >>> > >> > >> Just want to confirm that the suggestion is just applied for empty > >> split condition or all split conditions in define_insn_and_split? > >> I guess it's the former? > > > > No, I meant tha latter. E.g. in: > > > > (define_insn_and_split > > [=E2=80=A6] > > "TARGET_FOO" > > "=E2=80=A6" > > [=E2=80=A6] > > "reload_completed" > > [=E2=80=A6] > > ) > > > > the "reload_completed" condition is almost always a typo for > > "&& reload_completed". > > > > Like I say, it rarely makes sense for the split condition to > > ignore the insn condition and specify an entirely independent condition= . > > There might be some define_insn_and_splits that do that, but it'd often > > be less confusing to write the insn and split separately if so. > > > > Even if we do want to support independent insn and split conditions, > > that's always going to be the rare and surprising case, so it's the one > > that should need extra syntax. > > > > Thanks for the clarification! > > Since it may impact all ports, I wonder if there is a way to find out > this kind of "rare and surprising" case without a big coverage testing? > I'm happy to make a draft patch for it, but not sure how to early catch > those cases which need to be rewritten for those ports that I can't test > on (even with cfarm machines, the coverage seems still limited). So what Richard suggests would be to disallow split conditions that do not start with "&& ", it's probably easy to do that as well and look for build fails. That should catch all cases to look at. Richard. > BR, > Kewen >