This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Implement one optimization from build_range_check in match.pd (PR tree-optimization/81346)
- From: Martin Sebor <msebor at gmail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Richard Biener <rguenther at suse dot de>, gcc-patches at gcc dot gnu dot org, Marc Glisse <marc dot glisse at inria dot fr>
- Date: Tue, 18 Jul 2017 10:47:37 -0600
- Subject: Re: [PATCH] Implement one optimization from build_range_check in match.pd (PR tree-optimization/81346)
- Authentication-results: sourceware.org; auth=none
- References: <20170718134730.GQ2123@tucnak> <bdc0ed1e-4aae-7aa9-f533-72ae87f900e5@gmail.com> <20170718154313.GS2123@tucnak>
On 07/18/2017 09:43 AM, Jakub Jelinek wrote:
On Tue, Jul 18, 2017 at 09:31:11AM -0600, Martin Sebor wrote:
--- gcc/match.pd.jj 2017-07-17 16:25:20.000000000 +0200
+++ gcc/match.pd 2017-07-18 12:32:52.896924558 +0200
@@ -1125,6 +1125,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
&& wi::neg_p (@1, TYPE_SIGN (TREE_TYPE (@1))))
(cmp @2 @0))))))
+/* (X - 1U) <= INT_MAX-1U into (int) X > 0. */
Since the transformation applies to other types besides int I
suggest to make it clear in the comment. E.g., something like:
/* (X - 1U) <= TYPE_MAX - 1U into (TYPE) X > 0 for any integer
TYPE. */
(with spaces around all the operators as per GCC coding style).
I think many of the match.pd comments are also not fully generic
to describe what it does, just to give an idea what it does.
...
Examples of other comments that "suffer" from similar lack of sufficient
genericity, but perhaps are good enough to let somebody understand it
quickly:
Sure, but that doesn't make them a good example to follow. As
someone pointed out to me in code reviews, existing deviations
from the preferred style, whether documented or not, or lack of
clarity, aren't a license to add more. Please take my suggestion
here in the same constructive spirit.
Martin