From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x133.google.com (mail-lf1-x133.google.com [IPv6:2a00:1450:4864:20::133]) by sourceware.org (Postfix) with ESMTPS id 496913858C52 for ; Mon, 14 Nov 2022 09:12:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 496913858C52 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-lf1-x133.google.com with SMTP id r12so18245221lfp.1 for ; Mon, 14 Nov 2022 01:12:21 -0800 (PST) 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=QTHmyLz8mH68TC1OX7dCDiL4kMClRZ1qFl6Lj5sR/Wk=; b=o4l4kb7tM59adEvTN6kLcjQJwCeLOEMMFA5LMgx1PUanFoOY//si/mlM/Li1kq0Oi1 v1M9xKUjcnokSFZ+JtR02aDL2sZiwVJp+a9f+HTHkZ0okaNBqBgP+6LWKCpaqs7pY9bH ZPdeLIAzxlzbNBVS6fnGSkaTw6kWH6SOy2bICGBzq7ENMnj0rl91uUtSa79VrE5x8npA zrb+xv/LN1rN4249cI9usxQgvmVnR03Mx1hlJIGVAE/VcwhqXLv9whhGQcoPClq9arJm LRQQghv1LjmD2s3n3l2n8+AAK25H0Uz0pQ5KiBKEsVxgh7IH6zjrbPuKngRHUzAXtmmc sKWg== 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=QTHmyLz8mH68TC1OX7dCDiL4kMClRZ1qFl6Lj5sR/Wk=; b=NCjAZIS2HlYg2LQRkYk5jyC8MQxmDLvhBsLDHN5BbNYIJEJrRavvr7+qnEZrKEN51F 4Nw7h6ggJAyC8x48AwVRv7+Bj+4cr9bB6dJTNXaIgICPNnG7rfVgnWmfLZ4n+rJYyye0 BQ36665PijLH4jKst5BqTU16aTqZQYhmmkavbLZldGmbYJkvZgA1WZ+ks3tU0XgBE1eh ojQri/jDuaSxoOPiLGfgO0aYkneaxxGTu4Enenw57Kx+mcUiNO7nhZxlY9p7PJyZABT+ xo+Hb8PvHLkkvFdLWSvUBGqkqwRhrewoIEqVmd7mZYfH8j/TKwkg123SevjER/Q3EUm3 Z3BA== X-Gm-Message-State: ANoB5pn60/yM7120tU5ky9BgbwtRGWpRSd1tQtNHSGxocLCdC0otPPmN tbdEcKHp5dxsx8uhow4+6jL/s+W9pHmJ7unGcm0= X-Google-Smtp-Source: AA0mqf5vXZsRdhB4lu1rJCFz21ps1E3sDI9W9pB7axsVY/0L4VbChN2QaFOcOEt6hs/dYYyppQmucbtXQfKvr+Jmzsw= X-Received: by 2002:a05:6512:3c89:b0:499:b4b3:2f68 with SMTP id h9-20020a0565123c8900b00499b4b32f68mr3672991lfv.203.1668417139738; Mon, 14 Nov 2022 01:12:19 -0800 (PST) MIME-Version: 1.0 References: <20221112183048.389811-1-aldyh@redhat.com> In-Reply-To: <20221112183048.389811-1-aldyh@redhat.com> From: Richard Biener Date: Mon, 14 Nov 2022 10:12:07 +0100 Message-ID: Subject: Re: [PATCH] [PR68097] Try to avoid recursing for floats in tree_*_nonnegative_warnv_p. To: Aldy Hernandez Cc: GCC patches , Andrew MacLeod , richard.sandiford@arm.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,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 Sat, Nov 12, 2022 at 7:30 PM Aldy Hernandez wrote: > > It irks me that a PR named "we should track ranges for floating-point > hasn't been closed in this release. This is an attempt to do just > that. > > As mentioned in the PR, even though we track ranges for floats, it has > been suggested that avoiding recursing through SSA defs in > gimple_assign_nonnegative_warnv_p is also a goal. We can do this with > various ranger components without the need for a heavy handed approach > (i.e. a full ranger). > > I have implemented two versions of known_float_sign_p() that answer > the question whether we definitely know the sign for an operation or a > tree expression. > > Both versions use get_global_range_query, which is a wrapper to query > global ranges. This means, that no caching or propagation is done. > In the case of an SSA, we just return the global range for it (think > SSA_NAME_RANGE_INFO). In the case of a tree code with operands, we > also use get_global_range_query to resolve the operands, and then call > into range-ops, which is our lowest level component. There is no > ranger or gori involved. All we're doing is resolving the operation > with the ranges passed. > > This is enough to avoid recursing in the case where we definitely know > the sign of a range. Otherwise, we still recurse. > > Note that instead of get_global_range_query(), we could use > get_range_query() which uses a ranger (if active in a pass), or > get_global_range_query if not. This would allow passes that have an > active ranger (with enable_ranger) to use a full ranger. These passes > are currently, VRP, loop unswitching, DOM, loop versioning, etc. If > no ranger is active, get_range_query defaults to global ranges, so > there's no additional penalty. > > Would this be acceptable, at least enough to close (or rename the PR ;-))? I think the checks would belong to the gimple_stmt_nonnegative_warnv_p function only (that's the SSA name entry from the fold-const.cc ones)? I also notice the use of 'bool' for the "sign". That's not really descriptive. We have SIGNED and UNSIGNED (aka enum signop), not sure if that's the perfect match vs. NEGATIVE and NONNEGATIVE. Maybe the functions name is just bad and they should be known_float_negative_p? > PR tree-optimization/68097 > > gcc/ChangeLog: > > * fold-const.cc (known_float_sign_p): New. > (tree_unary_nonnegative_warnv_p): Call known_float_sign_p. > (tree_binary_nonnegative_warnv_p): Same. > (tree_single_nonnegative_warnv_p): Same. > --- > gcc/fold-const.cc | 51 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc > index b89cac91cae..bd74cfca996 100644 > --- a/gcc/fold-const.cc > +++ b/gcc/fold-const.cc > @@ -14577,6 +14577,44 @@ tree_simple_nonnegative_warnv_p (enum tree_code code, tree type) > return false; > } > > +/* Return true if T is of type floating point and has a known sign. > + If so, set the sign in SIGN. */ > + > +static bool > +known_float_sign_p (bool &sign, tree t) > +{ > + if (!frange::supports_p (TREE_TYPE (t))) > + return false; > + > + frange r; > + return (get_global_range_query ()->range_of_expr (r, t) > + && r.signbit_p (sign)); > +} > + > +/* Return true if TYPE is a floating-point type and (CODE OP0 OP1) has > + a known sign. If so, set the sign in SIGN. */ > + > +static bool > +known_float_sign_p (bool &sign, enum tree_code code, tree type, tree op0, > + tree op1 = NULL_TREE) > +{ > + if (!frange::supports_p (type)) > + return false; > + > + range_op_handler handler (code, type); > + if (handler) > + { > + frange res, r0, r1; > + get_global_range_query ()->range_of_expr (r0, op0); > + if (op1) > + get_global_range_query ()->range_of_expr (r1, op1); > + else > + r1.set_varying (type); > + return handler.fold_range (res, type, r0, r1) && res.signbit_p (sign); > + } > + return false; > +} > + > /* Return true if (CODE OP0) is known to be non-negative. If the return > value is based on the assumption that signed overflow is undefined, > set *STRICT_OVERFLOW_P to true; otherwise, don't change > @@ -14589,6 +14627,10 @@ tree_unary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0, > if (TYPE_UNSIGNED (type)) > return true; > > + bool sign; > + if (known_float_sign_p (sign, code, type, op0)) > + return !sign; > + > switch (code) > { > case ABS_EXPR: > @@ -14656,6 +14698,10 @@ tree_binary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0, > if (TYPE_UNSIGNED (type)) > return true; > > + bool sign; > + if (known_float_sign_p (sign, code, type, op0, op1)) > + return !sign; > + > switch (code) > { > case POINTER_PLUS_EXPR: > @@ -14778,6 +14824,8 @@ tree_binary_nonnegative_warnv_p (enum tree_code code, tree type, tree op0, > bool > tree_single_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth) > { > + bool sign; > + > if (TYPE_UNSIGNED (TREE_TYPE (t))) > return true; > > @@ -14796,6 +14844,9 @@ tree_single_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth) > return RECURSE (TREE_OPERAND (t, 1)) && RECURSE (TREE_OPERAND (t, 2)); > > case SSA_NAME: > + if (known_float_sign_p (sign, t)) > + return !sign; > + > /* Limit the depth of recursion to avoid quadratic behavior. > This is expected to catch almost all occurrences in practice. > If this code misses important cases that unbounded recursion > -- > 2.38.1 >