This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Overhaul __builtin_constant_p processing (take 2)

Hi Dave,
> I mentioned previously the regression for gcc.c-torture/execute/20010119-1.c
> on hppa2.0w-hp-hpux11* on the trunk.  I have tracked this regression to your
> patch:
> 2003-01-21  Roger Sayle  <roger at eyesopen dot com>
>         PR optimization/8423
> 	* cse.c (fold_rtx): Only eliminate a CONSTANT_P_RTX to 1 when
> 	its argument is constant, or 0 if !flag_gcse.
> 	* simplify-rtx.c (simplify_rtx): Convert CONSTANT_P_RTX to 1
> 	if it's argument is constant.
> 	* gcse.c (want_to_gcse_p): Ignore CONSTANT_P_RTX nodes.
> 	(hash_scan_set): Don't record CONSTANT_P_RTX expressions.
> 	(do_local_cprop): Don't propagate CONSTANT_P_RTX constants.
> 	* builtins.c (purge_builtin_constant_p): New function to force
> 	instantiation of any remaining CONSTANT_P_RTX nodes.
> 	* rtl.h (purge_builtin_constant_p): Prototype here.
> 	* toplev.c (rest_of_compilation): Invoke purge_builtin_constant_p
> 	pass after GCSE and before loop.
> 	(flag_gcse): No longer static.
> 	* flags.h (flag_gcse): Prototype here.

The world of GCC regressions is indeed a small one!

The failure of 20010119-1.c is not a failing of the above patch, but a
limitation of GCC's optimizers that was addressed by Kazu's patch that
caused the hppa bootstrap failure on mainline yesterday.

Let me explain.

Here's the important part of 20010119-1.c:

extern void foo (int b)
  int c = 0;
  int a = 10;
  while (c++ < b)
    (__builtin_constant_p (a) ? ((a) > 20000 ? undef () : bar (a)) : baz (a));

Prior to my 2003-01-21 patch, GCC always considered __builtin to be false,
and the loop would call the function "baz".  The improved constant
identification in my patch, now identifies it as a compile-time constant,
and, as the author probably originally intended, we now call "bar" on
most platforms.

The target specific problem is that hppa2.0w-hp-hpux11* is unable to
optimize the following:

extern void foo (int b)
  int c = 0;
  int a = 10;
  while (c++ < b)
    ((a) > 20000 ? undef () : bar (a));

i.e. the failure has absolutely nothing to do with __builtin_constant_p,
but is caused by GCC's inability to propagate the constant "a" into the
comparison.  This is triggered on PA and H8300 because the backend,
doesn't provide patterns that have comparison against immediate constant.

The solution that Kazu and I came up with is to use REG_EQUAL notes as
intermediate "patterns" that can be CSE'd and GCSE'd to allow both the
10 and 20000 constants to be propagated into a COMPARE.

Kazu's patch and the description of how this fixes 20010119-1.c is
described in

Unfortunately, as you pointed out yesterday on the gcc-patches list: his patch
runs into problems with the invalid (atleast dubious) REG_EQUAL notes
that GCC places on libcalls.

I posted a potential solution to the libcall REG_EQUAL notes issue, but before
I've had a convince myself its completely safe, Kazu decided to
revert his patch so that the affected platforms could continue to
bootstrap whilst the issues were ironed out.

I hope this explains the history and current status.  I believe
the way to proceed, once RTH agrees that the libcall REG_EQUAL
notes are indeed incorrect, is to apply my patch to avoid creating
them, then retry Kazu's GCSE patch.  The combination of the two
should fix 20010119-1.c on both H8300 and PA, and improving GCC's
handling of REG_EQUAL notes from there should fix a few more XFAILs
that I'm aware of on ARM, H8300 and x86_64.

Roger Sayle,                         E-mail: roger at eyesopen dot com
OpenEye Scientific Software,         WWW:
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]