From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb36.google.com (mail-yb1-xb36.google.com [IPv6:2607:f8b0:4864:20::b36]) by sourceware.org (Postfix) with ESMTPS id 8AA9E38582AD for ; Fri, 28 Oct 2022 06:05:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8AA9E38582AD Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-yb1-xb36.google.com with SMTP id m125so5053356ybb.6 for ; Thu, 27 Oct 2022 23:05:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=XuPq2R5jFWC027q1lz5Zuw+/QaJNLt9wY415jZmNLfg=; b=c78K3r5GIi/I6rj8/hKFZdOZQBZnhUfcVhAp/SchmiziiOemoTX9RG85oxN/jlIXrC oO25k0LfYBRCh/3hYZRB4GFoajia/pO5mTrX3VzTke3fIkHOXTfNMHYiV+5ajqQ79nNs sUjmUIHRC91aAYEjcj89npVa5NOnna6FT+vQuOuC/GvUUJmpjMcwQHsgcfCHaDqPOGXK TnwYiyUrgA0A4xf9iVOeOqnIDTBhqzjjGtrLnS7nJYng50wHhY293gHyOLB5fVB/Pmpu qbivLHOR8Yv+yCz3fvLlaZJ3b6waybYBaQHsGFfTSf6CXyC2CisMObTDgf8FYkqhVpzU hYDQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=XuPq2R5jFWC027q1lz5Zuw+/QaJNLt9wY415jZmNLfg=; b=lU7GweQxrk4NpWe1VKEnxnDadGakUkjOcZGQa3HFdnZZ7iztjFGHIVYBuvRwf6IoLh GzagAQS+KSyK9L+rksjpKFsJ18dHQeY8VOA4QIrDsTVtrTo2WexU7VgwbqJOjlXi+uGU gDuE4jdCn5vgI56DS9ya34qDosFeKvhZOJikv9i5Ix4pPJpMqHXAkc0p00GkxGlX2Xr/ kaw2+YZm6MAaruTG8B9bm20dfnx3Ds2OS+FFJgkHPEAHrk0dCSGxio7xJ36YM42Ev17G TTQQcnXMfZPphPiW5xYmVy74MGlSzQ/tVetavDZymoeTgAu/0UP3KLMIpOzGkQD96gsw zNaQ== X-Gm-Message-State: ACrzQf2Qreyw8jSHQGdktB5kiCT+GjrxU50w6msSMxNUViTOPwejefjH iJ2f+Ss29Cm3gpSQedLdeqKDMBn2C9hqJFMmHYqf9vR8nZY= X-Google-Smtp-Source: AMsMyM6QrQQi3CFD+D7qyvdQXUGmtHtMoWlxwcU/DVzNtQ20ex0uFYBZTdUEz/wEWI8pL2sOgtDslWuLmAuetHh6Jes= X-Received: by 2002:a05:6902:150e:b0:6ca:84f9:e837 with SMTP id q14-20020a056902150e00b006ca84f9e837mr32622659ybu.251.1666937138833; Thu, 27 Oct 2022 23:05:38 -0700 (PDT) MIME-Version: 1.0 References: <20221026185857.234023-1-hjl.tools@gmail.com> In-Reply-To: From: Hongtao Liu Date: Fri, 28 Oct 2022 14:08:46 +0800 Message-ID: Subject: Re: [PATCH] x86: Replace ne:CCC/ne:CCO with UNSPEC_CC_NE in neg patterns To: "H.J. Lu" Cc: gcc-patches@gcc.gnu.org, Uros Bizjak Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,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: On Fri, Oct 28, 2022 at 1:56 PM Hongtao Liu wrote: > > On Thu, Oct 27, 2022 at 2:59 AM H.J. Lu via Gcc-patches > wrote: > > > > In i386.md, neg patterns which set MODE_CC register like > > > > (set (reg:CCC FLAGS_REG) > > (ne:CCC (match_operand:SWI48 1 "general_reg_operand") (const_int 0))) > > > > can lead to errors when operand 1 is a constant value. If FLAGS_REG in > > > > (set (reg:CCC FLAGS_REG) > > (ne:CCC (const_int 2) (const_int 0))) > > > > is set to 1, RTX simplifiers may simplify > > > > (set (reg:SI 93) > > (neg:SI (ltu:SI (reg:CCC 17 flags) (const_int 0 [0])))) > > > > as > > > > (set (reg:SI 93) > > (neg:SI (ltu:SI (const_int 1) (const_int 0 [0])))) > > > > which leads to incorrect results since LTU on MODE_CC register isn't the > > same as "unsigned less than" in x86 backend. To prevent RTL optimizers > > from setting MODE_CC register to a constant, use UNSPEC_CC_NE to replace > > ne:CCC/ne:CCO when setting FLAGS_REG in neg patterns. > > > > gcc/ > > > > PR target/107172 > > * config/i386/i386.md (UNSPEC_CC_NE): New. > > Replace ne:CCC/ne:CCO with UNSPEC_CC_NE in neg patterns. > > > > gcc/testsuite/ > > > > PR target/107172 > > * gcc.target/i386/pr107172.c: New test. > > --- > > gcc/config/i386/i386.md | 45 +++++++++++++----------- > > gcc/testsuite/gcc.target/i386/pr107172.c | 26 ++++++++++++++ > > 2 files changed, 51 insertions(+), 20 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr107172.c > > > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > > index baf1f1f8fa2..aaa678e7314 100644 > > --- a/gcc/config/i386/i386.md > > +++ b/gcc/config/i386/i386.md > > @@ -113,6 +113,7 @@ (define_c_enum "unspec" [ > > UNSPEC_PEEPSIB > > UNSPEC_INSN_FALSE_DEP > > UNSPEC_SBB > > + UNSPEC_CC_NE > > > > ;; For SSE/MMX support: > > UNSPEC_FIX_NOTRUNC > > @@ -11470,7 +11471,7 @@ (define_insn_and_split "*neg2_doubleword" > > "&& reload_completed" > > [(parallel > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_dup 1) (const_int 0))) > > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE)) > > (set (match_dup 0) (neg:DWIH (match_dup 1)))]) > > (parallel > > [(set (match_dup 2) > > @@ -11499,7 +11500,8 @@ (define_peephole2 > > (match_operand:SWI48 1 "nonimmediate_gr_operand")) > > (parallel > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_operand:SWI48 2 "general_reg_operand") (const_int 0))) > > + (unspec:CCC [(match_operand:SWI48 2 "general_reg_operand") > > + (const_int 0)] UNSPEC_CC_NE)) > > (set (match_dup 2) (neg:SWI48 (match_dup 2)))]) > > (parallel > > [(set (match_dup 0) > > @@ -11517,7 +11519,7 @@ (define_peephole2 > > && !reg_mentioned_p (operands[2], operands[1])" > > [(parallel > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_dup 2) (const_int 0))) > > + (unspec:CCC [(match_dup 2) (const_int 0)] UNSPEC_CC_NE)) > > (set (match_dup 2) (neg:SWI48 (match_dup 2)))]) > > (parallel > > [(set (match_dup 0) > > @@ -11543,7 +11545,8 @@ (define_peephole2 > > (clobber (reg:CC FLAGS_REG))]) > > (parallel > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_operand:SWI48 1 "general_reg_operand") (const_int 0))) > > + (unspec:CCC [(match_operand:SWI48 1 "general_reg_operand") > > + (const_int 0)] UNSPEC_CC_NE)) > > (set (match_dup 1) (neg:SWI48 (match_dup 1)))]) > > (parallel > > [(set (match_dup 0) > > @@ -11559,7 +11562,7 @@ (define_peephole2 > > "REGNO (operands[0]) != REGNO (operands[1])" > > [(parallel > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_dup 1) (const_int 0))) > > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE)) > > (set (match_dup 1) (neg:SWI48 (match_dup 1)))]) > > (parallel > > [(set (match_dup 0) > > @@ -11635,9 +11638,9 @@ (define_insn "*negsi_2_zext" > > > > (define_insn "*neg_ccc_1" > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC > > - (match_operand:SWI 1 "nonimmediate_operand" "0") > > - (const_int 0))) > > + (unspec:CCC > > + [(match_operand:SWI 1 "nonimmediate_operand" "0") > > + (const_int 0)] UNSPEC_CC_NE)) > > (set (match_operand:SWI 0 "nonimmediate_operand" "=m") > > (neg:SWI (match_dup 1)))] > > "" > > @@ -11647,9 +11650,9 @@ (define_insn "*neg_ccc_1" > > > > (define_insn "*neg_ccc_2" > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC > > - (match_operand:SWI 1 "nonimmediate_operand" "0") > > - (const_int 0))) > > + (unspec:CCC > > + [(match_operand:SWI 1 "nonimmediate_operand" "0") > > + (const_int 0)] UNSPEC_CC_NE)) > > (clobber (match_scratch:SWI 0 "="))] > > "" > > "neg{}\t%0" > > @@ -11659,8 +11662,8 @@ (define_insn "*neg_ccc_2" > > (define_expand "x86_neg_ccc" > > [(parallel > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_operand:SWI48 1 "register_operand") > > - (const_int 0))) > > + (unspec:CCC [(match_operand:SWI48 1 "register_operand") > > + (const_int 0)] UNSPEC_CC_NE)) > > (set (match_operand:SWI48 0 "register_operand") > > (neg:SWI48 (match_dup 1)))])]) > > > > @@ -11686,8 +11689,9 @@ (define_insn "*negqi_ext_2" > > ;; Negate with jump on overflow. > > (define_expand "negv3" > > [(parallel [(set (reg:CCO FLAGS_REG) > > - (ne:CCO (match_operand:SWI 1 "register_operand") > > - (match_dup 3))) > > + (unspec:CCO > > + [(match_operand:SWI 1 "register_operand") > > + (match_dup 3)] UNSPEC_CC_NE)) > > (set (match_operand:SWI 0 "register_operand") > > (neg:SWI (match_dup 1)))]) > > (set (pc) (if_then_else > > @@ -11703,8 +11707,9 @@ (define_expand "negv3" > > > > (define_insn "*negv3" > > [(set (reg:CCO FLAGS_REG) > > - (ne:CCO (match_operand:SWI 1 "nonimmediate_operand" "0") > > - (match_operand:SWI 2 "const_int_operand"))) > > + (unspec:CCO [(match_operand:SWI 1 "nonimmediate_operand" "0") > > + (match_operand:SWI 2 "const_int_operand")] > > + UNSPEC_CC_NE)) > > (set (match_operand:SWI 0 "nonimmediate_operand" "=m") > > (neg:SWI (match_dup 1)))] > > "ix86_unary_operator_ok (NEG, mode, operands) > > @@ -11770,7 +11775,7 @@ (define_insn_and_split "*abs2_doubleword" > > "&& 1" > > [(parallel > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_dup 1) (const_int 0))) > > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE)) > > (set (match_dup 2) (neg:DWIH (match_dup 1)))]) > > (parallel > > [(set (match_dup 5) > > @@ -11814,7 +11819,7 @@ (define_insn_and_split "*nabs2_doubleword" > > "&& 1" > > [(parallel > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_dup 1) (const_int 0))) > > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE)) > > (set (match_dup 2) (neg:DWIH (match_dup 1)))]) > > (parallel > > [(set (match_dup 5) > > @@ -21456,7 +21461,7 @@ (define_split > > (const_int 0))))] > > [(set (reg:CCC FLAGS_REG) > > - (ne:CCC (match_dup 1) (const_int 0))) > > + (unspec:CCC [(match_dup 1) (const_int 0)] UNSPEC_CC_NE)) > Do we have a define_insn for this? It looks like a setcc to me. It's not, nevermind. > > (set (match_dup 0) > > (neg:SWI (ltu:SWI (reg:CCC FLAGS_REG) (const_int 0))))]) > > > > diff --git a/gcc/testsuite/gcc.target/i386/pr107172.c b/gcc/testsuite/gcc.target/i386/pr107172.c > > new file mode 100644 > > index 00000000000..d2c85f3f47c > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/pr107172.c > > @@ -0,0 +1,26 @@ > > +/* { dg-do run } */ > > +/* { dg-options "-O1 -ftree-vrp" } */ > > + > > +int a, c, d; > > +int > > +main() > > +{ > > + long e = 1; > > + int f = a = 1; > > +L1: > > + if (a) > > + a = 2; > > + int h = e = ~e; > > + c = -1; > > + if (e >= a) > > + goto L2; > > + if (-1 > a) > > + goto L1; > > + if (a) > > + f = -1; > > +L2: > > + d = (-f + d) & h; > > + if (d) > > + __builtin_abort(); > > + return 0; > > +} > > -- > > 2.37.3 > > > > > -- > BR, > Hongtao -- BR, Hongtao