Re: [PATCH] ifcvt: Handle INSV patterns properly when a temp reg is needed

[Andreas Krebbel <krebbel1 at de dot ibm dot com>]

Hi,

> This problem is also responsible for PR rtl-optimization/14261, and
> the solution you propose is almost identical to a fix that I suggested
> here http://gcc.gnu.org/ml/gcc-patches/2006-02/msg02039.html
Right, I suppose I should track the mailing list more carefully.

> Unfortunately, my patch was incorrect due to the dependency on
> BITS_BIG_ENDIAN in store_bit_field as identified by James Wilson's
> review http://gcc.gnu.org/ml/gcc-patches/2006-02/msg02073.html
> Jim also suggested a test system to identify this problem using a
> cross compiler to ia64-hpux.
> 
> At that point I gave up :-(.  If you're interested in tackling
> the BIS_BIG_ENDIAN issue, I'm happy to help by bootstrapping and
> regression testing candidate patches on ia64-unknown-linux-gnu
> and ia64-hp-hpux11.2x if you don't have access to them.
> 
> I'd also prefer to hold off on approving your "paradoxical SUBREGs
> in store_bit_field" patch, until we've confirmed that that change
> is needed. 
My problem with store_bit_field was that the function was left at

  /* No action is needed if the target is a register and if the field
     lies completely outside that register.  This can occur if the source
     code contains an out-of-bounds access to a small array.  */
  if (REG_P (op0) && bitnum >= GET_MODE_BITSIZE (GET_MODE (op0)))
    return value;

Due to the miscalculated BITNUM. But maybe there is an issue with 
BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN as well. On s390 both are set.

> For 4.1, we may decide that its safer just to prevent
> any ZERO_EXTRACTs from reaching this part of ifcvt.
Like so:

Index: gcc/ifcvt.c
===================================================================
*** gcc/ifcvt.c.orig    2006-03-31 17:03:27.000000000 +0200
--- gcc/ifcvt.c 2006-03-31 17:04:04.000000000 +0200
*************** noce_process_if_block (struct ce_if_bloc
*** 2231,2236 ****
--- 2231,2238 ----
      {
        if (no_new_pseudos || GET_MODE (x) == BLKmode)
        return FALSE;
+       if (GET_CODE (x) == ZERO_EXTRACT)
+       return FALSE;
        x = gen_reg_rtx (GET_MODE (GET_CODE (x) == STRICT_LOW_PART
                                 ? XEXP (x, 0) : x));
      }

Bye,

-Andreas-