[Bug rtl-optimization/106101] New: [12/13 Regression] ICE in reg_bitfield_target_p

[Bug rtl-optimization/106101] New: [12/13 Regression] ICE in reg_bitfield_target_p

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

            Bug ID: 106101
           Summary: [12/13 Regression] ICE in reg_bitfield_target_p
           Product: gcc
           Version: 12.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

Testcase reduced from opie

> ./cc1 -quiet /tmp/y.tab.i -O -I include -w
during RTL pass: combine
/tmp/y.tab.i: In function ‘yyparse’:
/tmp/y.tab.i:59:1: internal compiler error: Segmentation fault
   59 | }
      | ^
0x14010ac crash_signal
        /space/rguenther/src/gcc/gcc/toplev.cc:322
0x21932ef reg_bitfield_target_p
        /space/rguenther/src/gcc/gcc/combine.cc:14142
0x219473f distribute_notes
        /space/rguenther/src/gcc/gcc/combine.cc:14653
0x2173f8f try_combine
        /space/rguenther/src/gcc/gcc/combine.cc:4485
0x216a2f6 combine_instructions
        /space/rguenther/src/gcc/gcc/combine.cc:1268
0x2195541 rest_of_handle_combine
        /space/rguenther/src/gcc/gcc/combine.cc:14976
0x2195602 execute
        /space/rguenther/src/gcc/gcc/combine.cc:15021
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

[Bug rtl-optimization/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-valid-code
   Target Milestone|---                         |12.2
             Target|                            |s390x-*-*

[Bug rtl-optimization/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 53207
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53207&action=edit
reduced testcase

[Bug rtl-optimization/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |segher at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-06-27
           Keywords|                            |needs-bisection
     Ever confirmed|0                           |1

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Program received signal SIGSEGV, Segmentation fault.
0x00000000021932ef in reg_bitfield_target_p (x=0x7ffff6a4ee10,
body=0x7ffff6a4e450) at /space/rguenther/src/gcc/gcc/combine.cc:14142
14142         if (GET_CODE (target) == SUBREG)
(gdb) p target
$1 = (rtx) 0xafafaf0100000047

one frame up:

(gdb) p debug_rtx (place)
(insn 24 52 25 4 (set (strict_low_part (reg/v:SI 71 [ yyval ]))
        (mem/f:SI (plus:DI (reg:DI 85)
                (const_int 4 [0x4])) [2 *_3+4 S4 A32])) "/tmp/y.tab.i":41:23
1485 {movstrictsi}
     (nil))

static int
reg_bitfield_target_p (rtx x, rtx body)
{
...
      else if (GET_CODE (dest) == STRICT_LOW_PART)
        target = SUBREG_REG (XEXP (dest, 0));

but the strict_low_part doesn't contain a SUBREG ...!?  Adding a check to
that avail would of course fix this.

It's all ancient code though...

[Bug rtl-optimization/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #3 from Segher Boessenkool <segher at gcc dot gnu.org> ---
STRICT_LOW_PART is required to contain a SUBREG though.

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|rtl-optimization            |target
           Priority|P3                          |P2

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #3)
> STRICT_LOW_PART is required to contain a SUBREG though.

So

Trying 23 -> 24:
   23: r77:DI=[r85:DI]
      REG_DEAD r85:DI
   24: strict_low_part(r71:SI)=r77:DI#4
      REG_DEAD r77:DI
Successfully matched this instruction:
(set (strict_low_part (reg/v:SI 71 [ yyval ]))
    (mem/f:SI (plus:DI (reg:DI 85)
            (const_int 4 [0x4])) [2 *_3+4 S4 A32]))
allowing combination of insns 23 and 24

just exposes the issue that RTL expansion produces:

;; MEM[(char * *)&yyval] = _4;

(insn 23 22 24 (set (reg/f:DI 77)
        (mem/f:DI (reg/f:DI 62 [ _3 ]) [2 *_3+0 S8 A64])) "/tmp/y.tab.i":41:23
-1
     (nil))

(insn 24 23 0 (set (strict_low_part (reg/v:SI 71 [ yyval ]))
        (subreg:SI (reg/f:DI 77) 4)) "/tmp/y.tab.i":41:23 -1
     (nil))

which is emitted via

#3  0x0000000001fe5354 in gen_movstrictsi (operand0=0x7ffff6a49d20, 
    operand1=0x7ffff6a4e438) at insn-emit.cc:11776
#4  0x00000000019163e5 in s390_expand_insv (dest=0x7ffff6a4e3f0, 
    op1=0x7ffff68c8690, op2=0x7ffff68c8690, src=0x7ffff6a4e3d8)
    at /space/rguenther/src/gcc/gcc/config/s390/s390.cc:6530
#5  0x00000000020373a8 in gen_insv (operand0=0x7ffff6a4e3f0, 
    operand1=0x7ffff68c8690, operand2=0x7ffff68c8690, operand3=0x7ffff6a4e3d8)
    at /space/rguenther/src/gcc/gcc/config/s390/s390.md:4217
...
#8  0x0000000001242f07 in maybe_expand_insn (icode=CODE_FOR_insv, nops=4, 
    ops=0x7fffffffc500) at /space/rguenther/src/gcc/gcc/optabs.cc:7998
#9  0x0000000000e43d46 in store_bit_field_using_insv (insv=0x7fffffffc7a0, 
    op0=0x7ffff6a49d20, op0_mode=SImode, bitsize=32, bitnum=32, 

;
; movstrictsi instruction pattern(s).
;

(define_insn "movstrictsi"
  [(set (strict_low_part (match_operand:SI 0 "register_operand" "+d,d,d,d"))
                         (match_operand:SI 1 "general_operand" "d,R,T,t"))]
  "TARGET_ZARCH"
  "@
   lr\t%0,%1
   l\t%0,%1
   ly\t%0,%1
   ear\t%0,%1"
  [(set_attr "op_type" "RR,RX,RXY,RRE")
   (set_attr "type" "lr,load,load,*")
   (set_attr "cpu_facility" "*,*,longdisp,*")
   (set_attr "z10prop" "z10_fr_E1,z10_fwd_A3,z10_fwd_A3,z10_super_E1")])

all of which is ancient code ...

target issue after all.

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Thanks for tracking this down!

Interesting it survived so long.  We could use some RTL checking on this :-)

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #6 from Segher Boessenkool <segher at gcc dot gnu.org> ---
It looks like quite a few more backends use strict_low_part on random RTL,
which
is completely meaningless :-(

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #5)
> Thanks for tracking this down!
> 
> Interesting it survived so long.  We could use some RTL checking on this :-)

