Hi Robin,
Thank you for your suggestions!
The patch has been updated by adding a new attribute to
disable alternative for xtheadvector or RVV1.0 instead of
overlaoding group_overlap.
Joshua
------------------------------------------------------------------
发件人：钟居哲 <juzhe.zh...@rivai.ai>
发送时间：2024年1月10日(星期三) 21:43
收件人："rdapp.gcc"<rdapp....@gmail.com>;
"cooper.joshua"<cooper.jos...@linux.alibaba.com>;
"gcc-patches"<gcc-patches@gcc.gnu.org>
抄 送："rdapp.gcc"<rdapp....@gmail.com>;
"jim.wilson.gcc"<jim.wilson....@gmail.com>; palmer<pal...@dabbelt.com>;
andrew<and...@sifive.com>; "philipp.tomsich"<philipp.toms...@vrull.eu>; Jeff
Law<jeffreya...@gmail.com>; "Christoph Müllner"<christoph.muell...@vrull.eu>;
jinma<ji...@linux.alibaba.com>; Cooper Qu<cooper...@linux.alibaba.com>
主 题：Re: Re: [PATCH v5] RISC-V: Fix register overlap issue for some xtheadvector
instructions
>> For the other insns, I wonder if we could get away with not really
>>disabling the newly added early-clobber alternatives for RVV but
>>just disparaging ("?") them? That way we could re-use "full" for
>>the thv-disabled alternatives and "none" for the newly added ones.
>>("none" will still be misleading then, though :/)
I prefer to disable those early-clobber alternatives added of theadvector for
RVV,
since disparage still make RA possible reaches the early clobber alternatives.
>>If this doesn't work or others feel the separation is not strict
>>enough, I'd prefer a separate attribute rather than overloading
>>group_overlap. Maybe something like "spec_restriction" or similar
>>with two values "rvv" and "thv"?
I like this idea, it makes more sense to me. So I think it's better to add an
attribute to
disable alternative for theadvector or RVV1.0.
juzhe.zh...@rivai.ai
From: Robin Dapp
Date: 2024-01-10 21:36
To: Jun Sha (Joshua); gcc-patches
CC: rdapp.gcc; jim.wilson.gcc; palmer; andrew; philipp.tomsich; jeffreyalaw;
christoph.muellner; juzhe.zhong; Jin Ma; Xianmiao Qu
Subject: Re: [PATCH v5] RISC-V: Fix register overlap issue for some
xtheadvector instructions
Hi Joshua,
> For th.vmadc/th.vmsbc as well as narrowing arithmetic instructions
> and floating-point compare instructions, an illegal instruction
> exception will be raised if the destination vector register overlaps
> a source vector register group.
>
> To handle this issue, we use "group_overlap" and "enabled" attribute
> to disable some alternatives for xtheadvector.
> ;; Widening instructions have group-overlap constraints. Those are only
> ;; valid for certain register-group sizes. This attribute marks the
> ;; alternatives not matching the required register-group size as disabled.
> -(define_attr "group_overlap" "none,W21,W42,W84,W43,W86,W87,W0"
> +(define_attr "group_overlap"
> "none,W21,W42,W84,W43,W86,W87,W0,thv_disabled,rvv_disabled"
> (const_string "none"))
I realize there have been some discussions before but I find the naming
misleading. The group_overlap attribute is supposed to specify whether
groups overlap (and mark the respective alternatives accepting
only this overlap).
Then we check if the groups overlap and disable all non-matching
alternatives. "none" i.e. "no overlap" always matches.
Your first goal seems to be to disable existing non-early-clobber
alternatives for thv. For this, maybe "full", "same" (or "any"?) would
work? Please also add a comment in group_overlap_valid then that we
need not actually check for register equality.
For the other insns, I wonder if we could get away with not really
disabling the newly added early-clobber alternatives for RVV but
just disparaging ("?") them? That way we could re-use "full" for
the thv-disabled alternatives and "none" for the newly added ones.
("none" will still be misleading then, though :/)
If this doesn't work or others feel the separation is not strict
enough, I'd prefer a separate attribute rather than overloading
group_overlap. Maybe something like "spec_restriction" or similar
with two values "rvv" and "thv"?
Regards
Robin
