[Bug c/111337] New: ICE in gimple-isel.cc for RISC-V port

[Bug c/111337] New: ICE in gimple-isel.cc for RISC-V port

[juzhe.zhong at rivai dot ai]

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

            Bug ID: 111337
           Summary: ICE in gimple-isel.cc for RISC-V port
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: juzhe.zhong at rivai dot ai
  Target Milestone: ---

This following case cause ICE:

extern unsigned char a [150];
extern unsigned char b [150];
extern unsigned char c [150];
extern unsigned char d [150];
extern unsigned char e [150];

void foo () {
  for (int i = 92; i <= 141; i += 2) {
    int tmp = (d [i] && b [i]) <= (a [i] > c [i]);
    e [i] = tmp >> b [i];
  }
}

auto.c:7:6: internal compiler error: in gimple_expand_vec_cond_expr, at
gimple-isel.cc:284
    7 | void foo () {
      |      ^~~
0x1a9e309 gimple_expand_vec_cond_expr
        ../../../../gcc/gcc/gimple-isel.cc:283
0x1a9ea56 execute
        ../../../../gcc/gcc/gimple-isel.cc:390


Bause of this piece of code:
          gcc_assert (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (op0)))
                      == MODE_VECTOR_INT);

The statement cause such assertion FAIL was produced by "vrp2":

Before auto.c.202t.vrp2:

  mask__5.33_165 = vect__3.26_157 > vect__4.31_163;
  vect_patt_67.34_168 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ... }>;
  vect_patt_68.35_169 = (vector([4,4]) int) vect_patt_67.34_168;
  mask__7.36_170 = vect_patt_68.35_169 >= vect_patt_66.22_152;
  vect_patt_69.37_173 = VEC_COND_EXPR <mask__7.36_170, { 1, ... }, { 0, ... }>;
  vect_patt_70.38_174 = (vector([4,4]) unsigned char) vect_patt_69.37_173;
  vect_patt_71.39_176 = MIN_EXPR <vect_pretmp_43.16_142, { 7, ... }>;
  vect_patt_72.40_177 = vect_patt_70.38_174 >> vect_patt_71.39_176;
  .MASK_LEN_SCATTER_STORE (vectp_e.42_181, { 0, 2, 4, ... }, 1,
vect_patt_72.40_177, { -1, ... }, _186, 0);

auto.c.202t.vrp2:

  mask__5.33_165 = vect__3.26_157 > vect__4.31_163;
  vect_patt_67.34_168 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ... }>;
  vect_patt_68.35_169 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ... }>;
  mask__7.36_170 = VEC_COND_EXPR <mask__47.20_148, mask__5.33_165, { -1, ...
}>;
  vect_patt_69.37_173 = VEC_COND_EXPR <mask__7.36_170, { 1, ... }, { 0, ... }>;
  vect_patt_70.38_174 = VEC_COND_EXPR <mask__7.36_170, { 1, ... }, { 0, ... }>;
  vect_patt_71.39_176 = MIN_EXPR <vect_pretmp_43.16_142, { 7, ... }>;
  vect_patt_72.40_177 = vect_patt_70.38_174 >> vect_patt_71.39_176;
  .MASK_LEN_SCATTER_STORE (vectp_e.42_181, { 0, 2, 4, ... }, 1,
vect_patt_72.40_177, { -1, ... }, _186, 0);

There is a odd STMT here:

mask__7.36_170 = VEC_COND_EXPR <mask__47.20_148, mask__5.33_165, { -1, ... }>;

It seems that it should be logical operations of MASK?

[Bug c/111337] ICE in gimple-isel.cc for RISC-V port

[rdapp at gcc dot gnu.org]

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

Robin Dapp <rdapp at gcc dot gnu.org> changed:

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

--- Comment #1 from Robin Dapp <rdapp at gcc dot gnu.org> ---
This is gcc.dg/pr70252.c BTW.

What happens is that, starting with
      maskdest = (vec_cond mask1 1 0) >= (vec_cond mask2 1 0)
