On Fri, Jun 06, 2025 at 09:54:53PM +0100, Stafford Horne wrote:
> On Fri, Jun 06, 2025 at 11:31:00PM +0300, Dimitar Dimitrov wrote:
> > On Fri, Jun 06, 2025 at 06:12:13PM +0100, Stafford Horne wrote:
> > > On Fri, Jun 06, 2025 at 04:41:21PM +0100, Stafford Horne wrote:
> > > > On Fri, Jun 06, 2025 at 04:20:10PM +0100, Richard Sandiford wrote:
> > > > > Stafford Horne <sho...@gmail.com> writes:
> > > > > > Hello,
> > > > > >
> > > > > > This seems to be causing a build regression on the or1k port.
> > > > > >
> > > > > > I have not quite figured it out yet but I have bisected to this 
> > > > > > commit.
> > > > > >
> > > > > > The failure is as below, this seems to be caused by the cstoresi4 
> > > > > > instruction produced by
> > > > > > or1k.md.  So I think its likely something we are doing funny in the 
> > > > > > port.
> > > > > >
> > > > > > Thanks to Jeff for pointing this out.  If you have any idea let me 
> > > > > > know.
> > > > > >
> > > > > > Log:
> > > > > >
> > > > > >     during RTL pass: ce1
> > > > > >     dump file: libgcc2.c.286r.ce1
> > > > > >     /home/shorne/work/gnu-toolchain/gcc/libgcc/libgcc2.c: In 
> > > > > > function '__muldi3':
> > > > > >     /home/shorne/work/gnu-toolchain/gcc/libgcc/libgcc2.c:538:1: 
> > > > > > internal compiler error: in emit_move_multi_word, at expr.cc:4492
> > > > > >       538 | }
> > > > > >       | ^
> > > > > >     0x1b094df internal_error(char const*, ...)
> > > > > >         
> > > > > > /home/shorne/work/gnu-toolchain/gcc/gcc/diagnostic-global-context.cc:517
> > > > > >     0x6459c1 fancy_abort(char const*, int, char const*)
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/diagnostic.cc:1815
> > > > > >     0x473a2e emit_move_multi_word
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/expr.cc:4492
> > > > > >     0x93fd7d emit_move_insn(rtx_def*, rtx_def*)
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/expr.cc:4746
> > > > > >     0x9175f1 copy_to_reg(rtx_def*)
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/explow.cc:630
> > > > > >     0xd18977 gen_lowpart_general(machine_mode, rtx_def*)
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/rtlhooks.cc:56
> > > > > >     0x9414c7 convert_mode_scalar
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/expr.cc:818
> > > > > >     0x9421a1 convert_modes(machine_mode, machine_mode, rtx_def*, 
> > > > > > int)
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/expr.cc:961
> > > > > >     0xc0afa0 prepare_operand(insn_code, rtx_def*, int, 
> > > > > > machine_mode, machine_mode, int)
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/optabs.cc:4637
> > > > > >     0xc0b33a prepare_cmp_insn
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/optabs.cc:4538
> > > > > >     0xc0f0dd emit_conditional_move(rtx_def*, rtx_comparison, 
> > > > > > rtx_def*, rtx_def*, machine_mode, int)
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/optabs.cc:5098
> > > > > >     0x192cc7b noce_emit_cmove
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:1777
> > > > > >     0x193365b try_emit_cmove_seq
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:3410
> > > > > >     0x193365b noce_convert_multiple_sets_1
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:3705
> > > > > >     0x1933c71 noce_convert_multiple_sets
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:3496
> > > > > >     0x1938687 noce_process_if_block
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:4045
> > > > > >     0x1938687 noce_find_if_block
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:4726
> > > > > >     0x1938687 find_if_header
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:4931
> > > > > >     0x1938687 if_convert
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:6078
> > > > > >     0x1938d01 rest_of_handle_if_conversion
> > > > > >         /home/shorne/work/gnu-toolchain/gcc/gcc/ifcvt.cc:6143
> > > > > >
> > > > > > -Stafford
> > > > > 
> > > > > Can you attach the .i?
> > > > 
> > > > Sure please find attached, I can reproduce with the or1k compiler using:
> > > > 
> > > >   $HOME/work/gnu-toolchain/build-gcc/./gcc/cc1 -O2  __muldi3.i
> > > > 
> > > > I am still debugging, but I notice when reverting the patch the 
> > > > compiler never
> > > > goes to emit_move_multi_word().  I am trying to work backwards as to 
> > > > where it goes
> > > > different, it's just taking a bit of time as I haven't debugged GCC 
> > > > ICE's for a
> > > > while.
> > > 
> > > I made some progress on this.  In openRISC we generate cstoresi4 with 
> > > special
> > > register SR_F (condition flag) bit register.
> > > 
> > > This is represented as (reg:BI 34 ?sr_f).
> > > 
> > > The RTL produced for cstoresi4 becomes something like:
> > > 
> > >      // The compare
> > >      (insn 49 48 50 (set (reg:BI 34 ?sr_f)
> > >         (leu:BI (reg/v:SI 68 [ __x2D.6783 ])
> > >             (reg/v:SI 69 [ __x1D.6782 ]))) 
> > > "/home/shorne/work/gnu-toolchain/gcc/libgcc/libgcc2.c":532:22 -1
> > >      (nil)
> > >      // The conditional branch
> > >      (jump_insn 50 49 0 (set (pc)
> > >        (if_then_else (ne (reg:BI 34 ?sr_f)
> > >                (const_int 0 [0]))
> > >            (label_ref 0)
> > >            (pc))) 
> > > "/home/shorne/work/gnu-toolchain/gcc/libgcc/libgcc2.c":532:22 -1
> > >     (int_list:REG_BR_PROB 536870916 (nil)))
> > > 
> > > During ce1 the try_emit_cmove_seq would try to convert (reg:BI 34 ?sr_f) 
> > > to SI.
> > > Before this patch it was done in gen_lowpart_general by
> > > simplify_context::simplify_gen_subreg.
> > > 
> > > The simplity to subreg operation would convert:
> > >   (reg:BI 34 ?sr_f)
> > > 
> > > To:
> > >   (subreg:SI (reg:BI 34 ?sr_f) 0)
> > > 
> > > But now the call to validate_subreg is returning false and 
> > > simplify_gen_subreg
> > > returns NULL_RTX, this causes gen_lowpart_general to try to do 
> > > copy_to_reg,
> > > which fails.
> > > 
> > > I need to step away for a bit, but I would next look into why 
> > > validate_subreg
> > > is now failing to allow (reg:BI 34 ?sr_f) to be subreg'd.
> > 
> > I think that the REG_CAN_CHANGE_MODE_P call in validate_subreg is
> > rejecting the subreg, because or1k_can_change_mode_class rejects the
> > mode change of sr_f from BI to SI.
> > 
> Hi Dimitar,
> 
> That's right, I noticed the same thing and I am testing out a patch to
> or1k_can_change_mode_class now, it fixes the ICE.  But I'll have to spend a 
> bit
> more time testing.

Hi All,

The patch I have now is as below and it looks to work ok:

    --- a/gcc/config/or1k/or1k.cc
    +++ b/gcc/config/or1k/or1k.cc
    @@ -1408,8 +1408,9 @@ static bool
     or1k_can_change_mode_class (machine_mode from, machine_mode to,
                                reg_class_t rclass)
     {
    +  /* Allow cnoverting special flags to SI mode subregs.  */
       if (rclass == FLAG_REGS)
    -    return from == to;
    +    return from == to || (from == BImode && to == SImode);
       return true;
     }

I just wanted to make some notes here as originally I and Jeff thought it was
the cstore* pattern causing the issue.  But it was the cbranch* pattern.  If I
have anything wrong please let me know.

I haven't touched this stuff for a while, but it's slowly coming back.

The RTL that was having problems was:

   ;; expand cbranchsi4
    (insn 20 19 21 2 (set (reg:BI 34 ?sr_f)
            (leu:BI (reg/v:SI 68 [ __x2 ])
                (reg/v:SI 69 [ __x1 ]))) 
"/home/shorne/work/gnu-toolchain/gcc/libgcc/libgcc2.c":535:20 -1
         (nil))
    (jump_insn 21 20 22 2 (set (pc)             ;;
            (if_then_else (ne (reg:BI 34 ?sr_f) ;; <-- pairs with "*cbranch"
                    (const_int 0 [0]))          ;;
                (label_ref 25)                  ;;
                (pc))) 
"/home/shorne/work/gnu-toolchain/gcc/libgcc/libgcc2.c":535:20 -1
         (int_list:REG_BR_PROB 536870916 (nil))
     -> 25)

In ce1 (if-conversion) try_emit_cmove_seq tries to convert (ne (reg:BI 34 ?sr_f)
(const_int 0 [0])) to cmove.

The conversion results in:

    (insn 70 0 71 (set (reg:SI 110)
            (ashift:SI (subreg:SI (reg:BI 34 ?sr_f) 0)
                (const_int 31 [0x1f]))) -1
         (nil))

After this after this another sequence is also generated, cost analysis is done
and both sequences are discarded.

My patch restores the original behavior which allows the subreg conversion to be
valid again.

## Note on cstoresi4 and PUT_MODE.

There was some concern raised by Jeff about the use of PUT_MODE in cstoresi4.
This is here to convert a 'ne' to 'ne:SI', for example, allowing the generated
RTL to match with the "*scc" insn pattern later.

It seems to be working OK so I will leave this as is for now.  But if you have
any suggestions let me know.

;; cstoresi4 from or1k.md

    (define_expand "cstoresi4"
      [(set (match_operand:SI 0 "register_operand" "")
            (if_then_else:SI
              (match_operator 1 "comparison_operator"
                [(match_operand:SI 2 "reg_or_0_operand" "")
                 (match_operand:SI 3 "reg_or_s16_operand" "")])
              (match_dup 0)
              (const_int 0)))]
      ""
    {
      or1k_expand_compare (operands + 1);
      PUT_MODE (operands[1], SImode);
      emit_insn (gen_rtx_SET (operands[0], operands[1]));
      DONE;
    })

;; Example expanded cstoresi4
(insn 9 8 10 2 (set (reg:BI 34 ?sr_f)
        (ne:BI (reg:SI 48 [ _13 ])
            (reg:SI 55))) 
"/home/shorne/work/gnu-toolchain/gcc/libgcc/libgcc2.c":68:52 -1
     (nil))
(insn 10 9 11 2 (set (reg:SI 54 [ _2+-3 ])
        (ne:SI (reg:BI 34 ?sr_f)  <-- PUT_MODE -> pair with "*scc"
            (const_int 0 [0]))) 
"/home/shorne/work/gnu-toolchain/gcc/libgcc/libgcc2.c":68:52 -1
     (nil))

-Stafford

Reply via email to