We have almost no "RTL IL checking", but surely having that would be nice
(like we have verify_gimple_*).

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #8 from Segher Boessenkool <segher at gcc dot gnu.org> ---
There is structural RTL checking in rtl.h (see RTL_CHECK{1,2,C1,C2,C3} and the
various ELT and INT accessors).  This would be easier to use here if we used
some STRICT_LOW_PART_P everywhere :-)

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13 Regression] ICE in   |[12/13 Regression] ICE in
                   |reg_bitfield_target_p       |reg_bitfield_target_p since
                   |                            |r12-4428-g147ed0184f403b
                 CC|                            |marxin at gcc dot gnu.org
           Keywords|needs-bisection             |

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-4428-g147ed0184f403b.

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #10 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
We generate the movstrict target operand with gen_lowpart. If the operand for
gen_lowpart is already a paradoxical subreg the two subregs cancel each other
out and we end up with a plain reg. I'm testing the following patch right now.
It falls back to a normal move in that case and fixes the testcase:

diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
index 5aaf76a9490..d90ec1a6de1 100644
--- a/gcc/config/s390/s390.cc
+++ b/gcc/config/s390/s390.cc
@@ -6523,6 +6523,14 @@ s390_expand_insv (rtx dest, rtx op1, rtx op2, rtx src)
          rtx low_dest = gen_lowpart (smode, dest);
          rtx low_src = gen_lowpart (smode, src);