we fold to
      maskdest = mask1 >= (vec_cond (mask2 1 0))
and then sink the ">=" into the vec_cond so we end up with
      maskdest = vec_cond (mask2 ? mask1 : 0),
i.e. a vec_cond with a mask "data mode".

In gimple-isel, when the target does not provide a vcond_mask
implementation for that (which none does) we assert that the mask mode
be MODE_VECTOR_INT.

IMHO this should not happen and we should not sink comparisons (that could be
folded to masks) into vec_cond.

I'm preparing a patch that prevents the sinking of comparisons for mask types.

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
  vect_patt_67.34_168 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ... }>;
  vect_patt_68.35_169 = (vector([4,4]) int) vect_patt_67.34_168;

->

  vect_patt_68.35_169 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ... }>;

this one looks odd - it's probably

/* Sink unary conversions to branches, but only if we do fold both 
   and the target's truth type is the same as we already have.  */
(simplify
 (convert (vec_cond:s @0 @1 @2))
 (if (VECTOR_TYPE_P (type) 
      && types_match (TREE_TYPE (@0), truth_type_for (type)))
  (vec_cond @0 (convert! @1) (convert! @2)))) 

the problem is that this happily produces a vec_cond we might not be able
to expand - having the same truth type for the data type isn't enough
if there's no optab with a matching data mode.  Same for the view_convert
case.

It's a bit convoluted to ask for target support here, can you double-check
it isn't possible to add native riscv expanders for the missing case?

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[juzhe.zhong at rivai dot ai]

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

--- Comment #3 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
(In reply to Richard Biener from comment #2)
>   vect_patt_67.34_168 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ...
> }>;
>   vect_patt_68.35_169 = (vector([4,4]) int) vect_patt_67.34_168;
> 
> ->
> 
>   vect_patt_68.35_169 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ...
> }>;
> 
> this one looks odd - it's probably
> 
> /* Sink unary conversions to branches, but only if we do fold both 
>    and the target's truth type is the same as we already have.  */
> (simplify
>  (convert (vec_cond:s @0 @1 @2))
>  (if (VECTOR_TYPE_P (type) 
>       && types_match (TREE_TYPE (@0), truth_type_for (type)))
>   (vec_cond @0 (convert! @1) (convert! @2)))) 
> 
> the problem is that this happily produces a vec_cond we might not be able
> to expand - having the same truth type for the data type isn't enough
> if there's no optab with a matching data mode.  Same for the view_convert
> case.
> 
> It's a bit convoluted to ask for target support here, can you double-check
> it isn't possible to add native riscv expanders for the missing case?

You mean support(In reply to Richard Biener from comment #2)
>   vect_patt_67.34_168 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ...
> }>;
>   vect_patt_68.35_169 = (vector([4,4]) int) vect_patt_67.34_168;
> 
> ->
> 
>   vect_patt_68.35_169 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ...
> }>;
> 
> this one looks odd - it's probably
> 
> /* Sink unary conversions to branches, but only if we do fold both 
>    and the target's truth type is the same as we already have.  */
> (simplify
>  (convert (vec_cond:s @0 @1 @2))
>  (if (VECTOR_TYPE_P (type) 
>       && types_match (TREE_TYPE (@0), truth_type_for (type)))
>   (vec_cond @0 (convert! @1) (convert! @2)))) 
> 
> the problem is that this happily produces a vec_cond we might not be able
> to expand - having the same truth type for the data type isn't enough
> if there's no optab with a matching data mode.  Same for the view_convert
> case.
> 
> It's a bit convoluted to ask for target support here, can you double-check
> it isn't possible to add native riscv expanders for the missing case?

You mean we should add "vcond" patterns back ?

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[rguenther at suse dot de]

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

