Re: [PATCH] Add new model m32r2 of Renesas M32R.

[Kazuhiro Inaoka <inaoka dot kazuhiro at renesas dot com>]

Hi Nick,

> > >       case OPTION_WARN_PARALLEL:
> > >         warn_explicit_parallel_conflicts = 1;
> > > -       error_explicit_parallel_conflicts = 0;
> > >         break;
> > >
> > >       case OPTION_NO_WARN_PARALLEL:
> > >         warn_explicit_parallel_conflicts = 0;
> > > -       error_explicit_parallel_conflicts = 0;
> > >         break;
> > >
> > >       case OPTION_ERROR_PARALLEL:
> > > -       warn_explicit_parallel_conflicts = 1;
> > >         error_explicit_parallel_conflicts = 1;
> > >         break;
> > >
> > >       case OPTION_NO_ERROR_PARALLEL:
> > >         error_explicit_parallel_conflicts = 0;
> > > -       warn_explicit_parallel_conflicts = 0;
> > >         break;
> >
> > Why have you removed these lines ?  Surely the --warn... and
> > --error... options are incompatible ?  You cannot both warn about a
> > parallel conflict and generate an error for it.
> I agree with you at md_parse_options(). Please keep the lines in
> md_parse_options().
Oh, I misstook it.
I mean the following. Because if error_explicit_parallel_conflicts is not
true,
the code will be wrong.

    case OPTION_WARN_PARALLEL:
    warn_explicit_parallel_conflicts = 1;
    break;

    case OPTION_NO_WARN_PARALLEL:
    warn_explicit_parallel_conflicts = 0;
    break;

    case OPTION_ERROR_PARALLEL:
    warn_explicit_parallel_conflicts = 1;
    error_explicit_parallel_conflicts = 1;
    break;

    case OPTION_NO_ERROR_PARALLEL:
    error_explicit_parallel_conflicts = 0;
    warn_explicit_parallel_conflicts = 0;
    break;

Kazuhiro


> Hi Nick
>
> > Where are the error messages ?  If error_explicit_parallel_conflicts
> > is true, then shouldn't an error message be generated rather than a
> > warning ?
> If error_explicit_parallel_conflicts is true, then any error will happen
any
> where.
>
> > > +   if ( check_outputs && !error_explicit_parallel_conflicts )
> > > +     return 0;
> >
> > What is the purpose of this ?  It appears to mean that if
> > error_explicit_parallel_conflicts is not true, then the code will not
> > check for parallel conflicts, even if warn_explicit_parallel_conflicts
> > it true.
> This means that if error_explicit_parallel_conflicts is not true,
> then the code will not check for any parallel conflicts.
> This code will be wrong for general users, but we use it for error case
> to test of  developing chips. (same destination register .etc)
> If they use -no-error-explicit-parallel-conflicts option, the code will
not
> work.
>
> The case of parallel warning works correctly.
>     mv r1,r0 || mv r2,r1
> Warning: output of 1st instruction is the same as an input to  2nd
> instruction - is this intentional?
>
> The case of parallel error doesn't work.
>     mv r1,r0 || mv r1,r2
> Error: Instructions write to the same destination register.
>
>
> > >       case OPTION_WARN_PARALLEL:
> > >         warn_explicit_parallel_conflicts = 1;
> > > -       error_explicit_parallel_conflicts = 0;
> > >         break;
> > >
> > >       case OPTION_NO_WARN_PARALLEL:
> > >         warn_explicit_parallel_conflicts = 0;
> > > -       error_explicit_parallel_conflicts = 0;
> > >         break;
> > >
> > >       case OPTION_ERROR_PARALLEL:
> > > -       warn_explicit_parallel_conflicts = 1;
> > >         error_explicit_parallel_conflicts = 1;
> > >         break;
> > >
> > >       case OPTION_NO_ERROR_PARALLEL:
> > >         error_explicit_parallel_conflicts = 0;
> > > -       warn_explicit_parallel_conflicts = 0;
> > >         break;
> >
> > Why have you removed these lines ?  Surely the --warn... and
> > --error... options are incompatible ?  You cannot both warn about a
> > parallel conflict and generate an error for it.
> I agree with you at md_parse_options(). Please keep the lines in
> md_parse_options().
>
> Thank you for your advice.
>
> Kazuhiro
>