[PING] PR112380: Defend against CLOBBERs in RTX expressions in combine.cc

[PING] PR112380: Defend against CLOBBERs in RTX expressions in combine.cc

[Roger Sayle]

I'd like to ping my patch for PR rtl-optimization/112380.
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636203.html

For those unfamiliar with the (clobber (const_int 0)) idiom used by
combine, I'll explain a little of the ancient history...

Back before time, in the prehistory of git/subversion/cvs or even
ChangeLogs,
in March 1987 to be precise, Richard Stallman's GCC version 0.9, had RTL
optimization passes similar to those in use today.  This far back, combine.c
contained the function gen_lowpart_for_combine, which was documented as
"Like gen_lowpart but for use in combine" where "it is not possible to
create any new pseudoregs." and "return zero if we don't see a way to
make a lowpart.".  And indeed, this function returned (rtx)0, and the
single caller of gen_lowpart_for_combine checked whether the return value
was non-zero.

Unfortunately, gcc 0.9's combine also contained bugs; At three places in
combine.c, it called gen_lowpart, the first of these looked like:
          return gen_rtx (AND, GET_MODE (x),
                          gen_lowpart (GET_MODE (x), XEXP (to, 0)),
                          XEXP (to, 1));

Time passes, and by version 1.21 in May 1988 (in fact before the
earliest ChangeLogs were introduced for version 1.17 in January 1988),
this issue had been identified, and a helpful reminder placed at the
top of the code:

/* It is not safe to use ordinary gen_lowpart in combine.
   Use gen_lowpart_for_combine instead.  See comments there.  */
#define gen_lowpart dont_use_gen_lowpart_you_dummy

However, to save a little effort, and avoid checking the return value
for validity at all of the callers of gen_lowpart_for_combine, RMS
invented the "(clobber (const_int 0))" idiom, which was returned
instead of zero.  The comment above gen_lowpart_for_combine was
modified to state:

/* If for some reason this cannot do its job, an rtx
   (clobber (const_int 0)) is returned.
   An insn containing that will not be recognized.  */

Aside: Around this time Bjarne Stroustrup was also trying to avoid
testing function return values for validitity, so introduced exceptions
into C++.

Thirty five years later this decision (short-cut) still haunts combine.
Using "(clobber (const_int 0))", like error_mark_node, that can appear
anywhere in a RTX expression makes it hard to impose strict typing (to
catch things like a CLOBBER of a CLOBBER) and as shown by bugzilla's
PR rtl-optimization/112380 these RTX occasionally escape from combine
to cause problems in generic RTL handling functions.

This patch doesn't eliminate combine.cc's such of (clobber (const_int 0)),
we still allocate memory to indicate exceptional conditions, and require
the garbage collector to clean things up, but testing the values returned
from functions for errors/exceptions is good software engineering, and
hopefully a step in the right direction.  I'd hoped allowing combine to
continue exploring alternate simplifications would also lead to better
code generation, but I've not been able to find any examples on x86_64.

This patch has been retested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target_board=unix{-m32}
with no new failures.  Ok for mainline?


2023-11-12  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        PR rtl-optimization/112380
        * combine.cc (find_split_point): Check if gen_lowpart returned
        a CLOBBER.
        (subst): Check if combine_simplify_rtx returned a CLOBBER.
        (simplify_set): Check if force_to_mode returned a CLOBBER.
        Check if gen_lowpart returned a CLOBBER.
        (expand_field_assignment): Likewise.
        (make_extraction): Check if force_to_mode returned a CLOBBER.
        (force_int_to_mode): Likewise.
        (simplify_and_const_int_1): Check if VAROP is a CLOBBER, after
        call to force_to_mode (and before).
        (simplify_comparison): Check if force_to_mode returned a CLOBBER.
        Check if gen_lowpart returned a CLOBBER.

gcc/testsuite/ChangeLog
        PR rtl-optimization/112380
        * gcc.dg/pr112380.c: New test case.


Thanks in advance,
Roger
--

Re: [PING] PR112380: Defend against CLOBBERs in RTX expressions in combine.cc

[Jeff Law]

On 12/10/23 07:56, Roger Sayle wrote:
> 
> I'd like to ping my patch for PR rtl-optimization/112380.
> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/636203.html
Sorry, I'd hoped Segher would chime in.  The first simple patch & 
testcase are OK and can go in immediately.  It'll take more time on the 
larger patch to work through it.

jeff