This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: target/17107: Fix
- From: Roger Sayle <roger at eyesopen dot com>
- To: Nathan Sidwell <nathan at codesourcery dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, David Edelsohn <dje at watson dot ibm dot com>
- Date: Wed, 17 Nov 2004 19:49:56 -0700 (MST)
- Subject: Re: target/17107: Fix
On Wed, 17 Nov 2004, Nathan Sidwell wrote:
> 2004-11-17 Nathan Sidwell <nathan@codesourcery.com>
>
> PR target/17107
> * fold-const.c (RANGE_TEST_NON_SHORT_CIRCUIT): Rename to ...
> (LOGICAL_OP_NON_SHORT_CIRCUIT): ... here.
> (fold_range_test): Adjust.
> (fold_truth_op): Use it.
> * config/rs6000/rs6000.h (RANGE_TEST_NON_SHORT_CIRCUIT): Rename to ...
> (LOGICAL_OP_NON_SHORT_CIRCUIT): ... here.
Sorry for the delay, it took a while to bootstrap powerpc-unknown-linux-gnu
and step through PR 17107 in the debugger. This patch is OK for mainline,
though I do have one minor suggestion.
I'd personally prefer to keep the current structure of fold_truthop; the
existing code is both shorter (20 vs 26 lines), and IMHO slightly easier
to understand. The change to introduce the new local variable inner_code,
and flow control through a switch statement probably does reduce the size
of the final executable, but my personal belief is that its preferrable
to keep fold's control flow as transparent as possible, ideally as the
original conditionalized transformations (rules) with no or few sequential
dependencies between them. This simplifies changes (such as your patch)
that need only modify the conditions upon which a rule/transform triggers.
With the above tweak, your change to fold_truthop actually becomes a
one-liner, i.e. "+ if (BLAH)". I hope you don't strongly disagree?
Many thanks and sorry again for the delay.
Roger
--