--- Comment #4 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 12 Sep 2023, juzhe.zhong at rivai dot ai wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111337
> 
> --- Comment #3 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
> (In reply to Richard Biener from comment #2)
> >   vect_patt_67.34_168 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ...
> > }>;
> >   vect_patt_68.35_169 = (vector([4,4]) int) vect_patt_67.34_168;
> > 
> > ->
> > 
> >   vect_patt_68.35_169 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ...
> > }>;
> > 
> > this one looks odd - it's probably
> > 
> > /* Sink unary conversions to branches, but only if we do fold both 
> >    and the target's truth type is the same as we already have.  */
> > (simplify
> >  (convert (vec_cond:s @0 @1 @2))
> >  (if (VECTOR_TYPE_P (type) 
> >       && types_match (TREE_TYPE (@0), truth_type_for (type)))
> >   (vec_cond @0 (convert! @1) (convert! @2)))) 
> > 
> > the problem is that this happily produces a vec_cond we might not be able
> > to expand - having the same truth type for the data type isn't enough
> > if there's no optab with a matching data mode.  Same for the view_convert
> > case.
> > 
> > It's a bit convoluted to ask for target support here, can you double-check
> > it isn't possible to add native riscv expanders for the missing case?
> 
> You mean support(In reply to Richard Biener from comment #2)
> >   vect_patt_67.34_168 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ...
> > }>;
> >   vect_patt_68.35_169 = (vector([4,4]) int) vect_patt_67.34_168;
> > 
> > ->
> > 
> >   vect_patt_68.35_169 = VEC_COND_EXPR <mask__5.33_165, { 1, ... }, { 0, ...
> > }>;
> > 
> > this one looks odd - it's probably
> > 
> > /* Sink unary conversions to branches, but only if we do fold both 
> >    and the target's truth type is the same as we already have.  */
> > (simplify
> >  (convert (vec_cond:s @0 @1 @2))
> >  (if (VECTOR_TYPE_P (type) 
> >       && types_match (TREE_TYPE (@0), truth_type_for (type)))
> >   (vec_cond @0 (convert! @1) (convert! @2)))) 
> > 
> > the problem is that this happily produces a vec_cond we might not be able
> > to expand - having the same truth type for the data type isn't enough
> > if there's no optab with a matching data mode.  Same for the view_convert
> > case.
> > 
> > It's a bit convoluted to ask for target support here, can you double-check
> > it isn't possible to add native riscv expanders for the missing case?
> 
> You mean we should add "vcond" patterns back ?

No, add the missing vcond_mask one?  But maybe I'm misunderstanding the
problem.

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[juzhe.zhong at rivai dot ai]

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

--- Comment #5 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
Oh. I see.


(define_expand "@vcond_mask_<mode><mode>"
  [(match_operand:VB 0 "register_operand")
   (match_operand:VB 3 "register_operand")
   (match_operand:VB 1 "nonmemory_operand")
   (match_operand:VB 2 "register_operand")]
  "TARGET_VECTOR"
  {
   printf ("vcond_mask\n");
  })

I implemented this pattern which can address this issue....

Note that VB is VECTR_BOOL...

So, this is a more reasonable way to do. Is that right?

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[rguenther at suse dot de]

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

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 12 Sep 2023, juzhe.zhong at rivai dot ai wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111337
> 
> --- Comment #5 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
> Oh. I see.
> 
> 
> (define_expand "@vcond_mask_<mode><mode>"
>   [(match_operand:VB 0 "register_operand")
>    (match_operand:VB 3 "register_operand")
>    (match_operand:VB 1 "nonmemory_operand")
>    (match_operand:VB 2 "register_operand")]
>   "TARGET_VECTOR"
>   {
>    printf ("vcond_mask\n");
>   })
> 
> I implemented this pattern which can address this issue....
> 
> Note that VB is VECTR_BOOL...
> 
> So, this is a more reasonable way to do. Is that right?

