On Sat, Jun 07, 2025 at 06:53:28PM +0300, Dimitar Dimitrov wrote:
> On Sat, Jun 07, 2025 at 11:38:46AM +0100, Stafford Horne wrote:
> > 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;
> >      }
> 
> Another patch I intend to merge tomorrow will tighten the checks even
> more: https://gcc.gnu.org/pipermail/gcc-patches/2025-June/685837.html
> 
> Seeing that or1k_hard_regno_mode_ok forbids SImode for class FLAG_REGS,
> the validate_subreg may once again start rejecting subregs for or1k.

OK, I tested with this patch and confirm it causes the same ICE for the or1k
port again, with a slighly differnt code path.

I tried to patch with the below and it causes another.  I think this will not
work as we should not allow FLAG_REGS directly as SI.

    --- a/gcc/config/or1k/or1k.cc
    +++ b/gcc/config/or1k/or1k.cc
    @@ -1390,9 +1390,10 @@ static bool
     or1k_hard_regno_mode_ok (unsigned int regno, machine_mode mode)
     {
       /* For OpenRISC, GENERAL_REGS can hold anything, while
    -     FLAG_REGS are really single bits within SP[SR].  */
    +     FLAG_REGS are really single bits within SP[SR].
    +     We also allow SImode for FLAG_REGS to allow subregs.  */
       if (REGNO_REG_CLASS (regno) == FLAG_REGS)
    -    return mode == BImode;
    +    return mode == BImode || mode == SImode;
       return true;
     }

With the above patch we get the failure:

    In file included from 
/home/shorne/work/gnu-toolchain/gcc/libstdc++-v3/src/c++17/floating_from_chars.cc:86:
    
/home/shorne/work/gnu-toolchain/gcc/libstdc++-v3/src/c++17/fast_float/fast_float.h:
 In function 'std::from_chars_result 
{anonymous}::fast_float::from_chars_advanced(const char*, const char*, T&, 
parse_options) [with T = double]':
    
/home/shorne/work/gnu-toolchain/gcc/libstdc++-v3/src/c++17/fast_float/fast_float.h:3040:1:
 error: insn does not satisfy its constraints:
     3040 | }
          | ^
    (insn 9056 5757 7408 339 (set (reg:SI 21 r27 [orig:3711+-3 ] [3711])
            (reg:SI 34 ?sr_f [+-3 ])) 
"/home/shorne/work/gnu-toolchain/gcc/libstdc++-v3/src/c++17/fast_float/fast_float.h":261:30
 28 {*movsi_internal}
         (nil))
    during RTL pass: reload
    
/home/shorne/work/gnu-toolchain/gcc/libstdc++-v3/src/c++17/fast_float/fast_float.h:3040:1:
 internal compiler error: in extract_constrain_insn, at recog.cc:2783
    0x19b4af9 internal_error(char const*, ...)
            
/home/shorne/work/gnu-toolchain/gcc/gcc/diagnostic-global-context.cc:517
    0x1998bf0 fancy_abort(char const*, int, char const*)
            /home/shorne/work/gnu-toolchain/gcc/gcc/diagnostic.cc:1803
    0xcfec74 _fatal_insn(char const*, rtx_def const*, char const*, int, char 
const*)
            /home/shorne/work/gnu-toolchain/gcc/gcc/rtl-error.cc:108
    0xcfeca5 _fatal_insn_not_found(rtx_def const*, char const*, int, char 
const*)
            /home/shorne/work/gnu-toolchain/gcc/gcc/rtl-error.cc:118
    0xcd2684 extract_constrain_insn(rtx_insn*)
            /home/shorne/work/gnu-toolchain/gcc/gcc/recog.cc:2783
    0xb954ea check_rtl
            /home/shorne/work/gnu-toolchain/gcc/gcc/lra.cc:2202
    0xb974cd lra(_IO_FILE*, int)
            /home/shorne/work/gnu-toolchain/gcc/gcc/lra.cc:2636
    0xb5dc6f do_reload
            /home/shorne/work/gnu-toolchain/gcc/gcc/ira.cc:5986
    0xb5dfb6 execute
            /home/shorne/work/gnu-toolchain/gcc/gcc/ira.cc:6174

The RTL generated: (reg:SI 34 ?sr_f [+-3 ]) is incorrect, and I think it's
being allowed due to my patch.

-Stafford

> > 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