+         /* In case two subregs cancelled each other out, do a normal
+            move.  */
+         if (!SUBREG_P (low_dest))
+           {
+             emit_move_insn (low_dest, low_src);
+             return true;
+           }
+
          switch (smode)
            {
            case E_QImode: emit_insn (gen_movstrictqi (low_dest, low_src));
return true;

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #11 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
I've tried to change our movstrict backend patterns to use a predicate on the
dest operand which enforces a subreg. However, since reload strips the subreg
away when assigning hard regs we end up with a STRICT_LOW_PART of a reg again.
At least after reload something like this should be acceptable - right?

298r.ira:
(insn 8 16 17 3 (set (strict_low_part (subreg:SI (reg/v:DI 64 [ e ]) 4))
        (const_int 0 [0])) "t.cc":37:17 1485 {movstrictsi}
     (nil))

299r.reload:
(insn 8 16 17 3 (set (strict_low_part (reg:SI 11 %r11 [orig:64 e+4 ] [64]))
        (mem/u/c:SI (symbol_ref/u:DI ("*.LC0") [flags 0x2]) [0  S4 A32]))
"t.cc":37:17 1485 {movstrictsi}
     (nil))

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #18 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #17)
...
> Yes, but that says the high 48 bits of the hardware reg are untouched, which
> is not true (only the high 16 of the low 32 are guaranteed unmodified).

Right, if the original register mode does not match the mode of the full
hardreg, we continue to need that mode as the upper bound. So with the subreg
folding in reload we appear to loose information we need to interpret the
STRICT_LOW_PART correctly.

I'm testing the following patch in combination with my other fix now:

diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
index 4ddbe477d92..9c125a9ce38 100644
--- a/gcc/lra-spills.cc
+++ b/gcc/lra-spills.cc
@@ -855,6 +855,7 @@ lra_final_code_change (void)

          for (i = id->insn_static_data->n_operands - 1; i >= 0; i--)
            if ((DEBUG_INSN_P (insn) || ! static_id->operand[i].is_operator)
+               && ! static_id->operand[i].strict_low
                && alter_subregs (id->operand_loc[i], ! DEBUG_INSN_P (insn)))
              {
                lra_update_dup (id, i);

With that change the SUBREG folding from comment #11 happens later in final
(cleanup_subreg_operands). I'm not sure whether we would have to prevent it
there as well?!

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #19 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Andreas Krebbel from comment #18)
> (In reply to Segher Boessenkool from comment #17)
> ...
> > Yes, but that says the high 48 bits of the hardware reg are untouched, which
> > is not true (only the high 16 of the low 32 are guaranteed unmodified).
> 
> Right, if the original register mode does not match the mode of the full
> hardreg, we continue to need that mode as the upper bound. So with the
> subreg folding in reload we appear to loose information we need to interpret
> the STRICT_LOW_PART correctly.

Exactly.  This is why strict_low_part of anything else than a subreg is
ill-defined.

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #20 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andreas Krebbel <krebbel@gcc.gnu.org>:

https://gcc.gnu.org/g:585a21bab3ec688c2039bff2922cc372d8558283

commit r13-2201-g585a21bab3ec688c2039bff2922cc372d8558283
Author: Andreas Krebbel <krebbel@linux.ibm.com>
Date:   Fri Jul 29 09:55:54 2022 +0200

    PR 106101: IBM zSystems: Fix strict_low_part problem

    This avoids generating illegal (strict_low_part (reg ...)) RTXs. This
    required two changes:

    1. Do not use gen_lowpart to generate the inner expression of a
    STRICT_LOW_PART.  gen_lowpart might fold the SUBREG either because
    there is already a paradoxical subreg or because it can directly be
    applied to the register. A new wrapper function makes sure that we
    always end up having an actual SUBREG.

    2. Change the movstrict patterns to enforce a SUBREG as inner operand
    of the STRICT_LOW_PARTs.  The new predicate introduced for the
    destination operand requires a SUBREG expression with a
    register_operand as inner operand.  However, since reload strips away
    the majority of the SUBREGs we have to accept single registers as well
    once we reach reload.

    Bootstrapped and regression tested on IBM zSystems 64 bit.

    gcc/ChangeLog:

            PR target/106101
            * config/s390/predicates.md (subreg_register_operand): New
            predicate.
            * config/s390/s390-protos.h (s390_gen_lowpart_subreg): New
            function prototype.
            * config/s390/s390.cc (s390_gen_lowpart_subreg): New function.
            (s390_expand_insv): Use s390_gen_lowpart_subreg instead of
            gen_lowpart.
            * config/s390/s390.md ("*get_tp_64", "*zero_extendhisi2_31")
            ("*zero_extendqisi2_31", "*zero_extendqihi2_31"): Likewise.
            ("movstrictqi", "movstricthi", "movstrictsi"): Use the
            subreg_register_operand predicate instead of register_operand.

    gcc/testsuite/ChangeLog:

            PR target/106101
            * gcc.c-torture/compile/pr106101.c: New test.

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #21 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
I have committed a patch now which accepts only SUBREGs before reload and then
also REGs to deal with how LRA operates right now.

I've continued a bit with the patch from Comment 18. It bootstraps on s390x and
x86-64. On s390x also the testsuite is clean. However, I see a few failures in
the arch specific tests on x86-64. The cases I looked at so far are the result
of several peepholes and splitters not being triggered anymore. I've fixed most
of them I think but there are also cases where I'm not sure what to do exactly.

In case of a matching constraint between a strict_low_part operand and a normal
operand. Reload now (with the patch from Comment 18) would remove the subreg on
the operand with the matching constraint and would leave it in for the
strict_low_part operand.

(insn 9 8 16 2 (parallel [
            (set (strict_low_part (subreg:QI (reg/v:SI 0 ax [orig:86 a ] [86])
0))
                (and:QI (reg:QI 0 ax [orig:86 a ] [86])
                    (reg:SI 4 si [92])))
            (clobber (reg:CC 17 flags))
        ]) "/home/andreas/gcc/gcc/testsuite/gcc.target/i386/pr91188-1a.c":20:10
553 {*andqi_1_slp}
     (nil))

I think this should be addressed separately. Once we solved it I will adjust
the s390x backend again if necessary.

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #22 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
The longer a have been looking at these STRICT_LOW_PART issue the more I think
that STRICT_LOW_PART is an awful way to express what we need:

- the information needed to understand what it is doing is distributed across 3
RTXs (strict_low_part (subreg:mode1 (reg:mode2 xx) OFS))
- the big problems arise since the involved RTXs are separately optimized and
we might end up with partial information without a clear definition of how to
deal with that
- actually it is really hard to handle the RTXs as one unit. Recursively
walking RTXs needs to record whether we are in a STRICT_LOW_PART or not.


I think it might make sense to explore other ways to express this:

1. SUBREG flag - Looks easy, but it would be hard to catch all places which
should care about that flag.

2. Introduce a new RTX code which has a mode and an offset attached but does
not require an additional SUBREG anymore.

3. Since a STRICT_LOW_PART is essentially a bit insertion operation we could
express it always with a ZERO_EXTRACT target operand and get rid of
STRICT_LOW_PART entirely. A ZERO_EXTRACT would be somewhat more cumbersome to
deal with, since it would always require to check the bit width and offset for
all the cases which just use mode boundaries. But at least most passes know how
to deal with them already.

[Bug target/106101] [12/13 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #23 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Andreas Krebbel from comment #22)
> The longer a have been looking at these STRICT_LOW_PART issue the more I
> think that STRICT_LOW_PART is an awful way to express what we need:
> 
> - the information needed to understand what it is doing is distributed
> across 3 RTXs (strict_low_part (subreg:mode1 (reg:mode2 xx) OFS))
> - the big problems arise since the involved RTXs are separately optimized
> and we might end up with partial information without a clear definition of
> how to deal with that
> - actually it is really hard to handle the RTXs as one unit. Recursively
> walking RTXs needs to record whether we are in a STRICT_LOW_PART or not.
> 
> 
> I think it might make sense to explore other ways to express this:
> 
> 1. SUBREG flag - Looks easy, but it would be hard to catch all places which
> should care about that flag.
> 
> 2. Introduce a new RTX code which has a mode and an offset attached but does
> not require an additional SUBREG anymore.
> 
> 3. Since a STRICT_LOW_PART is essentially a bit insertion operation we could
> express it always with a ZERO_EXTRACT target operand and get rid of
> STRICT_LOW_PART entirely. A ZERO_EXTRACT would be somewhat more cumbersome
> to deal with, since it would always require to check the bit width and
> offset for all the cases which just use mode boundaries. But at least most
> passes know how to deal with them already.

4. With existing simple RTL:

(set (reg:DI x) (ior:DI (and:DI (reg:DI x) (const_int -4294967296))
                        (zero_extend:DI (reg:SI y))))

(ZERO_EXTRACT is never useful IMNSHO: it just makes the easy cases slightly
easier to write, and causes a lot of useless work everywhere else).

[Bug target/106101] [12 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13 Regression] ICE in   |[12 Regression] ICE in
                   |reg_bitfield_target_p since |reg_bitfield_target_p since
                   |r12-4428-g147ed0184f403b    |r12-4428-g147ed0184f403b
   Last reconfirmed|2022-06-27 00:00:00         |2023-1-17

--- Comment #24 from Richard Biener <rguenth at gcc dot gnu.org> ---
Re-confirmed on the branch (building opie), but that (the build of opie) is
fixed on trunk.

[Bug target/106101] [12 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #25 from Richard Biener <rguenth at gcc dot gnu.org> ---
Andreas - can you backport this please?

[Bug target/106101] [12 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #26 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Andreas Krebbel
<krebbel@gcc.gnu.org>:

https://gcc.gnu.org/g:e1357577e6e39430869e294f94c2c547717b960f

commit r12-9058-ge1357577e6e39430869e294f94c2c547717b960f
Author: Andreas Krebbel <krebbel@linux.ibm.com>
Date:   Mon Jan 23 08:56:05 2023 +0100

    PR 106101: IBM zSystems: Fix strict_low_part problem

    This avoids generating illegal (strict_low_part (reg ...)) RTXs. This
    required two changes:

    1. Do not use gen_lowpart to generate the inner expression of a
    STRICT_LOW_PART.  gen_lowpart might fold the SUBREG either because
    there is already a paradoxical subreg or because it can directly be
    applied to the register. A new wrapper function makes sure that we
    always end up having an actual SUBREG.

    2. Change the movstrict patterns to enforce a SUBREG as inner operand
    of the STRICT_LOW_PARTs.  The new predicate introduced for the
    destination operand requires a SUBREG expression with a
    register_operand as inner operand.  However, since reload strips away
    the majority of the SUBREGs we have to accept single registers as well
    once we reach reload.

    Bootstrapped and regression tested on IBM zSystems 64 bit.

    gcc/ChangeLog:

            PR target/106101
            * config/s390/predicates.md (subreg_register_operand): New
            predicate.
            * config/s390/s390-protos.h (s390_gen_lowpart_subreg): New
            function prototype.
            * config/s390/s390.cc (s390_gen_lowpart_subreg): New function.
            (s390_expand_insv): Use s390_gen_lowpart_subreg instead of
            gen_lowpart.
            * config/s390/s390.md ("*get_tp_64", "*zero_extendhisi2_31")
            ("*zero_extendqisi2_31", "*zero_extendqihi2_31"): Likewise.
            ("movstrictqi", "movstricthi", "movstrictsi"): Use the
            subreg_register_operand predicate instead of register_operand.

    gcc/testsuite/ChangeLog:

            PR target/106101
            * gcc.c-torture/compile/pr106101.c: New test.

    (cherry picked from commit 585a21bab3ec688c2039bff2922cc372d8558283)

[Bug target/106101] [12 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #27 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug target/106101] [12 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

Florian Weimer <fw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fw at gcc dot gnu.org

--- Comment #28 from Florian Weimer <fw at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #1)
> Created attachment 53207 [details]
> reduced testcase

Do you happen to have the unreduced test case still?

    int *yyvsp = 0;

followed by:

  if (strncmp( yyvsp[0], "~", 1) == 0) {

and

   free(yyvsp[0]);

looks rather bogus.

[Bug target/106101] [12 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #29 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Florian Weimer from comment #28)
> (In reply to Richard Biener from comment #1)
> > Created attachment 53207 [details]
> > reduced testcase
> 
> Do you happen to have the unreduced test case still?
> 
>     int *yyvsp = 0;
> 
> followed by:
> 
>   if (strncmp( yyvsp[0], "~", 1) == 0) {
> 
> and
> 
>    free(yyvsp[0]);
> 
> looks rather bogus.

You can make `yyvsp` and argument if you are worried about the null pointer and
you still get the crash back in 12.1.0.
That is:
```
int yyparse (int *yyvsp)
{
    int yystate = 0;
```

See https://godbolt.org/z/9YMsar1Ej .

[Bug target/106101] [12 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #30 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #29)
> (In reply to Florian Weimer from comment #28)
> > (In reply to Richard Biener from comment #1)
> > > Created attachment 53207 [details]
> > > reduced testcase
> > 
> > Do you happen to have the unreduced test case still?
> > 
> >     int *yyvsp = 0;
> > 
> > followed by:
> > 
> >   if (strncmp( yyvsp[0], "~", 1) == 0) {
> > 
> > and
> > 
> >    free(yyvsp[0]);
> > 
> > looks rather bogus.
> 
> You can make `yyvsp` and argument if you are worried about the null pointer
> and you still get the crash back in 12.1.0.
> That is:
> ```
> int yyparse (int *yyvsp)
> {
>     int yystate = 0;
> ```
> 
> See https://godbolt.org/z/9YMsar1Ej .

And you can even make it a pointer to a pointer of char to hit the same bug to
get around the even more fuzziness of freeing an int rather than a pointer:
```
int yyparse (char **yyvsp)
```
See https://godbolt.org/z/v7xKxWE3K

[Bug target/106101] [12 Regression] ICE in reg_bitfield_target_p since r12-4428-g147ed0184f403b

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106101

--- Comment #31 from Florian Weimer <fw at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #30)
> And you can even make it a pointer to a pointer of char to hit the same bug
> to get around the even more fuzziness of freeing an int rather than a
> pointer:
> ```
> int yyparse (char **yyvsp)
> ```
> See https://godbolt.org/z/v7xKxWE3K

-int yyparse (void)
+int yyparse (char **yyvsp)
 {
     int yystate = 0;
-    int *yyvsp = 0;


Nice, thank you.