From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30799 invoked by alias); 23 Jan 2020 15:03:13 -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 30791 invoked by uid 89); 23 Jan 2020 15:03:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=HX-Received:a6b X-HELO: mail-io1-f68.google.com Received: from mail-io1-f68.google.com (HELO mail-io1-f68.google.com) (209.85.166.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Jan 2020 15:03:11 +0000 Received: by mail-io1-f68.google.com with SMTP id n21so3202109ioo.10 for ; Thu, 23 Jan 2020 07:03:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KxTfW3tCxOR+b70Z2Dvwbor7PIa+Pco8pisSDdqylk8=; b=jb7w0ZGZX1mqfCsqKZW0LHAeTct0i1SWAOz17jwkHPSbSGqOCij19JgQLtat4y3+5U Vn6Y0OkkP3KP7A/5idSpItnXQTpXr37+3Vpil69Teg6+yPYy2Mz78gGC6tQ21mrku3hl QmWntps+Qo9G8yflFauAKeJsYzN9U2jxPd95ISTvdH4ThuLns4Ljr/D15dy+8YU0g5Qp d9kKrLbj10czZWOvAgXYDGW71NRdHEuGYoUQZko2hNXUO9qk9sFKNfx3YPYizQ/I/Q/K SOSddGvXWTaGzHzoZueZUjdhOiV6CtTwKjQOOn3KKzbwl+Z2qo9Obx0IWyAYuwYofK+K +hVA== MIME-Version: 1.0 References: <20200123081643.GV10088@tucnak> <20200123093256.GW10088@tucnak> <20200123131715.GY10088@tucnak> In-Reply-To: <20200123131715.GY10088@tucnak> From: Uros Bizjak Date: Thu, 23 Jan 2020 15:15:00 -0000 Message-ID: Subject: Re: [PATCH] wide-int: i386: Fix ICEs on TImode signed overflow add/sub patterns [PR93376] To: Jakub Jelinek Cc: Richard Biener , "gcc-patches@gcc.gnu.org" , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2020-01/txt/msg01588.txt.bz2 On Thu, Jan 23, 2020 at 2:17 PM Jakub Jelinek wrote: > > On Thu, Jan 23, 2020 at 10:38:31AM +0100, Uros Bizjak wrote: > > On Thu, Jan 23, 2020 at 10:33 AM Jakub Jelinek wrote: > > > > > > On Thu, Jan 23, 2020 at 09:14:42AM +0000, Richard Sandiford wrote: > > > > > The other patch is something suggested by Richard S., avoid using OImode > > > > > for this and instead use a partial int mode that is smaller. This is still > > > > > playing with fire because even the partial int mode is larger than > > > > > MAX_BITSIZE_MODE_ANY_INT, but at least its precision fits into those 3 > > > > > WIDE_INT_MAX_ELTS wide-int.h uses. > > > > > > > > I think we should increase MAX_BITSIZE_MODE_ANY_INT to the precision > > > > of POImode at the same time, and make that precision less than 192 > > > > (191 maybe?). That way everything is "correct" without increasing > > > > the size of wide_int. > > > > > > The 192 was chosen just to be nice and round, I guess it could be just 130 > > > or say 160 (128 + 32). > > > > > > Uros, would you be ok with bumping MAX_BITSIZE_MODE_ANY_INT to 160 > > > and changing the POImode patch to use 160 bits instead of 192 > > > if it passes testing? > > > > We can try 160 and hope that nothing breaks due to "weird" number. > > Following passed bootstrap/regtest on x86_64-linux and i686-linux. > > Ok for trunk then? > > 2020-01-23 Jakub Jelinek > > PR target/93376 > * config/i386/i386-modes.def (POImode): New mode. > (MAX_BITSIZE_MODE_ANY_INT): Change from 128 to 160. > * config/i386/i386.md (DPWI): New mode attribute. > (addv4, subv4): Use instead of . > (QWI): Rename to... > (QPWI): ... this. Use POI instead of OI for TImode. > (*addv4_doubleword, *addv4_doubleword_1, > *subv4_doubleword, *subv4_doubleword_1): Use > instead of . > > * gcc.dg/pr93376.c: New test. LGTM. Thanks, Uros. > --- gcc/config/i386/i386-modes.def.jj 2020-01-22 16:10:29.812837184 +0100 > +++ gcc/config/i386/i386-modes.def 2020-01-23 11:43:37.931345071 +0100 > @@ -107,10 +107,19 @@ INT_MODE (XI, 64); > PARTIAL_INT_MODE (HI, 16, P2QI); > PARTIAL_INT_MODE (SI, 32, P2HI); > > +/* Mode used for signed overflow checking of TImode. As > + MAX_BITSIZE_MODE_ANY_INT is only 160, wide-int.h reserves only that > + rounded up to multiple of HOST_BITS_PER_WIDE_INT bits in wide_int etc., > + so OImode is too large. For the overflow checking we actually need > + just 1 or 2 bits beyond TImode precision. Use 160 bits to have > + a multiple of 32. */ > +PARTIAL_INT_MODE (OI, 160, POI); > + > /* Keep the OI and XI modes from confusing the compiler into thinking > that these modes could actually be used for computation. They are > - only holders for vectors during data movement. */ > -#define MAX_BITSIZE_MODE_ANY_INT (128) > + only holders for vectors during data movement. Include POImode precision > + though. */ > +#define MAX_BITSIZE_MODE_ANY_INT (160) > > /* The symbol Pmode stands for one of the above machine modes (usually SImode). > The tm.h file specifies which one. It is not a distinct mode. */ > --- gcc/config/i386/i386.md.jj 2020-01-22 16:10:29.815837139 +0100 > +++ gcc/config/i386/i386.md 2020-01-23 11:40:54.923822116 +0100 > @@ -6054,15 +6054,18 @@ (define_insn "*addqi_ext_2" > [(set_attr "type" "alu") > (set_attr "mode" "QI")]) > > +;; Like DWI, but use POImode instead of OImode. > +(define_mode_attr DPWI [(QI "HI") (HI "SI") (SI "DI") (DI "TI") (TI "POI")]) > + > ;; Add with jump on overflow. > (define_expand "addv4" > [(parallel [(set (reg:CCO FLAGS_REG) > (eq:CCO > - (plus: > - (sign_extend: > + (plus: > + (sign_extend: > (match_operand:SWIDWI 1 "nonimmediate_operand")) > (match_dup 4)) > - (sign_extend: > + (sign_extend: > (plus:SWIDWI (match_dup 1) > (match_operand:SWIDWI 2 > ""))))) > @@ -6078,7 +6081,7 @@ (define_expand "addv4" > if (CONST_SCALAR_INT_P (operands[2])) > operands[4] = operands[2]; > else > - operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]); > + operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]); > }) > > (define_insn "*addv4" > @@ -6123,17 +6126,17 @@ (define_insn "addv4_1" > (const_string "")))]) > > ;; Quad word integer modes as mode attribute. > -(define_mode_attr QWI [(SI "TI") (DI "OI")]) > +(define_mode_attr QPWI [(SI "TI") (DI "POI")]) > > (define_insn_and_split "*addv4_doubleword" > [(set (reg:CCO FLAGS_REG) > (eq:CCO > - (plus: > - (sign_extend: > + (plus: > + (sign_extend: > (match_operand: 1 "nonimmediate_operand" "%0,0")) > - (sign_extend: > + (sign_extend: > (match_operand: 2 "x86_64_hilo_general_operand" "r,o"))) > - (sign_extend: > + (sign_extend: > (plus: (match_dup 1) (match_dup 2))))) > (set (match_operand: 0 "nonimmediate_operand" "=ro,r") > (plus: (match_dup 1) (match_dup 2)))] > @@ -6172,11 +6175,11 @@ (define_insn_and_split "*addv4_doub > (define_insn_and_split "*addv4_doubleword_1" > [(set (reg:CCO FLAGS_REG) > (eq:CCO > - (plus: > - (sign_extend: > + (plus: > + (sign_extend: > (match_operand: 1 "nonimmediate_operand" "%0")) > - (match_operand: 3 "const_scalar_int_operand" "")) > - (sign_extend: > + (match_operand: 3 "const_scalar_int_operand" "")) > + (sign_extend: > (plus: > (match_dup 1) > (match_operand: 2 "x86_64_hilo_general_operand" ""))))) > @@ -6570,11 +6573,11 @@ (define_insn "*subsi_2_zext" > (define_expand "subv4" > [(parallel [(set (reg:CCO FLAGS_REG) > (eq:CCO > - (minus: > - (sign_extend: > + (minus: > + (sign_extend: > (match_operand:SWIDWI 1 "nonimmediate_operand")) > (match_dup 4)) > - (sign_extend: > + (sign_extend: > (minus:SWIDWI (match_dup 1) > (match_operand:SWIDWI 2 > ""))))) > @@ -6590,7 +6593,7 @@ (define_expand "subv4" > if (CONST_SCALAR_INT_P (operands[2])) > operands[4] = operands[2]; > else > - operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]); > + operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]); > }) > > (define_insn "*subv4" > @@ -6637,12 +6640,12 @@ (define_insn "subv4_1" > (define_insn_and_split "*subv4_doubleword" > [(set (reg:CCO FLAGS_REG) > (eq:CCO > - (minus: > - (sign_extend: > + (minus: > + (sign_extend: > (match_operand: 1 "nonimmediate_operand" "0,0")) > - (sign_extend: > + (sign_extend: > (match_operand: 2 "x86_64_hilo_general_operand" "r,o"))) > - (sign_extend: > + (sign_extend: > (minus: (match_dup 1) (match_dup 2))))) > (set (match_operand: 0 "nonimmediate_operand" "=ro,r") > (minus: (match_dup 1) (match_dup 2)))] > @@ -6679,11 +6682,11 @@ (define_insn_and_split "*subv4_doub > (define_insn_and_split "*subv4_doubleword_1" > [(set (reg:CCO FLAGS_REG) > (eq:CCO > - (minus: > - (sign_extend: > + (minus: > + (sign_extend: > (match_operand: 1 "nonimmediate_operand" "0")) > - (match_operand: 3 "const_scalar_int_operand" "")) > - (sign_extend: > + (match_operand: 3 "const_scalar_int_operand" "")) > + (sign_extend: > (minus: > (match_dup 1) > (match_operand: 2 "x86_64_hilo_general_operand" ""))))) > --- gcc/testsuite/gcc.dg/pr93376.c.jj 2020-01-23 11:40:54.923822116 +0100 > +++ gcc/testsuite/gcc.dg/pr93376.c 2020-01-23 11:40:54.923822116 +0100 > @@ -0,0 +1,20 @@ > +/* PR target/93376 */ > +/* { dg-do compile { target int128 } } */ > +/* { dg-options "-Og -finline-functions-called-once" } */ > + > +unsigned a, b, c; > + > +int > +bar (int x) > +{ > + short s = __builtin_sub_overflow (~x, 0, &b); > + a = __builtin_ffsll (~x); > + return __builtin_add_overflow_p (-(unsigned __int128) a, s, > + (unsigned __int128) 0); > +} > + > +void > +foo (void) > +{ > + c = bar (0); > +} > > > Jakub >