Ah, I suppose ISEL could special-case VECTOR_BOOLEAN_P COND_EXPR
to expand to bitwise ops - it already does this, but only for
non-vector-mode masks:

  /* Lower mask typed, non-vector mode VEC_COND_EXPRs to bitwise 
operations.
     Those can end up generated by folding and at least for integer mode 
masks
     we cannot expect vcond expanders to exist.  We lower a ? b : c
     to (b & a) | (c & ~a).  */
  if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (lhs))
      && !VECTOR_MODE_P (mode))
    {
      gcc_assert (types_compatible_p (TREE_TYPE (op0), TREE_TYPE (op1)));
      gimple_seq stmts = NULL;
      tree type = TREE_TYPE (lhs);
      location_t loc = gimple_location (stmt);
      tree tem0 = gimple_build (&stmts, loc, BIT_AND_EXPR, type, op1, 
op0);
      tree tem1 = gimple_build (&stmts, loc, BIT_NOT_EXPR, type, op0);
      tree tem2 = gimple_build (&stmts, loc, BIT_AND_EXPR, type, op2, 
tem1);
      tree tem3 = gimple_build (&stmts, loc, BIT_IOR_EXPR, type, tem0, 
tem2);
      gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
      return gimple_build_assign (lhs, tem3);
    }

that could be a viable expansion strathegy when the rest fails,
but as you can see we need four stmts for this.  If you can
think of a smarter, maybe even single-instruction, way for riscv
then yes, handling the above pattern looks good.

I wonder whether SVE/GCN have those.

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[juzhe.zhong at rivai dot ai]

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

--- Comment #7 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
(In reply to rguenther@suse.de from comment #6)
> On Tue, 12 Sep 2023, juzhe.zhong at rivai dot ai wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111337
> > 
> > --- Comment #5 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
> > Oh. I see.
> > 
> > 
> > (define_expand "@vcond_mask_<mode><mode>"
> >   [(match_operand:VB 0 "register_operand")
> >    (match_operand:VB 3 "register_operand")
> >    (match_operand:VB 1 "nonmemory_operand")
> >    (match_operand:VB 2 "register_operand")]
> >   "TARGET_VECTOR"
> >   {
> >    printf ("vcond_mask\n");
> >   })
> > 
> > I implemented this pattern which can address this issue....
> > 
> > Note that VB is VECTR_BOOL...
> > 
> > So, this is a more reasonable way to do. Is that right?
> 
> Ah, I suppose ISEL could special-case VECTOR_BOOLEAN_P COND_EXPR
> to expand to bitwise ops - it already does this, but only for
> non-vector-mode masks:
> 
>   /* Lower mask typed, non-vector mode VEC_COND_EXPRs to bitwise 
> operations.
>      Those can end up generated by folding and at least for integer mode 
> masks
>      we cannot expect vcond expanders to exist.  We lower a ? b : c
>      to (b & a) | (c & ~a).  */
>   if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (lhs))
>       && !VECTOR_MODE_P (mode))
>     {
>       gcc_assert (types_compatible_p (TREE_TYPE (op0), TREE_TYPE (op1)));
>       gimple_seq stmts = NULL;
>       tree type = TREE_TYPE (lhs);
>       location_t loc = gimple_location (stmt);
>       tree tem0 = gimple_build (&stmts, loc, BIT_AND_EXPR, type, op1, 
> op0);
>       tree tem1 = gimple_build (&stmts, loc, BIT_NOT_EXPR, type, op0);
>       tree tem2 = gimple_build (&stmts, loc, BIT_AND_EXPR, type, op2, 
> tem1);
>       tree tem3 = gimple_build (&stmts, loc, BIT_IOR_EXPR, type, tem0, 
> tem2);
>       gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
>       return gimple_build_assign (lhs, tem3);
>     }
> 
> that could be a viable expansion strathegy when the rest fails,
> but as you can see we need four stmts for this.  If you can
> think of a smarter, maybe even single-instruction, way for riscv
> then yes, handling the above pattern looks good.
> 
> I wonder whether SVE/GCN have those.

I have supported this pattern but also need 4 instructions:

