Thank you @Jeff Law <jeffreya...@gmail.com> and noted the suggestions and
will follow the same .
~U
On Fri, Aug 15, 2025 at 7:07 PM Jeff Law <jeffreya...@gmail.com> wrote:
>
>
> On 8/15/25 3:00 AM, Umesh Kalappa wrote:
> > Addressed the comments and tested "runtest --tool gcc
> --target_board='riscv-sim/-march=rv64gc_zba_zbb_zbc_zbs/-mabi=lp64/-mcmodel=medlow'
> riscv.exp" and 32 bit too
> >
> > lint warnings can be ignored for riscv-ext.opt.
> >
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv-ext-mips.def (DEFINE_RISCV_EXT):
> > Added mips prefetch extension.
> > * config/riscv/riscv-ext.opt: Generated file.
> > * config/riscv/riscv.md (prefetch):
> > Added mips prefetch address operand constraint.
> > * config/riscv/constraints.md: Added mips specific constraint.
> > * config/riscv/predicates.md (prefetch_operand):
> > Updated for mips nine bits offset.
> > * config/riscv/riscv.cc (riscv_prefetch_offset_address_p):
> > Legitimate address with offset for prefetch check.
> > * config/riscv/riscv-protos.h: Likewise.
> > * config/riscv/riscv.h:
> > Macros to support for mips cached type.
> > * doc/riscv-ext.texi: Updated for mips prefetch.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/riscv/mipsprefetch.c: Test file for mips.pref.
> > ---
> > gcc/config/riscv/constraints.md | 4 +++
> > gcc/config/riscv/predicates.md | 20 +++++++++++-
> > gcc/config/riscv/riscv-ext-mips.def | 13 ++++++++
> > gcc/config/riscv/riscv-ext.opt | 2 ++
> > gcc/config/riscv/riscv-protos.h | 2 ++
> > gcc/config/riscv/riscv.cc | 31 +++++++++++++++++++
> > gcc/config/riscv/riscv.h | 11 +++++++
> > gcc/config/riscv/riscv.md | 18 ++++++++---
> > gcc/doc/riscv-ext.texi | 4 +++
> > gcc/testsuite/gcc.target/riscv/mipsprefetch.c | 31 +++++++++++++++++++
> > 10 files changed, 131 insertions(+), 5 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/riscv/mipsprefetch.c
> >
>
>
> >
> > +;; REG or REG+D where D fits in a uimm9
> > +(define_predicate "mips_prefetch_operand"
> > + (ior (match_operand 0 "register_operand")
> > + (match_test "prefetch_const_operand (op, VOIDmode)")
> > + (and (match_code "plus")
> > + (match_test "register_operand (XEXP (op, 0), word_mode)")
> > + (match_test "prefetch_const_operand (XEXP (op, 1),
> VOIDmode)"))))
> Formatting nit. Instead of 8 spaces, use a tab. I've fixed this.
>
>
>
> > +
> > +DEFINE_RISCV_EXT (
> > + /* NAME. */ xmipscbop,
> > + /* UPPERCASE_NAME. */ XMIPSCBOP,
> > + /* FULL_NAME. */ "Mips Prefetch extension",
> > + /* DESC. */ "",
> > + /* URL. */ ,
> Consider adding your URL here for the extension as a follow-up. We
> don't require it, but it's potentially helpful in the future if someone
> needs to lookup precisely how this extension is defined.
>
>
>
>
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index e9217dcb043..7722497289b 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -15449,6 +15449,37 @@ synthesize_add (rtx operands[3])
> > return true;
> > }
> >
> > +/*
> > + hint : argument specify the target cache
> > +
> > + TODO : locality is unused.
> When we refer to arguments, variable names, etc in comments it is
> customary to put them in all caps. I've fixed this.
>
> > +
> > + Return the first operand of the associated PREF or PREFX insn. */
> > +rtx
> > +riscv_prefetch_cookie (rtx hint, rtx locality)
> > +{
> > + return (GEN_INT (INTVAL (hint)
> > + + CacheHint::DCACHE_HINT + INTVAL (locality) * 0));
> > +}
> Formatting nit. When we wrap a line the operator is indented inside the
> parens. Like this
>
> (something
> + something else)
>
> I've fixed this.
>
> > +
> > +/* Return true if X is a legitimate address with offset for prefetch.
> > + MODE is the mode of the value being accessed. */
> > +bool
> > +riscv_prefetch_offset_address_p (rtx x, machine_mode mode)
> > +{
> > + struct riscv_address_info addr;
> > +
> > + if (riscv_classify_address (&addr, x, mode, false)
> > + && addr.type == ADDRESS_REG)
> > + {
> > + if (TARGET_XMIPSCBOP)
> > + return (CONST_INT_P (addr.offset)
> > + && MIPS_RISCV_9BIT_OFFSET_P (INTVAL (addr.offset)));
> Similarly here. I've fixed this too.
>
>
>
> > {
> > + if (TARGET_XMIPSCBOP)
> > + {
> > + /* Mips Prefetch write is nop for p8700. */
> > + if (operands[1] != CONST0_RTX (GET_MODE (operands[1])))
> > + return "nop";
> > +
> > + operands[1] = riscv_prefetch_cookie (operands[1], operands[2]);
> > + return "mips.pref\t%1,%a0";
> > + }
> > +
> Always make sure to indent two positions further for the THEN/ELSE
> clauses after an if-statement. like this;
>
> if (whatever)
> true clause;
> else
> false clause;
>
> I've fixed this too.
>
> I've pushed this patch with the fixes noted above to the trunk. Thanks!
>
> jeff
>
>
>
>