(define_expand "@vcond_mask_<mode><mode>"
  [(match_operand:VB 0 "register_operand")
   (match_operand:VB 1 "register_operand")
   (match_operand:VB 2 "register_operand")
   (match_operand:VB 3 "register_operand")]
  "TARGET_VECTOR"
  {
    /* mask1 = operands[3] & operands[1].  */
    rtx mask1 = expand_binop (<MODE>mode, and_optab, operands[1],
                              operands[3], NULL_RTX, 0,
                              OPTAB_DIRECT);
    /* mask2 = ~operands[3] & operands[2].  */
    rtx inverse = expand_unop (<MODE>mode, one_cmpl_optab, operands[3],
                               NULL_RTX,
                               OPTAB_DIRECT);
    rtx mask2 = expand_binop (<MODE>mode, and_optab, operands[2],
                              inverse, NULL_RTX, 0,
                              OPTAB_DIRECT);
    /* result = mask1 | mask2.  */
    rtx result = expand_binop (<MODE>mode, ior_optab, mask1,
                               mask2, NULL_RTX, 0,
                               OPTAB_DIRECT);
    emit_move_insn (operands[0], result);
    DONE;
  })

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[rdapp at gcc dot gnu.org]

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

--- Comment #8 from Robin Dapp <rdapp at gcc dot gnu.org> ---
Yes, I doubt we would get much below 4 instructions with riscv specifics.

A quick grep yesterday didn't reveal any aarch64 or gcn patterns for those (as
long as they are not hidden behind some pattern replacement).  But aarch64
doesn't encounter this situation anyway as we fold differently before.

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[juzhe.zhong at rivai dot ai]

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

--- Comment #9 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
So, should we support this pattern in RISC-V backend ?
Or adjust gimple-isel to generate these 4 STMTs naturally?

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[rdapp at gcc dot gnu.org]

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

--- Comment #10 from Robin Dapp <rdapp at gcc dot gnu.org> ---
I would be OK with the riscv implementation, then we don't need to touch isel. 
Maybe a future vector extension will also help us here so we could just switch
the implementation then.

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[juzhe.zhong at rivai dot ai]

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

--- Comment #11 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
(In reply to Robin Dapp from comment #10)
> I would be OK with the riscv implementation, then we don't need to touch
> isel.  Maybe a future vector extension will also help us here so we could
> just switch the implementation then.

Thank. I will send a patch for it.

I guess the last issue related to testsuite is the incorrect COND_LEN_XXX
gimple fold?

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[rdapp at gcc dot gnu.org]

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

--- Comment #12 from Robin Dapp <rdapp at gcc dot gnu.org> ---
Yes, as far as I know.  I would also go ahead and merge the test suite patch
now as there is already a v2 fix posted.  Even if it's not the correct one it
will be done soon so we should not let that block enabling the test suite.

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[rsandifo at gcc dot gnu.org]

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

--- Comment #13 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #6)
> I wonder whether SVE/GCN have those.
Just to answer this: yeah, SVE does have both vector and predicate
SEL (vcond_mask).  So the fold is useful there.

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[cvs-commit at gcc dot gnu.org]

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

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Robin Dapp <rdapp@gcc.gnu.org>:

https://gcc.gnu.org/g:701b9309b687ed46188b9caeb7d88ad60b0212e5

commit r14-3910-g701b9309b687ed46188b9caeb7d88ad60b0212e5
Author: Juzhe-Zhong <juzhe.zhong@rivai.ai>
Date:   Tue Sep 12 21:32:02 2023 +0800

    RISC-V: Support VECTOR BOOL vcond_mask optab[PR111337]

    As this PR: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111337

    We support VECTOR BOOL vcond_mask to fix this following ICE:
    0x1a9e309 gimple_expand_vec_cond_expr
            ../../../../gcc/gcc/gimple-isel.cc:283
    0x1a9ea56 execute
            ../../../../gcc/gcc/gimple-isel.cc:390

    gcc/ChangeLog:

            PR target/111337
            * config/riscv/autovec.md (vcond_mask_<mode><mode>): New pattern.

[Bug middle-end/111337] ICE in gimple-isel.cc for RISC-V port

[juzhe.zhong at rivai dot ai]

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

JuzheZhong <juzhe.zhong at rivai dot ai> changed:

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

--- Comment #15 from JuzheZhong <juzhe.zhong at rivai dot ai> ---
fixed