回复：RISC-V: Support XTheadVector extensions
Hi Kito, Thank you for your support. We will get involved into community work actively for our XTheadVector patches. Let's work on upstream together during this process. Currently, we are polishing up our patches according to Ju-Zhe's suggestions. Patch v3 with higher quality and less invasion will come soon. Joshua -- 发件人：Kito Cheng 发送时间：2023年11月18日(星期六) 18:33 收件人：Philipp Tomsich 抄 送：Jeff Law; "juzhe.zh...@rivai.ai"; "gcc-patches"; "kito.cheng"; "cooper.joshua"; Robin Dapp; jkridner 主 题：Re: RISC-V: Support XTheadVector extensions I guess it would be worth to state my thought publicly: I *support* adding the T-head vector (a.k.a. vector 0.7) to upstream GCC since T-Head vector already ships a large enough number of boards, also it's not really T-head's problem as Palmer described in another mail. My biggest concern before is T-head folks didn't involved into community work too much, so accept that definitely will increasing work for maintainers, however I saw T-head folks is trying to contribute stuffs to upstream now, so may not a concern now, also I believe accept this patch will encourage they work more on upstream together, which is benefit to each other. Back to the one of the biggest issues for the patch set: GCC 14 or GCC 15. My general thought is it may be OK if it's less invasive enough, then should be OK for GCC 14, but I don't have a strong opinion, since as you know I am not the main developer of the vector part, so I will let Ju-Zhe make the final decision, because he is the one who contributes most things to RISC-V vector gcc support.
Re: T-Head Vector for GCC-14? (was Re: RISC-V: Support XTheadVector extensions)
On Tue, Nov 28, 2023 at 5:21 PM Jeff Law wrote: > > On 11/28/23 12:56, Philipp Tomsich wrote: > > >> That's obviously a risky thing to do given it was sent right at the end > >> of the window, but it meets the rules. > >> > >> Folks in the call seemed generally amenable to at least trying for 14, > >> so unless anyone's opposed on the lists it seems like the way to go. > >> IIRC we ended up with the following TODO list: > >> > >> * Make sure this doesn't regress on the targets we already support. > >>From the sounds of things there's been test suite runs that look fine, > >>so hopefully that's all manageable. Christoph said he'd send > >>something out, we've had a bunch of test skew so there might be a bit > >>lurking but it should be generally manageable. > >> * We agree on some sort of support lifecycle. There seemed to be > >>basically two proposals: merge for 14 with the aim of quickly > >>deperecating it (maybe even for 15), or merge for 14 with the aim of > >>keeping it until it ends up un-tested (ie, requiring test results are > >>published for every release). > > > > We expect real-world users, including the BeagleV-AHEAD community, to > > need support for the foreseeable future. > > Keeping it until it ends up untested (and test cases are reasonably > > clean) sounds like a good threshold to ensure the integrity of the > > codebase while giving this a clear path to stay in for its useful > > life. > I can live with it being in the tree as long as it's maintained > (measured by ongoing testing with reasonable results). > > I'd proposed that it could end up deprecated quickly, but that was based > on the assumption that once V1.0 compliant hardware was widely available > that we'd see less and less interest in the thead extensions. > At BeagleBoard.org, we focus on long-term support and availability. Long-term support is a key for us engaging with education, both institutional and continuing, and industrial automation. Getting this into mainline such that we can develop solutions that integrate with mainline Linux distributions is key for us to enable broader RISC-V adoption. If it is deprecated at some point, that won't be terrible as long as we are able to get to a good snapshot where integration with the rest of the open source developer community has reasonably happened. The good news is it *will* get tested. We have confidence in that side of things. We have a great community that will engage the compiler and identify regressions. My expectation is that the Alibaba folks really know the C910 CPU core and will help us get things right. I'll be here to help escalate issues to them if they become unresponsive to the list. Others involved in the BeagleBoard.org project will help make sure I know when I need to escalate such issues. Let me know if there's anything I can do to encourage this being merged and worrying about deprecation later. -- https://beagleboard.org/about/jkridner - a 501c3 non-profit educating around open hardware computing
Re: T-Head Vector for GCC-14? (was Re: RISC-V: Support XTheadVector extensions)
On 11/28/23 12:56, Philipp Tomsich wrote: That's obviously a risky thing to do given it was sent right at the end of the window, but it meets the rules. Folks in the call seemed generally amenable to at least trying for 14, so unless anyone's opposed on the lists it seems like the way to go. IIRC we ended up with the following TODO list: * Make sure this doesn't regress on the targets we already support. From the sounds of things there's been test suite runs that look fine, so hopefully that's all manageable. Christoph said he'd send something out, we've had a bunch of test skew so there might be a bit lurking but it should be generally manageable. * We agree on some sort of support lifecycle. There seemed to be basically two proposals: merge for 14 with the aim of quickly deperecating it (maybe even for 15), or merge for 14 with the aim of keeping it until it ends up un-tested (ie, requiring test results are published for every release). We expect real-world users, including the BeagleV-AHEAD community, to need support for the foreseeable future. Keeping it until it ends up untested (and test cases are reasonably clean) sounds like a good threshold to ensure the integrity of the codebase while giving this a clear path to stay in for its useful life. I can live with it being in the tree as long as it's maintained (measured by ongoing testing with reasonable results). I'd proposed that it could end up deprecated quickly, but that was based on the assumption that once V1.0 compliant hardware was widely available that we'd see less and less interest in the thead extensions. Jeff
Re: RISC-V: Support XTheadVector extensions
On 11/28/23 12:45, Palmer Dabbelt wrote: IMO we're just stuck between a rock and a hard place here. Specifically, this isn't just an assembly syntax change but also comes with a bunch of behaviorial changes to the instructions in question -- I'm specifically thinking of things like the register packing, which IIRC changed a ton between 0.7 and 0.8 (and then again more for 1.0). That's the kind of stuff that tends to have non-local implications on the port, and thus can trip people up. So if we model this as just assembly syntax then we risk people tripping over the differences, but if we try to model it as a whole different extension then we have more code to manage. I'd start with the assembly syntax approach, as it should be the option with less code which is always nice. If that turns out to be a problem then we can always just duplicate the patterns, but it's way harder to merge them back together if we start out with things duplicated. The way I think about the assembly bits is it allows us to share a good amount of code between the two implementations. There's obviously going to be some differences that will require more extensive work and that's where I think most of our review effort ought to be. During the patchwork call we also ended up talking about the P extension (and the likely vendor flavors). Nothing's appeared for there yet, but the theory is that the RZ/Five (Renesas' line of RISC-V chips that came out earlier this year) has some P-related extension. There's also some SIMD in CORE-V, as well as a bunch of low-hanging fruit missing from V that we'll probably see more vendor extensions for. The only P bits that made the gcc-14 deadline were those from Embecosm, so I'd tend to want to push all the other P stuff out to gcc-15. So I think if the goal is to have a single vector target for RISC-V then we've probably lost already. That's probably not feasible. But I think there's a good amount of sharable bits between the V1.0 and the thead-vector support. But we've got time to sort this out. I don't think the code in question was targeted towards gcc-14. [In case anyone else is watching: see the forked thread, it might be amied for 14 now...] It's aimed for evaluation/review given the submission occurred before the gcc-14 deadline. jeff
Re: T-Head Vector for GCC-14? (was Re: RISC-V: Support XTheadVector extensions)
On Tue, 28 Nov 2023 at 20:31, Palmer Dabbelt wrote: > > On Wed, 22 Nov 2023 14:27:50 PST (-0800), jeffreya...@gmail.com wrote: > > ... > > [Trimming everything else, as this is a big change. I'm also making it > a new subject/thread, so folks can see.] > > > More generally, I think I need to soften my prior statement about > > deferring this to gcc-15. This code was submitted in time for the > > gcc-14 deadline, so it should be evaluated just like we do anything else > > that makes the deadline. There are various criteria we use to evaluate > > if something should get integrated and we should just work through this > > series like we always do and not treat it specially in any way. > > We talked about this some in the pachwork meeting today. There's a lot > of moving parts here, so here's my best bet at summarizing > > It seems like folks broadly agree: I think the only reason everyone was > so quick to defer to 15 was because we though the Vrull guys even want > to, but sounds like there's some interest in getting this into 14. Thank you for the follow-up on this, as I had the original conversation with Jeff in passing. We (and the Alibaba folks and the BeagleV-AHEAD community) would prefer to get this into 14. > That's obviously a risky thing to do given it was sent right at the end > of the window, but it meets the rules. > > Folks in the call seemed generally amenable to at least trying for 14, > so unless anyone's opposed on the lists it seems like the way to go. > IIRC we ended up with the following TODO list: > > * Make sure this doesn't regress on the targets we already support. > From the sounds of things there's been test suite runs that look fine, > so hopefully that's all manageable. Christoph said he'd send > something out, we've had a bunch of test skew so there might be a bit > lurking but it should be generally manageable. > * We agree on some sort of support lifecycle. There seemed to be > basically two proposals: merge for 14 with the aim of quickly > deperecating it (maybe even for 15), or merge for 14 with the aim of > keeping it until it ends up un-tested (ie, requiring test results are > published for every release). We expect real-world users, including the BeagleV-AHEAD community, to need support for the foreseeable future. Keeping it until it ends up untested (and test cases are reasonably clean) sounds like a good threshold to ensure the integrity of the codebase while giving this a clear path to stay in for its useful life. Philipp. > * We actually find some time to sit down and do the code review. > That'll be a chunk of work and time is tight since most of us are > focusing on V-1.0, but hopefully we've got time to fit things in. > * There's some options for testing without hardware: QEMU dropped > support for V-0.7.1 a while ago, but there's a patch set that's not > yet on the lists to bring that back. > > So I think unless anyone's opposed, we can at least start looking into > getting this into GCC-14 -- there's obviously still a ton of review work > to do and we might find something problematic, but we won't know until > we actually sit down and do the reviews. > > --- > > Then for my opinions: > > The only policy worry I have is the support lifecycle: IMO merging > something we're going to quickly deprecate is going to lead to headaches > for users, so we should only merge this if we're going to plan on > supporting it for the life of the hardware. That's always hard to > define, but we talked through the option of pushing this onto the users: > we'd require test results published for every GCC release, and if no > reasonably cleas test results are published then we'll assume the HW is > defunct and support for it can be deprecated. That's sort of patterned > on how glibc documents deprecating ports. > > IIRC we didn't really end up with any deprecation policy when merging > the other vendor support, so I'd argue we should just make that the > general plan for supporting vendor extensions. It pushes a little more > work to the vendors/users than we have before, but I think it's a good > balance. It's also a pretty easy policy for vendors to understand: if > they want their custom stuff supported, they need to demonstrate it > works.
Re: RISC-V: Support XTheadVector extensions
On Fri, 17 Nov 2023 16:01:27 PST (-0800), jeffreya...@gmail.com wrote: On 11/17/23 16:16, 钟居哲 wrote: >> I assume this hunk is meant for riscv_output_operand in riscv.cc. We may also need to add '^' to the punct_valid_p hook. But yes, this is the preferred way to go when all we need to do is prefix the instruction with "th.". No. I don't think we need to add '^' . I don't want theadvector to touch any codes of vector.md. Mixing up theadvector with RVV1.0 is a nighmare for RVV maintain. People like me don't want to touch any thing related to Thead. But anyway, I will take care of that in GCC-15. I suspect it's going to be even worse if you we have multiple patterns with the same underlying RTL, but just different output strings. The standard way to handle that has been with an output modifier and/or ASSEMBLER_DIALECT. If you look at the PA port for example, the assembler syntax changed dramatically between the PA1.0/PA1.1 era and the PA2.0 era. But we support both variants trivially without duplicating all the patterns. IMO we're just stuck between a rock and a hard place here. Specifically, this isn't just an assembly syntax change but also comes with a bunch of behaviorial changes to the instructions in question -- I'm specifically thinking of things like the register packing, which IIRC changed a ton between 0.7 and 0.8 (and then again more for 1.0). That's the kind of stuff that tends to have non-local implications on the port, and thus can trip people up. So if we model this as just assembly syntax then we risk people tripping over the differences, but if we try to model it as a whole different extension then we have more code to manage. I'd start with the assembly syntax approach, as it should be the option with less code which is always nice. If that turns out to be a problem then we can always just duplicate the patterns, but it's way harder to merge them back together if we start out with things duplicated. During the patchwork call we also ended up talking about the P extension (and the likely vendor flavors). Nothing's appeared for there yet, but the theory is that the RZ/Five (Renesas' line of RISC-V chips that came out earlier this year) has some P-related extension. There's also some SIMD in CORE-V, as well as a bunch of low-hanging fruit missing from V that we'll probably see more vendor extensions for. So I think if the goal is to have a single vector target for RISC-V then we've probably lost already. But we've got time to sort this out. I don't think the code in question was targeted towards gcc-14. [In case anyone else is watching: see the forked thread, it might be amied for 14 now...] jeff
T-Head Vector for GCC-14? (was Re: RISC-V: Support XTheadVector extensions)
On Wed, 22 Nov 2023 14:27:50 PST (-0800), jeffreya...@gmail.com wrote: ... [Trimming everything else, as this is a big change. I'm also making it a new subject/thread, so folks can see.] More generally, I think I need to soften my prior statement about deferring this to gcc-15. This code was submitted in time for the gcc-14 deadline, so it should be evaluated just like we do anything else that makes the deadline. There are various criteria we use to evaluate if something should get integrated and we should just work through this series like we always do and not treat it specially in any way. We talked about this some in the pachwork meeting today. There's a lot of moving parts here, so here's my best bet at summarizing It seems like folks broadly agree: I think the only reason everyone was so quick to defer to 15 was because we though the Vrull guys even want to, but sounds like there's some interest in getting this into 14. That's obviously a risky thing to do given it was sent right at the end of the window, but it meets the rules. Folks in the call seemed generally amenable to at least trying for 14, so unless anyone's opposed on the lists it seems like the way to go. IIRC we ended up with the following TODO list: * Make sure this doesn't regress on the targets we already support. From the sounds of things there's been test suite runs that look fine, so hopefully that's all manageable. Christoph said he'd send something out, we've had a bunch of test skew so there might be a bit lurking but it should be generally manageable. * We agree on some sort of support lifecycle. There seemed to be basically two proposals: merge for 14 with the aim of quickly deperecating it (maybe even for 15), or merge for 14 with the aim of keeping it until it ends up un-tested (ie, requiring test results are published for every release). * We actually find some time to sit down and do the code review. That'll be a chunk of work and time is tight since most of us are focusing on V-1.0, but hopefully we've got time to fit things in. * There's some options for testing without hardware: QEMU dropped support for V-0.7.1 a while ago, but there's a patch set that's not yet on the lists to bring that back. So I think unless anyone's opposed, we can at least start looking into getting this into GCC-14 -- there's obviously still a ton of review work to do and we might find something problematic, but we won't know until we actually sit down and do the reviews. --- Then for my opinions: The only policy worry I have is the support lifecycle: IMO merging something we're going to quickly deprecate is going to lead to headaches for users, so we should only merge this if we're going to plan on supporting it for the life of the hardware. That's always hard to define, but we talked through the option of pushing this onto the users: we'd require test results published for every GCC release, and if no reasonably cleas test results are published then we'll assume the HW is defunct and support for it can be deprecated. That's sort of patterned on how glibc documents deprecating ports. IIRC we didn't really end up with any deprecation policy when merging the other vendor support, so I'd argue we should just make that the general plan for supporting vendor extensions. It pushes a little more work to the vendors/users than we have before, but I think it's a good balance. It's also a pretty easy policy for vendors to understand: if they want their custom stuff supported, they need to demonstrate it works.
Re: RISC-V: Support XTheadVector extensions
I am less worry about the thead vector combined with other zv extension, instead we should reject those combinations at all. My reason is thead vector is transitional products, they won't have any further new products with that longer, also it's not compatible with all other zv extension in theory, zv extension requires at least zve32x which is subset of v1p0, and I don't think it's valid to use thead vector as replacement required extension - it should just introduce another thead vector extension instead. Jeff Law 於 2023年11月23日 週四 06:27 寫道： > > > On 11/22/23 07:24, Christoph Müllner wrote: > > On Wed, Nov 22, 2023 at 2:52 PM 钟居哲 wrote: > >> > >> I am totally ok to approve theadvector on GCC-14 before stage 3 close > >> as long as it doesn't touch the current RVV codes too much and binutils > supports theadvector. > >> > >> I have provided the draft approach: > >> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637349.html > >> which turns out doesn't need to change any codes of vector.md. > >> I strongly suggest follow this draft. I can be actively review > theadvector during stage 3. > >> And hopefully can help you land theadvector on GCC-14. > > > > I see now two approaches: > > 1) Let GCC emit RVV instructions for XTheadVector for instructions > > that are in both > > 2) Use the ASM_OUTPUT_OPCODE hook to output "th." for these instructions > > > > No doubt, the ASM_OUTPUT_OPCODE hook approach is better than our > > format-string approach, but would 1) not be the even better > > solution? It would also mean, that not a single test case is required > > for these overlapping instructions (only a few tests that ensure that > > we don't emit RVV instructions that are not available in > > XTheadVector). Besides that, letting GCC emit RVV instructions for > > XTheadVector is a very clever idea, because it fully utilizes the > > fact that both extensions overlap to a huge degree. > > > > The ASM_OUTPUT_OPCODE approach could lead to an issue if we enable > XTheadVector > > with any other vector extension, say Zvfoo. In this case the Zvfoo > > instructions will all be prefixed as well with "th.". I know that it > > is not likely to run into this problem (such a machine does not exist > > in real hardware), but it is possible to trigger this issue easily > > and approach 1) would not have this potential issue. > I'm not a big fan of the ASM_OUTPUT_OPCODE approach.While it is > simple, I worry a bit about it from a long term maintenance standpoint. > As you note we could well end up at some point with an extension that > has an mnenomic starting with "v" that would blow up. But I certainly > see the appeal of such a simple test to support thead vector. > > Given there are at least 3 approaches that can fix that problem (%^, > assembler dialect or ASM_OUTPUT_OPCODE), maybe we could set that > discussion aside in the immediate term and see if there are other issues > that are potentially more substantial. > > > > > -- > > > > More generally, I think I need to soften my prior statement about > deferring this to gcc-15. This code was submitted in time for the > gcc-14 deadline, so it should be evaluated just like we do anything else > that makes the deadline. There are various criteria we use to evaluate > if something should get integrated and we should just work through this > series like we always do and not treat it specially in any way. > > > jeff >
Re: RISC-V: Support XTheadVector extensions
On Wed, Nov 22, 2023 at 11:48 PM Kito Cheng wrote: > > I am less worry about the thead vector combined with other zv extension, > instead we should reject those combinations at all. > > My reason is thead vector is transitional products, they won't have any > further new products with that longer, also it's not compatible with all > other zv extension in theory, zv extension requires at least zve32x which is > subset of v1p0, and I don't think it's valid to use thead vector as > replacement required extension - it should just introduce another thead > vector extension instead. The "transitional products" argument is probably enough to add this restriction, so we will add this to the first patch of the series. Further, we'll implement approach 1 (emitting no "th." prefix for instructions in vector.md) with an additional patch on top, which implements the ASM_OUTPUT_OPCODE hook (with a comment that clarifies why "ptr[0] == 'v'" is sufficient there). So the decision about this can be postponed and we can focus on the rest of the patchset as Jeff suggested. Thanks for the inputs! > > > > Jeff Law 於 2023年11月23日 週四 06:27 寫道： >> >> >> >> On 11/22/23 07:24, Christoph Müllner wrote: >> > On Wed, Nov 22, 2023 at 2:52 PM 钟居哲 wrote: >> >> >> >> I am totally ok to approve theadvector on GCC-14 before stage 3 close >> >> as long as it doesn't touch the current RVV codes too much and binutils >> >> supports theadvector. >> >> >> >> I have provided the draft approach: >> >> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637349.html >> >> which turns out doesn't need to change any codes of vector.md. >> >> I strongly suggest follow this draft. I can be actively review >> >> theadvector during stage 3. >> >> And hopefully can help you land theadvector on GCC-14. >> > >> > I see now two approaches: >> > 1) Let GCC emit RVV instructions for XTheadVector for instructions >> > that are in both >> > 2) Use the ASM_OUTPUT_OPCODE hook to output "th." for these instructions >> > >> > No doubt, the ASM_OUTPUT_OPCODE hook approach is better than our >> > format-string approach, but would 1) not be the even better >> > solution? It would also mean, that not a single test case is required >> > for these overlapping instructions (only a few tests that ensure that >> > we don't emit RVV instructions that are not available in >> > XTheadVector). Besides that, letting GCC emit RVV instructions for >> > XTheadVector is a very clever idea, because it fully utilizes the >> > fact that both extensions overlap to a huge degree. >> > >> > The ASM_OUTPUT_OPCODE approach could lead to an issue if we enable >> XTheadVector >> > with any other vector extension, say Zvfoo. In this case the Zvfoo >> > instructions will all be prefixed as well with "th.". I know that it >> > is not likely to run into this problem (such a machine does not exist >> > in real hardware), but it is possible to trigger this issue easily >> > and approach 1) would not have this potential issue. >> I'm not a big fan of the ASM_OUTPUT_OPCODE approach.While it is >> simple, I worry a bit about it from a long term maintenance standpoint. >> As you note we could well end up at some point with an extension that >> has an mnenomic starting with "v" that would blow up. But I certainly >> see the appeal of such a simple test to support thead vector. >> >> Given there are at least 3 approaches that can fix that problem (%^, >> assembler dialect or ASM_OUTPUT_OPCODE), maybe we could set that >> discussion aside in the immediate term and see if there are other issues >> that are potentially more substantial. >> >> >> >> >> -- >> >> >> >> More generally, I think I need to soften my prior statement about >> deferring this to gcc-15. This code was submitted in time for the >> gcc-14 deadline, so it should be evaluated just like we do anything else >> that makes the deadline. There are various criteria we use to evaluate >> if something should get integrated and we should just work through this >> series like we always do and not treat it specially in any way. >> >> >> jeff
Re: Re: RISC-V: Support XTheadVector extensions
I prefer ASM_OUTPUT_OPCODE or assembler dialect to %^ and I don't want to see any change of vector.md. %^ will cause high burden for future maintainment. Besides, ASM_OUTPUT_OPCODE can the whole string. My patch is just a draft. We can exlude for example, in zvbb, we can exclude appending "th." in vrev.v instruction. juzhe.zh...@rivai.ai From: Jeff Law Date: 2023-11-23 06:27 To: Christoph Müllner; 钟居哲 CC: gcc-patches; kito.cheng; kito.cheng; cooper.joshua; rdapp.gcc; philipp.tomsich; Cooper Qu; jinma; Nelson Chu Subject: Re: RISC-V: Support XTheadVector extensions On 11/22/23 07:24, Christoph Müllner wrote: > On Wed, Nov 22, 2023 at 2:52 PM 钟居哲 wrote: >> >> I am totally ok to approve theadvector on GCC-14 before stage 3 close >> as long as it doesn't touch the current RVV codes too much and binutils >> supports theadvector. >> >> I have provided the draft approach: >> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637349.html >> which turns out doesn't need to change any codes of vector.md. >> I strongly suggest follow this draft. I can be actively review theadvector >> during stage 3. >> And hopefully can help you land theadvector on GCC-14. > > I see now two approaches: > 1) Let GCC emit RVV instructions for XTheadVector for instructions > that are in both > 2) Use the ASM_OUTPUT_OPCODE hook to output "th." for these instructions > > No doubt, the ASM_OUTPUT_OPCODE hook approach is better than our > format-string approach, but would 1) not be the even better > solution? It would also mean, that not a single test case is required > for these overlapping instructions (only a few tests that ensure that > we don't emit RVV instructions that are not available in > XTheadVector). Besides that, letting GCC emit RVV instructions for > XTheadVector is a very clever idea, because it fully utilizes the > fact that both extensions overlap to a huge degree. > > The ASM_OUTPUT_OPCODE approach could lead to an issue if we enable XTheadVector > with any other vector extension, say Zvfoo. In this case the Zvfoo > instructions will all be prefixed as well with "th.". I know that it > is not likely to run into this problem (such a machine does not exist > in real hardware), but it is possible to trigger this issue easily > and approach 1) would not have this potential issue. I'm not a big fan of the ASM_OUTPUT_OPCODE approach.While it is simple, I worry a bit about it from a long term maintenance standpoint. As you note we could well end up at some point with an extension that has an mnenomic starting with "v" that would blow up. But I certainly see the appeal of such a simple test to support thead vector. Given there are at least 3 approaches that can fix that problem (%^, assembler dialect or ASM_OUTPUT_OPCODE), maybe we could set that discussion aside in the immediate term and see if there are other issues that are potentially more substantial. -- More generally, I think I need to soften my prior statement about deferring this to gcc-15. This code was submitted in time for the gcc-14 deadline, so it should be evaluated just like we do anything else that makes the deadline. There are various criteria we use to evaluate if something should get integrated and we should just work through this series like we always do and not treat it specially in any way. jeff
Re: RISC-V: Support XTheadVector extensions
On 11/22/23 07:24, Christoph Müllner wrote: On Wed, Nov 22, 2023 at 2:52 PM 钟居哲 wrote: I am totally ok to approve theadvector on GCC-14 before stage 3 close as long as it doesn't touch the current RVV codes too much and binutils supports theadvector. I have provided the draft approach: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637349.html which turns out doesn't need to change any codes of vector.md. I strongly suggest follow this draft. I can be actively review theadvector during stage 3. And hopefully can help you land theadvector on GCC-14. I see now two approaches: 1) Let GCC emit RVV instructions for XTheadVector for instructions that are in both 2) Use the ASM_OUTPUT_OPCODE hook to output "th." for these instructions No doubt, the ASM_OUTPUT_OPCODE hook approach is better than our format-string approach, but would 1) not be the even better solution? It would also mean, that not a single test case is required for these overlapping instructions (only a few tests that ensure that we don't emit RVV instructions that are not available in XTheadVector). Besides that, letting GCC emit RVV instructions for XTheadVector is a very clever idea, because it fully utilizes the fact that both extensions overlap to a huge degree. The ASM_OUTPUT_OPCODE approach could lead to an issue if we enable XTheadVector with any other vector extension, say Zvfoo. In this case the Zvfoo instructions will all be prefixed as well with "th.". I know that it is not likely to run into this problem (such a machine does not exist in real hardware), but it is possible to trigger this issue easily and approach 1) would not have this potential issue. I'm not a big fan of the ASM_OUTPUT_OPCODE approach.While it is simple, I worry a bit about it from a long term maintenance standpoint. As you note we could well end up at some point with an extension that has an mnenomic starting with "v" that would blow up. But I certainly see the appeal of such a simple test to support thead vector. Given there are at least 3 approaches that can fix that problem (%^, assembler dialect or ASM_OUTPUT_OPCODE), maybe we could set that discussion aside in the immediate term and see if there are other issues that are potentially more substantial. -- More generally, I think I need to soften my prior statement about deferring this to gcc-15. This code was submitted in time for the gcc-14 deadline, so it should be evaluated just like we do anything else that makes the deadline. There are various criteria we use to evaluate if something should get integrated and we should just work through this series like we always do and not treat it specially in any way. jeff
Re: Re: RISC-V: Support XTheadVector extensions
On Wed, Nov 22, 2023 at 2:52 PM 钟居哲 wrote: > > I am totally ok to approve theadvector on GCC-14 before stage 3 close > as long as it doesn't touch the current RVV codes too much and binutils > supports theadvector. > > I have provided the draft approach: > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637349.html > which turns out doesn't need to change any codes of vector.md. > I strongly suggest follow this draft. I can be actively review theadvector > during stage 3. > And hopefully can help you land theadvector on GCC-14. I see now two approaches: 1) Let GCC emit RVV instructions for XTheadVector for instructions that are in both 2) Use the ASM_OUTPUT_OPCODE hook to output "th." for these instructions No doubt, the ASM_OUTPUT_OPCODE hook approach is better than our format-string approach, but would 1) not be the even better solution? It would also mean, that not a single test case is required for these overlapping instructions (only a few tests that ensure that we don't emit RVV instructions that are not available in XTheadVector). Besides that, letting GCC emit RVV instructions for XTheadVector is a very clever idea, because it fully utilizes the fact that both extensions overlap to a huge degree. The ASM_OUTPUT_OPCODE approach could lead to an issue if we enable XTheadVector with any other vector extension, say Zvfoo. In this case the Zvfoo instructions will all be prefixed as well with "th.". I know that it is not likely to run into this problem (such a machine does not exist in real hardware), but it is possible to trigger this issue easily and approach 1) would not have this potential issue. Thanks, Christoph > > Thanks. > > > juzhe.zh...@rivai.ai > > > From: Christoph Müllner > Date: 2023-11-22 18:07 > To: juzhe.zh...@rivai.ai > CC: gcc-patches; kito.cheng; Kito.cheng; cooper.joshua; Robin Dapp; > jeffreyalaw; Philipp Tomsich; Cooper Qu; Jin Ma; Nelson Chu > Subject: Re: RISC-V: Support XTheadVector extensions > Hi Juzhe, > > Sorry for the late reply, but I was not on CC, so I missed this email. > > On Fri, Nov 17, 2023 at 2:41 PM juzhe.zh...@rivai.ai > wrote: > > > > Ok. I just read the theadvector extension. > > > > https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc > > > > Theadvector is not custom extension. Just a uarch to disable some of the > > RVV1.0 extension > > Theadvector can be considered as subextension of 'V' extension with > > disabling some of the > > instructions and adding some new thead vector target load/store (This is > > another story). > > > > So, for disabling the instruction that theadvector doesn't support. > > You don't need to touch such many codes. > > > > Here is a much simpler approach to do (I think it's definitely working): > > 1. Don't change any codes in vector.md and keep GCC generates ASM with > > "th." prefix. > > 2. Add !TARGET_THEADVECTOR into vector-iterator.md to disable the mode you > > don't want. > > For example , theadvector doesn't support fractional vector. > > > > Then it's pretty simple: > > > > RVVMF2SI "TARGET_VECTOR && !TARGET_THEADVECTOR". > > > > 3. Remove all the tests you add in this patch. > > 4. You can add theadvector specific load/store for example, th.vlb > > instructions they are allowed. > > 5. Modify binutils, and make th.vmulh.vv as the pseudo instruction of > > vmulh.vv > > 6. So with compile option "-S", you will still see ASM as "vmulh.vv". but > > with objdump, you will see th.vmulh.vv. > > Yes, all these points sound reasonable, to minimize the patchset size. > I believe in point 1 you meant "without th. prefix". > > I've added Jin Ma (who is the main author of the Binutils patchset) so > he is also aware > of the proposal to use pseudo instructions to avoid duplication in Binutils. > > Thank you very much! > Christoph > > > > > > After this change, you can send V2, then I can continue to review on GCC-15. > > > > Thanks. > > > > > > juzhe.zh...@rivai.ai > > > > > > From: juzhe.zh...@rivai.ai > > Date: 2023-11-17 19:39 > > To: gcc-patches > > CC: kito.cheng; kito.cheng; cooper.joshua; Robin Dapp; jeffreyalaw > > Subject: RISC-V: Support XTheadVector extensions > > 90% theadvector extension reusing current RVV 1.0 instructions patterns: > > Just change ASM, For example: > > > > @@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar" > > (match_operand:VFULLI_D 3 "regi
Re: Re: RISC-V: Support XTheadVector extensions
I am totally ok to approve theadvector on GCC-14 before stage 3 close
as long as it doesn't touch the current RVV codes too much and binutils
supports theadvector.
I have provided the draft approach:
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637349.html
which turns out doesn't need to change any codes of vector.md.
I strongly suggest follow this draft. I can be actively review theadvector
during stage 3.
And hopefully can help you land theadvector on GCC-14.
Thanks.
juzhe.zh...@rivai.ai
From: Christoph Müllner
Date: 2023-11-22 18:07
To: juzhe.zh...@rivai.ai
CC: gcc-patches; kito.cheng; Kito.cheng; cooper.joshua; Robin Dapp;
jeffreyalaw; Philipp Tomsich; Cooper Qu; Jin Ma; Nelson Chu
Subject: Re: RISC-V: Support XTheadVector extensions
Hi Juzhe,
Sorry for the late reply, but I was not on CC, so I missed this email.
On Fri, Nov 17, 2023 at 2:41 PM juzhe.zh...@rivai.ai
wrote:
>
> Ok. I just read the theadvector extension.
>
> https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
>
> Theadvector is not custom extension. Just a uarch to disable some of the
> RVV1.0 extension
> Theadvector can be considered as subextension of 'V' extension with disabling
> some of the
> instructions and adding some new thead vector target load/store (This is
> another story).
>
> So, for disabling the instruction that theadvector doesn't support.
> You don't need to touch such many codes.
>
> Here is a much simpler approach to do (I think it's definitely working):
> 1. Don't change any codes in vector.md and keep GCC generates ASM with "th."
> prefix.
> 2. Add !TARGET_THEADVECTOR into vector-iterator.md to disable the mode you
> don't want.
> For example , theadvector doesn't support fractional vector.
>
> Then it's pretty simple:
>
> RVVMF2SI "TARGET_VECTOR && !TARGET_THEADVECTOR".
>
> 3. Remove all the tests you add in this patch.
> 4. You can add theadvector specific load/store for example, th.vlb
> instructions they are allowed.
> 5. Modify binutils, and make th.vmulh.vv as the pseudo instruction of vmulh.vv
> 6. So with compile option "-S", you will still see ASM as "vmulh.vv". but
> with objdump, you will see th.vmulh.vv.
Yes, all these points sound reasonable, to minimize the patchset size.
I believe in point 1 you meant "without th. prefix".
I've added Jin Ma (who is the main author of the Binutils patchset) so
he is also aware
of the proposal to use pseudo instructions to avoid duplication in Binutils.
Thank you very much!
Christoph
>
> After this change, you can send V2, then I can continue to review on GCC-15.
>
> Thanks.
>
>
> juzhe.zh...@rivai.ai
>
>
> From: juzhe.zh...@rivai.ai
> Date: 2023-11-17 19:39
> To: gcc-patches
> CC: kito.cheng; kito.cheng; cooper.joshua; Robin Dapp; jeffreyalaw
> Subject: RISC-V: Support XTheadVector extensions
> 90% theadvector extension reusing current RVV 1.0 instructions patterns:
> Just change ASM, For example:
>
> @@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
> (match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")] VMULH)
>(match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
>"TARGET_VECTOR"
> - "vmulh.vx\t%0,%3,%z4%p1"
> + "%^vmulh.vx\t%0,%3,%z4%p1"
>[(set_attr "type" "vimul")
> (set_attr "mode" "")])
>
> + if (letter == '^')
> +{
> + if (TARGET_XTHEADVECTOR)
> + fputs ("th.", file);
> + return;
> +}
>
>
> For almost all patterns, you just simply append "th." in the ASM prefix.
> like change "vmulh.vv" -> "th.vmulh.vv"
>
> Almost all theadvector instructions are not new features, all same as RVV1.0.
> Why do you invent the such ISA doesn't include any features that RVV1.0
> doesn't satisfy ?
>
> I am not explicitly object this patch. But I should know the reason.
>
> Btw, stage 1 will close soon. So I will review this patch on GCC-15 as long
> as all other RISC-V maintainers agree.
>
>
>
> juzhe.zh...@rivai.ai
Re: RISC-V: Support XTheadVector extensions
Hi Juzhe,
Sorry for the late reply, but I was not on CC, so I missed this email.
On Fri, Nov 17, 2023 at 2:41 PM juzhe.zh...@rivai.ai
wrote:
>
> Ok. I just read the theadvector extension.
>
> https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
>
> Theadvector is not custom extension. Just a uarch to disable some of the
> RVV1.0 extension
> Theadvector can be considered as subextension of 'V' extension with disabling
> some of the
> instructions and adding some new thead vector target load/store (This is
> another story).
>
> So, for disabling the instruction that theadvector doesn't support.
> You don't need to touch such many codes.
>
> Here is a much simpler approach to do (I think it's definitely working):
> 1. Don't change any codes in vector.md and keep GCC generates ASM with "th."
> prefix.
> 2. Add !TARGET_THEADVECTOR into vector-iterator.md to disable the mode you
> don't want.
> For example , theadvector doesn't support fractional vector.
>
> Then it's pretty simple:
>
> RVVMF2SI "TARGET_VECTOR && !TARGET_THEADVECTOR".
>
> 3. Remove all the tests you add in this patch.
> 4. You can add theadvector specific load/store for example, th.vlb
> instructions they are allowed.
> 5. Modify binutils, and make th.vmulh.vv as the pseudo instruction of vmulh.vv
> 6. So with compile option "-S", you will still see ASM as "vmulh.vv". but
> with objdump, you will see th.vmulh.vv.
Yes, all these points sound reasonable, to minimize the patchset size.
I believe in point 1 you meant "without th. prefix".
I've added Jin Ma (who is the main author of the Binutils patchset) so
he is also aware
of the proposal to use pseudo instructions to avoid duplication in Binutils.
Thank you very much!
Christoph
>
> After this change, you can send V2, then I can continue to review on GCC-15.
>
> Thanks.
>
>
> juzhe.zh...@rivai.ai
>
>
> From: juzhe.zh...@rivai.ai
> Date: 2023-11-17 19:39
> To: gcc-patches
> CC: kito.cheng; kito.cheng; cooper.joshua; Robin Dapp; jeffreyalaw
> Subject: RISC-V: Support XTheadVector extensions
> 90% theadvector extension reusing current RVV 1.0 instructions patterns:
> Just change ASM, For example:
>
> @@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
> (match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")] VMULH)
>(match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
>"TARGET_VECTOR"
> - "vmulh.vx\t%0,%3,%z4%p1"
> + "%^vmulh.vx\t%0,%3,%z4%p1"
>[(set_attr "type" "vimul")
> (set_attr "mode" "")])
>
> + if (letter == '^')
> +{
> + if (TARGET_XTHEADVECTOR)
> + fputs ("th.", file);
> + return;
> +}
>
>
> For almost all patterns, you just simply append "th." in the ASM prefix.
> like change "vmulh.vv" -> "th.vmulh.vv"
>
> Almost all theadvector instructions are not new features, all same as RVV1.0.
> Why do you invent the such ISA doesn't include any features that RVV1.0
> doesn't satisfy ?
>
> I am not explicitly object this patch. But I should know the reason.
>
> Btw, stage 1 will close soon. So I will review this patch on GCC-15 as long
> as all other RISC-V maintainers agree.
>
>
>
> juzhe.zh...@rivai.ai
Re: Re: RISC-V: Support XTheadVector extensions
Adding Drew and Robert.
On Sun, Nov 19, 2023 at 10:04 PM juzhe.zh...@rivai.ai
wrote:
> As kito's suggestions. I just have a quick try.
>
> This patch should does following things:
>
> 1. Remove all new API that RVV1.0 doesn't have. E.g. vlb.
> They should be another separate patch to be reviewed.
> So the first series patch should be "Support part of theadvector API
> base on current RVV1.0 API"
>
> 2. Here is a another approach which must work for theadvector:
>
>diff --git a/gcc/config/riscv/riscv-protos.h
> b/gcc/config/riscv/riscv-protos.h
> index ae528db1898..24b514c58df 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -646,6 +646,7 @@ extern bool th_classify_address (struct
> riscv_address_info *,
> extern const char *th_output_move (rtx, rtx);
> extern bool th_print_operand_address (FILE *, machine_mode, rtx);
> #endif
> +extern void th_vector_asm_output_opcode (FILE *, const char *);
>
> extern bool riscv_use_divmod_expander (void);
> void riscv_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree, int);
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 3701f41b1b3..9631a428341 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -10088,6 +10088,13 @@ extract_base_offset_in_addr (rtx mem, rtx *base,
> rtx *offset)
>return false;
> }
>
> +void
> +th_vector_asm_output_opcode (FILE *f, const char *ptr)
> +{
> + if (ptr[0] == 'v')
> +fprintf (f, "th.");
> +}
> +
> /* Initialize the GCC target structure. */
> #undef TARGET_ASM_ALIGNED_HI_OP
> #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index 6205d7533f4..be02a926028 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -1206,4 +1206,6 @@ extern void riscv_remove_unneeded_save_restore_calls
> (void);
> #define HAVE_POST_MODIFY_DISP TARGET_XTHEADMEMIDX
> #define HAVE_PRE_MODIFY_DISP TARGET_XTHEADMEMIDX
>
> +#define ASM_OUTPUT_OPCODE(STREAM, PTR) th_vector_asm_output_opcode
> (STREAM, PTR);
> +
> #endif /* ! GCC_RISCV_H */
>
> It does work:
>
> /tmp/cc0yrKxw.s:1692: Error: unrecognized opcode `th.vsetivli
> zero,8,e8,mf2,ta,ma'
> /tmp/cc0yrKxw.s:1693: Error: unrecognized opcode `th.vmv.v.i v1,0'
> /tmp/cc0yrKxw.s:1694: Error: unrecognized opcode `th.vse8.v v1,0(a5)'
> /tmp/cc0yrKxw.s:1696: Error: unrecognized opcode `th.vse8.v v1,0(a5)'
> make[2]: *** [Makefile:935: _gcov.o] Error 1
> make[2]: *** Waiting for unfinished jobs
> /tmp/cc2KYYTs.s: Assembler messages:
> /tmp/cc2KYYTs.s:1606: Error: unrecognized opcode `th.vsetivli
> zero,8,e8,mf2,ta,ma'
> /tmp/cc2KYYTs.s:1610: Error: unrecognized opcode `th.vle8.v v1,0(a1)'
> /tmp/cc2KYYTs.s:1615: Error: unrecognized opcode `th.vse8.v v1,0(sp)'
> /tmp/cc2KYYTs.s:1617: Error: unrecognized opcode `th.vle8.v v1,0(a2)'
> /tmp/cc2KYYTs.s:1618: Error: unrecognized opcode `th.vse8.v v1,0(a5)'
> /tmp/cc2KYYTs.s:1651: Error: unrecognized opcode `th.vsetivli
> zero,8,e8,mf2,ta,ma'
> /tmp/cc2KYYTs.s:1671: Error: unrecognized opcode `th.vle8.v v1,0(a4)'
> /tmp/cc2KYYTs.s:1674: Error: unrecognized opcode `th.vse8.v v1,0(a0)'
> /tmp/cc2KYYTs.s:2469: Error: unrecognized opcode `th.vsetivli
> zero,8,e8,mf2,ta,ma'
> /tmp/cc2KYYTs.s:2569: Error: unrecognized opcode `th.vsetivli
> zero,8,e8,mf2,ta,ma'
> /tmp/cc2KYYTs.s:2580: Error: unrecognized opcode `th.vle8.v v1,0(a2)'
> /tmp/cc2KYYTs.s:2581: Error: unrecognized opcode `th.vse8.v v1,0(a5)'
> /tmp/cc2KYYTs.s:2643: Error: unrecognized opcode `th.vsetivli
> zero,8,e8,mf2,ta,ma'
> /tmp/cc2KYYTs.s:2671: Error: unrecognized opcode `th.vsetivli
> zero,8,e8,mf2,ta,ma'
> /tmp/cc2KYYTs.s:3294: Error: unrecognized opcode `th.vsetivli
> zero,8,e8,mf2,ta,ma'
> /tmp/cc2KYYTs.s:3317: Error: unrecognized opcode `th.vle8.v v1,0(a4)'
> /tmp/cc2KYYTs.s:3319: Error: unrecognized opcode `th.vse8.v v1,0(a4)'
> /tmp/cc2KYYTs.s:3322: Error: unrecognized opcode `th.vle8.v v1,0(a4)'
> /tmp/cc2KYYTs.s:3324: Error: unrecognized opcode `th.vse8.v v1,0(a4)'
>
> But we need binutils support theadvector first, otherwise, it will fail
> during building.
>
> 3. Add theadvector gating on target-support.exp. We don't want to run
> theadvector test
> when we don't enable theadvector.
>
> Thanks.
>
> --
> juzhe.zh...@rivai.ai
>
>
> *From:* Kito Cheng
> *Date:* 2023-11-18 18:32
> *To:* Philipp Tomsich
> *CC:* Jeff Law ; juzhe.zh...@rivai.ai; gcc-patches
> ; kito.cheng ;
> cooper.joshua ; Robin Dapp
> ; jkridner
> *Subject:* Re: RISC-V: Support XTheadVector extensions
> I g
Re: Re: RISC-V: Support XTheadVector extensions
As kito's suggestions. I just have a quick try.
This patch should does following things:
1. Remove all new API that RVV1.0 doesn't have. E.g. vlb.
They should be another separate patch to be reviewed.
So the first series patch should be "Support part of theadvector API base
on current RVV1.0 API"
2. Here is a another approach which must work for theadvector:
diff --git a/gcc/config/riscv/riscv-protos.h
b/gcc/config/riscv/riscv-protos.h
index ae528db1898..24b514c58df 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -646,6 +646,7 @@ extern bool th_classify_address (struct riscv_address_info
*,
extern const char *th_output_move (rtx, rtx);
extern bool th_print_operand_address (FILE *, machine_mode, rtx);
#endif
+extern void th_vector_asm_output_opcode (FILE *, const char *);
extern bool riscv_use_divmod_expander (void);
void riscv_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree, int);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 3701f41b1b3..9631a428341 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -10088,6 +10088,13 @@ extract_base_offset_in_addr (rtx mem, rtx *base, rtx
*offset)
return false;
}
+void
+th_vector_asm_output_opcode (FILE *f, const char *ptr)
+{
+ if (ptr[0] == 'v')
+fprintf (f, "th.");
+}
+
/* Initialize the GCC target structure. */
#undef TARGET_ASM_ALIGNED_HI_OP
#define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index 6205d7533f4..be02a926028 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -1206,4 +1206,6 @@ extern void riscv_remove_unneeded_save_restore_calls
(void);
#define HAVE_POST_MODIFY_DISP TARGET_XTHEADMEMIDX
#define HAVE_PRE_MODIFY_DISP TARGET_XTHEADMEMIDX
+#define ASM_OUTPUT_OPCODE(STREAM, PTR) th_vector_asm_output_opcode (STREAM,
PTR);
+
#endif /* ! GCC_RISCV_H */
It does work:
/tmp/cc0yrKxw.s:1692: Error: unrecognized opcode `th.vsetivli
zero,8,e8,mf2,ta,ma'
/tmp/cc0yrKxw.s:1693: Error: unrecognized opcode `th.vmv.v.i v1,0'
/tmp/cc0yrKxw.s:1694: Error: unrecognized opcode `th.vse8.v v1,0(a5)'
/tmp/cc0yrKxw.s:1696: Error: unrecognized opcode `th.vse8.v v1,0(a5)'
make[2]: *** [Makefile:935: _gcov.o] Error 1
make[2]: *** Waiting for unfinished jobs
/tmp/cc2KYYTs.s: Assembler messages:
/tmp/cc2KYYTs.s:1606: Error: unrecognized opcode `th.vsetivli
zero,8,e8,mf2,ta,ma'
/tmp/cc2KYYTs.s:1610: Error: unrecognized opcode `th.vle8.v v1,0(a1)'
/tmp/cc2KYYTs.s:1615: Error: unrecognized opcode `th.vse8.v v1,0(sp)'
/tmp/cc2KYYTs.s:1617: Error: unrecognized opcode `th.vle8.v v1,0(a2)'
/tmp/cc2KYYTs.s:1618: Error: unrecognized opcode `th.vse8.v v1,0(a5)'
/tmp/cc2KYYTs.s:1651: Error: unrecognized opcode `th.vsetivli
zero,8,e8,mf2,ta,ma'
/tmp/cc2KYYTs.s:1671: Error: unrecognized opcode `th.vle8.v v1,0(a4)'
/tmp/cc2KYYTs.s:1674: Error: unrecognized opcode `th.vse8.v v1,0(a0)'
/tmp/cc2KYYTs.s:2469: Error: unrecognized opcode `th.vsetivli
zero,8,e8,mf2,ta,ma'
/tmp/cc2KYYTs.s:2569: Error: unrecognized opcode `th.vsetivli
zero,8,e8,mf2,ta,ma'
/tmp/cc2KYYTs.s:2580: Error: unrecognized opcode `th.vle8.v v1,0(a2)'
/tmp/cc2KYYTs.s:2581: Error: unrecognized opcode `th.vse8.v v1,0(a5)'
/tmp/cc2KYYTs.s:2643: Error: unrecognized opcode `th.vsetivli
zero,8,e8,mf2,ta,ma'
/tmp/cc2KYYTs.s:2671: Error: unrecognized opcode `th.vsetivli
zero,8,e8,mf2,ta,ma'
/tmp/cc2KYYTs.s:3294: Error: unrecognized opcode `th.vsetivli
zero,8,e8,mf2,ta,ma'
/tmp/cc2KYYTs.s:3317: Error: unrecognized opcode `th.vle8.v v1,0(a4)'
/tmp/cc2KYYTs.s:3319: Error: unrecognized opcode `th.vse8.v v1,0(a4)'
/tmp/cc2KYYTs.s:3322: Error: unrecognized opcode `th.vle8.v v1,0(a4)'
/tmp/cc2KYYTs.s:3324: Error: unrecognized opcode `th.vse8.v v1,0(a4)'
But we need binutils support theadvector first, otherwise, it will fail during
building.
3. Add theadvector gating on target-support.exp. We don't want to run
theadvector test
when we don't enable theadvector.
Thanks.
juzhe.zh...@rivai.ai
From: Kito Cheng
Date: 2023-11-18 18:32
To: Philipp Tomsich
CC: Jeff Law; juzhe.zh...@rivai.ai; gcc-patches; kito.cheng; cooper.joshua;
Robin Dapp; jkridner
Subject: Re: RISC-V: Support XTheadVector extensions
I guess it would be worth to state my thought publicly:
I *support* adding the T-head vector (a.k.a. vector 0.7) to upstream
GCC since T-Head vector already ships a large enough number of boards,
also it's not really T-head's problem as Palmer described in another
mail.
My biggest concern before is T-head folks didn't involved into
community work too much, so accept that definitely will increasing
work for maintainers, however I saw T-head folks is trying to
contribute stuffs to upstream now, so may not a concern now, also I
believe accept this patch will encourage they work more on upstream
together, which is benefit to each other.
Back to the one of the biggest issues for t
Re: Re: RISC-V: Support XTheadVector extensions
Currently I start to work on full coverage testing (with different compile option test GCC testsuite) and fix bugs which is highest priority definitely. I am not able to find the time review this patch on GCC-14 for now. So conservatively, postpone it to GCC-15. If we are lucky that I stablize RVV support quickly, we still have a chance to make it landed on GCC-14. It all depends on my review. But no worry, I will review that eventually. juzhe.zh...@rivai.ai From: Kito Cheng Date: 2023-11-18 18:32 To: Philipp Tomsich CC: Jeff Law; juzhe.zh...@rivai.ai; gcc-patches; kito.cheng; cooper.joshua; Robin Dapp; jkridner Subject: Re: RISC-V: Support XTheadVector extensions I guess it would be worth to state my thought publicly: I *support* adding the T-head vector (a.k.a. vector 0.7) to upstream GCC since T-Head vector already ships a large enough number of boards, also it's not really T-head's problem as Palmer described in another mail. My biggest concern before is T-head folks didn't involved into community work too much, so accept that definitely will increasing work for maintainers, however I saw T-head folks is trying to contribute stuffs to upstream now, so may not a concern now, also I believe accept this patch will encourage they work more on upstream together, which is benefit to each other. Back to the one of the biggest issues for the patch set: GCC 14 or GCC 15. My general thought is it may be OK if it's less invasive enough, then should be OK for GCC 14, but I don't have a strong opinion, since as you know I am not the main developer of the vector part, so I will let Ju-Zhe make the final decision, because he is the one who contributes most things to RISC-V vector gcc support.
Re: RISC-V: Support XTheadVector extensions
I guess it would be worth to state my thought publicly: I *support* adding the T-head vector (a.k.a. vector 0.7) to upstream GCC since T-Head vector already ships a large enough number of boards, also it's not really T-head's problem as Palmer described in another mail. My biggest concern before is T-head folks didn't involved into community work too much, so accept that definitely will increasing work for maintainers, however I saw T-head folks is trying to contribute stuffs to upstream now, so may not a concern now, also I believe accept this patch will encourage they work more on upstream together, which is benefit to each other. Back to the one of the biggest issues for the patch set: GCC 14 or GCC 15. My general thought is it may be OK if it's less invasive enough, then should be OK for GCC 14, but I don't have a strong opinion, since as you know I am not the main developer of the vector part, so I will let Ju-Zhe make the final decision, because he is the one who contributes most things to RISC-V vector gcc support.
Re: RISC-V: Support XTheadVector extensions
On Fri, 17 Nov 2023 at 22:47, Jeff Law wrote:
>
>
>
> On 11/17/23 04:39, juzhe.zh...@rivai.ai wrote:
> > 90% theadvector extension reusing current RVV 1.0 instructions patterns:
> > Just change ASM, For example:
> >
> > @@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
> >(match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")]
> > VMULH)
> > (match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
> > "TARGET_VECTOR"
> > - "vmulh.vx\t%0,%3,%z4%p1"
> > + "%^vmulh.vx\t%0,%3,%z4%p1"
> > [(set_attr "type" "vimul")
> > (set_attr "mode" "")])
> >
> > + if (letter == '^')
> > +{
> > + if (TARGET_XTHEADVECTOR)
> > + fputs ("th.", file);
> > + return;
> > +}
> I assume this hunk is meant for riscv_output_operand in riscv.cc. We
> may also need to add '^' to the punct_valid_p hook. But yes, this is
> the preferred way to go when all we need to do is prefix the instruction
> with "th.".
>
>
> >
> > Btw, stage 1 will close soon. So I will review this patch on GCC-15 as
> > long as all other RISC-V maintainers agree.
> I *think* it's a gcc-15 issue. Philipp T. and I briefly spoke about
> this at the RVI summit a couple weeks back and he indicated the thead
> vector work was targeting gcc-15.
To restate the intent clearly:
- Getting this merged into GCC14 would be our most favored outcome, as
boards with XTheadV are quite common in the field: Allwinner D1,
BeagleBoard BeagleV-Ahead, Sophgo Milk-V;
- If that is not possible and we end up with an "ok for 15", we can
still resolve the downstream ecosystem issues (primarily felt by the
BeagleV-Ahead community) gracefully.
>From our brief discussion, I understood you thought it more realistic
to land this early into GCC15.
If we end up targeting GCC15, I would still like to achieve an
agreement on design early. This would allow our team to make any
needed changes and maintain them in a vendor-branch (on the GCC GIT
reposirty) until GCC15 opens up.
Philipp.
Re: RISC-V: Support XTheadVector extensions
On Fri, Nov 17, 2023 at 12:40 PM juzhe.zh...@rivai.ai
wrote:
>
> 90% theadvector extension reusing current RVV 1.0 instructions patterns:
> Just change ASM, For example:
>
> @@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
> (match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")] VMULH)
>(match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
>"TARGET_VECTOR"
> - "vmulh.vx\t%0,%3,%z4%p1"
> + "%^vmulh.vx\t%0,%3,%z4%p1"
>[(set_attr "type" "vimul")
> (set_attr "mode" "")])
>
> + if (letter == '^')
> +{
> + if (TARGET_XTHEADVECTOR)
> + fputs ("th.", file);
> + return;
> +}
>
>
> For almost all patterns, you just simply append "th." in the ASM prefix.
> like change "vmulh.vv" -> "th.vmulh.vv"
>
> Almost all theadvector instructions are not new features, all same as RVV1.0.
> Why do you invent the such ISA doesn't include any features that RVV1.0
> doesn't satisfy ?
>
> I am not explicitly object this patch. But I should know the reason.
Palmer already outlined the reason why this has been implemented in HW.
I want to add some comments on the specification and the design of the
SW support.
We have to face the fact here, that T-Head implemented the 0.7.1 draft
version of RVV (plus a VLENB CSR).
I don't want to waste time and discuss who is to blame for that (this
has been done elsewhere in enough detail).
Also, there are mechanisms now in place to avoid that something like
this happens again.
When we call this extension "RVV-0.7.1-draft" (plus VLENB), then we
are facing arguments that
claim that a RVV "draft" cannot be treated as a ratified extension.
Further, there are arguments
that only one RVV extension exists (the one that was ratified).
Therefore, T-Head's vector extension was
several times described as a "custom-extension", which is
"non-conforming" (uses encoding space
of standard extension). Of course, this hides the fact that the
extension is identical to RVV 1.0 to a high degree.
Anyway, I don't think that these arguments and descriptions are wrong.
So, in order to avoid pointless discussions about what it is, and why
it is what it is,
we simply accepted this description and gave the extension the name
XTheadVector.
In RISC-V vendor instructions and CSRs need to have vendor prefixes ("th.").
The specification can be found (together with all other XThead*
extensions) here:
https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
Some further details, which are worth mentioning here about this specification:
* Factional LMUL values are not supported.
* Zvlsseg was an extension in RVV 0.7.1, but is part in RVV 1.0.
Since T-Head has these instructions as well, we followed the RVV 1.0
idea and made
these instructions mandatory for XTheadVector (ie. avoiding
introduction of useless extensions).
* Zvamo was an extension in RVV 0.7.1, which was dropped in RVV 1.0.
Since T-Head has these instructions as well, we defined XTheadZvamo.
So the result is that we have a custom extension, which uses the RVI
encoding space
and which "by accident" has a huge overlap with RVV 1.0.
We are all fine with this, as long as this is our ticket to get the
extension supported upstream
(in the sense that everyone's opinions are respected and a solution is
found which
will not trigger useless discussions about things that happened a long
time ago).
The implementation follows this idea: it is a vendor extension and is
kept as separate
as possible from standard extensions. However, avoid duplication was
one of our most important
goals, so we came up with reusing the overlapping functionality by
just adding the instruction prefixes.
For the intrinsics API, we use a more user-friendly (pragmatic) approach:
* state the set of supported RVV intrinsic functions
* state the missing support of fractional LMUL values
* list the extension-specific intrinsic functions for the additional
load/store functionality
I hope this gives a good overview of our reasoning.
Let me know if you have further questions.
BR
Christoph
>
> Btw, stage 1 will close soon. So I will review this patch on GCC-15 as long
> as all other RISC-V maintainers agree.
>
>
>
> juzhe.zh...@rivai.ai
[PATCH v2 0/9] RISC-V: Support XTheadVector extensions
This patch series presents gcc implementation of the XTheadVector extension [1]. [1] https://github.com/T-head-Semi/thead-extension-spec/ I updated my patch series, because I forgot to add co-authors in the last version. Contributors: Jun Sha (Joshua) Jin Ma Christoph Müllner RISC-V: minimal support for xtheadvector RISC-V: Handle differences between xtheadvector and vector RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part1) RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part2) RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part3) RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part4) RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part5) RISC-V: Add support for xtheadvector-specific load/store intrinsics RISC-V: Disable fractional type intrinsics for XTheadVector --- gcc/common/config/riscv/riscv-common.cc | 10 + gcc/config.gcc| 2 +- gcc/config/riscv/riscv-c.cc | 8 +- gcc/config/riscv/riscv-protos.h | 1 + .../riscv/riscv-vector-builtins-bases.cc | 122 +++ .../riscv/riscv-vector-builtins-bases.h | 30 + .../riscv/riscv-vector-builtins-functions.def | 2 + .../riscv/riscv-vector-builtins-shapes.cc | 122 +++ .../riscv/riscv-vector-builtins-shapes.h | 2 + .../riscv/riscv-vector-builtins-types.def | 120 +++ gcc/config/riscv/riscv-vector-builtins.cc | 300 ++- gcc/config/riscv/riscv-vector-switch.def | 144 ++-- gcc/config/riscv/riscv.cc | 13 +- gcc/config/riscv/riscv.opt| 2 + gcc/config/riscv/riscv_th_vector.h| 49 ++ .../riscv/thead-vector-builtins-functions.def | 30 + gcc/config/riscv/thead-vector.md | 235 ++ gcc/config/riscv/vector-iterators.md | 4 + gcc/config/riscv/vector.md| 778 +- .../riscv/predef-__riscv_th_v_intrinsic.c | 11 + .../gcc.target/riscv/rvv/base/pragma-1.c | 2 +- .../gcc.target/riscv/rvv/fractional-type.c| 79 ++ .../gcc.target/riscv/rvv/xtheadvector.c | 13 + .../rvv/xtheadvector/autovec/vadd-run-nofm.c | 4 + .../riscv/rvv/xtheadvector/autovec/vadd-run.c | 81 ++ .../xtheadvector/autovec/vadd-rv32gcv-nofm.c | 10 + .../rvv/xtheadvector/autovec/vadd-rv32gcv.c | 8 + .../xtheadvector/autovec/vadd-rv64gcv-nofm.c | 10 + .../rvv/xtheadvector/autovec/vadd-rv64gcv.c | 8 + .../rvv/xtheadvector/autovec/vadd-template.h | 70 ++ .../rvv/xtheadvector/autovec/vadd-zvfh-run.c | 54 ++ .../riscv/rvv/xtheadvector/autovec/vand-run.c | 75 ++ .../rvv/xtheadvector/autovec/vand-rv32gcv.c | 7 + .../rvv/xtheadvector/autovec/vand-rv64gcv.c | 7 + .../rvv/xtheadvector/autovec/vand-template.h | 61 ++ .../rvv/xtheadvector/binop_vv_constraint-1.c | 68 ++ .../rvv/xtheadvector/binop_vv_constraint-3.c | 27 + .../rvv/xtheadvector/binop_vv_constraint-4.c | 27 + .../rvv/xtheadvector/binop_vv_constraint-5.c | 29 + .../rvv/xtheadvector/binop_vv_constraint-6.c | 28 + .../rvv/xtheadvector/binop_vv_constraint-7.c | 29 + .../rvv/xtheadvector/binop_vx_constraint-1.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-10.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-11.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-12.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-13.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-14.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-15.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-16.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-17.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-18.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-19.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-2.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-20.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-21.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-22.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-23.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-24.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-25.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-26.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-27.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-28.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-29.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-3.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-30.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-31.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-32.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-33.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-34.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-35.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-36.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-37.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-38.c | 68 ++
Re: Re: RISC-V: Support XTheadVector extensions
>> I suspect it's going to be even worse if you we have multiple patterns >> with the same underlying RTL, but just different output strings. No. We don't need to add (duplicate) any new patterns. I know RVV GCC very well. I know how to do that. juzhe.zh...@rivai.ai From: Jeff Law Date: 2023-11-18 08:01 To: 钟居哲; palmer CC: gcc-patches; kito.cheng; kito.cheng; cooper.joshua; rdapp.gcc Subject: Re: RISC-V: Support XTheadVector extensions On 11/17/23 16:16, 钟居哲 wrote: > >> I assume this hunk is meant for riscv_output_operand in riscv.cc. We >>>may also need to add '^' to the punct_valid_p hook. But yes, this is >>>the preferred way to go when all we need to do is prefix the instruction >>>with "th.". > > No. I don't think we need to add '^' . I don't want theadvector to touch > any codes > of vector.md. > Mixing up theadvector with RVV1.0 is a nighmare for RVV maintain. > People like me don't want to touch any thing related to Thead. > But anyway, I will take care of that in GCC-15. I suspect it's going to be even worse if you we have multiple patterns with the same underlying RTL, but just different output strings. The standard way to handle that has been with an output modifier and/or ASSEMBLER_DIALECT. If you look at the PA port for example, the assembler syntax changed dramatically between the PA1.0/PA1.1 era and the PA2.0 era. But we support both variants trivially without duplicating all the patterns. But we've got time to sort this out. I don't think the code in question was targeted towards gcc-14. jeff
Re: RISC-V: Support XTheadVector extensions
On 11/17/23 16:16, 钟居哲 wrote: >> I assume this hunk is meant for riscv_output_operand in riscv.cc. We may also need to add '^' to the punct_valid_p hook. But yes, this is the preferred way to go when all we need to do is prefix the instruction with "th.". No. I don't think we need to add '^' . I don't want theadvector to touch any codes of vector.md. Mixing up theadvector with RVV1.0 is a nighmare for RVV maintain. People like me don't want to touch any thing related to Thead. But anyway, I will take care of that in GCC-15. I suspect it's going to be even worse if you we have multiple patterns with the same underlying RTL, but just different output strings. The standard way to handle that has been with an output modifier and/or ASSEMBLER_DIALECT. If you look at the PA port for example, the assembler syntax changed dramatically between the PA1.0/PA1.1 era and the PA2.0 era. But we support both variants trivially without duplicating all the patterns. But we've got time to sort this out. I don't think the code in question was targeted towards gcc-14. jeff
Re: Re: RISC-V: Support XTheadVector extensions
>> I assume this hunk is meant for riscv_output_operand in riscv.cc. We
>> may also need to add '^' to the punct_valid_p hook. But yes, this is
>> the preferred way to go when all we need to do is prefix the instruction
>> with "th.".
No. I don't think we need to add '^' . I don't want theadvector to touch any
codes
of vector.md.
Mixing up theadvector with RVV1.0 is a nighmare for RVV maintain.
People like me don't want to touch any thing related to Thead.
But anyway, I will take care of that in GCC-15.
juzhe.zh...@rivai.ai
From: Palmer Dabbelt
Date: 2023-11-18 01:11
To: juzhe.zhong
CC: gcc-patches; Kito Cheng; kito.cheng; cooper.joshua; rdapp.gcc; jeffreyalaw
Subject: Re: RISC-V: Support XTheadVector extensions
On Fri, 17 Nov 2023 03:39:48 PST (-0800), juzhe.zh...@rivai.ai wrote:
> 90% theadvector extension reusing current RVV 1.0 instructions patterns:
> Just change ASM, For example:
>
> @@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
> (match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")] VMULH)
>(match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
>"TARGET_VECTOR"
> - "vmulh.vx\t%0,%3,%z4%p1"
> + "%^vmulh.vx\t%0,%3,%z4%p1"
>[(set_attr "type" "vimul")
> (set_attr "mode" "")])
> + if (letter == '^')
> +{
> + if (TARGET_XTHEADVECTOR)
> + fputs ("th.", file);
> + return;
> +}
>
> For almost all patterns, you just simply append "th." in the ASM prefix.
> like change "vmulh.vv" -> "th.vmulh.vv"
>
> Almost all theadvector instructions are not new features, all same as RVV1.0.
> Why do you invent the such ISA doesn't include any features that RVV1.0
> doesn't satisfy ?
>
> I am not explicitly object this patch. But I should know the reason.
There's some more in the later threads, but with the top posting it kind
of got lost so I'm just replying here.
This really isn't T-Head's fault: we announced V-0.7 as a stable draft
that was being implemented, and then T-Head went and implemented it.
Most of that history has been scrubbed by RVI, but you can still find
some stuff like this old talk on YouTube
<https://www.youtube.com/watch?v=F66F1nT1T8o>.
In general we've just figured out a way to make things work when HW
vendors end up in a grey area in RISC-V land. That obviously results in
a bunch of pain for the SW people, but this stuff is only useful if we
can run on real HW and that always involves some amount of pain.
Hopefully we can get to a point where we make fewer problems for
ourselves, but we've got a long history to dig out from and there's
going to be a lot more of this in the future.
So I don't like this XTHeadV stuff, but I think we're best to take it:
these guys tried to do the right thing and got thrown under the bus by
RVI, we should help them. This is almost certainly going to be a lot
more pain that we're used to, just given the size of the extensions in
question, but I still think it's the right way to go.
The other option is to essentially just tell them to fork the ISA, which
isn't good for anyone.
> Btw, stage 1 will close soon. So I will review this patch on GCC-15 as long
> as all other RISC-V maintainers agree.
I agree this is gcc-15 material: there's a lot of subtle differences in
behavior between 0.7 and 1.0, even when the mnemonics are the same.
We're already pretty buried in testing for 14, so trying to pick up
another target is going to be a huge headache (particularly one that's a
bit special).
>
>
>
>
> juzhe.zh...@rivai.ai
Re: RISC-V: Support XTheadVector extensions
On Fri, 17 Nov 2023 03:39:48 PST (-0800), juzhe.zh...@rivai.ai wrote:
90% theadvector extension reusing current RVV 1.0 instructions patterns:
Just change ASM, For example:
@@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
(match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")]
VMULH)
(match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
"TARGET_VECTOR"
- "vmulh.vx\t%0,%3,%z4%p1"
+ "%^vmulh.vx\t%0,%3,%z4%p1"
[(set_attr "type" "vimul")
(set_attr "mode" "")])
+ if (letter == '^')
+{
+ if (TARGET_XTHEADVECTOR)
+ fputs ("th.", file);
+ return;
+}
For almost all patterns, you just simply append "th." in the ASM prefix.
like change "vmulh.vv" -> "th.vmulh.vv"
Almost all theadvector instructions are not new features, all same as RVV1.0.
Why do you invent the such ISA doesn't include any features that RVV1.0 doesn't
satisfy ?
I am not explicitly object this patch. But I should know the reason.
There's some more in the later threads, but with the top posting it kind
of got lost so I'm just replying here.
This really isn't T-Head's fault: we announced V-0.7 as a stable draft
that was being implemented, and then T-Head went and implemented it.
Most of that history has been scrubbed by RVI, but you can still find
some stuff like this old talk on YouTube
.
In general we've just figured out a way to make things work when HW
vendors end up in a grey area in RISC-V land. That obviously results in
a bunch of pain for the SW people, but this stuff is only useful if we
can run on real HW and that always involves some amount of pain.
Hopefully we can get to a point where we make fewer problems for
ourselves, but we've got a long history to dig out from and there's
going to be a lot more of this in the future.
So I don't like this XTHeadV stuff, but I think we're best to take it:
these guys tried to do the right thing and got thrown under the bus by
RVI, we should help them. This is almost certainly going to be a lot
more pain that we're used to, just given the size of the extensions in
question, but I still think it's the right way to go.
The other option is to essentially just tell them to fork the ISA, which
isn't good for anyone.
Btw, stage 1 will close soon. So I will review this patch on GCC-15 as long as
all other RISC-V maintainers agree.
I agree this is gcc-15 material: there's a lot of subtle differences in
behavior between 0.7 and 1.0, even when the mnemonics are the same.
We're already pretty buried in testing for 14, so trying to pick up
another target is going to be a huge headache (particularly one that's a
bit special).
juzhe.zh...@rivai.ai
Re: RISC-V: Support XTheadVector extensions
On 11/17/23 04:39, juzhe.zh...@rivai.ai wrote:
90% theadvector extension reusing current RVV 1.0 instructions patterns:
Just change ASM, For example:
@@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
(match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")]
VMULH)
(match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
"TARGET_VECTOR"
- "vmulh.vx\t%0,%3,%z4%p1"
+ "%^vmulh.vx\t%0,%3,%z4%p1"
[(set_attr "type" "vimul")
(set_attr "mode" "")])
+ if (letter == '^')
+{
+ if (TARGET_XTHEADVECTOR)
+ fputs ("th.", file);
+ return;
+}
I assume this hunk is meant for riscv_output_operand in riscv.cc. We
may also need to add '^' to the punct_valid_p hook. But yes, this is
the preferred way to go when all we need to do is prefix the instruction
with "th.".
Btw, stage 1 will close soon. So I will review this patch on GCC-15 as
long as all other RISC-V maintainers agree.
I *think* it's a gcc-15 issue. Philipp T. and I briefly spoke about
this at the RVI summit a couple weeks back and he indicated the thead
vector work was targeting gcc-15.
Jeff
Re: RISC-V: Support XTheadVector extensions
Ok. I just read the theadvector extension.
https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
Theadvector is not custom extension. Just a uarch to disable some of the RVV1.0
extension
Theadvector can be considered as subextension of 'V' extension with disabling
some of the
instructions and adding some new thead vector target load/store (This is
another story).
So, for disabling the instruction that theadvector doesn't support.
You don't need to touch such many codes.
Here is a much simpler approach to do (I think it's definitely working):
1. Don't change any codes in vector.md and keep GCC generates ASM with "th."
prefix.
2. Add !TARGET_THEADVECTOR into vector-iterator.md to disable the mode you
don't want.
For example , theadvector doesn't support fractional vector.
Then it's pretty simple:
RVVMF2SI "TARGET_VECTOR && !TARGET_THEADVECTOR".
3. Remove all the tests you add in this patch.
4. You can add theadvector specific load/store for example, th.vlb instructions
they are allowed.
5. Modify binutils, and make th.vmulh.vv as the pseudo instruction of vmulh.vv
6. So with compile option "-S", you will still see ASM as "vmulh.vv". but with
objdump, you will see th.vmulh.vv.
After this change, you can send V2, then I can continue to review on GCC-15.
Thanks.
juzhe.zh...@rivai.ai
From: juzhe.zh...@rivai.ai
Date: 2023-11-17 19:39
To: gcc-patches
CC: kito.cheng; kito.cheng; cooper.joshua; Robin Dapp; jeffreyalaw
Subject: RISC-V: Support XTheadVector extensions
90% theadvector extension reusing current RVV 1.0 instructions patterns:
Just change ASM, For example:
@@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
(match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")]
VMULH)
(match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
"TARGET_VECTOR"
- "vmulh.vx\t%0,%3,%z4%p1"
+ "%^vmulh.vx\t%0,%3,%z4%p1"
[(set_attr "type" "vimul")
(set_attr "mode" "")])
+ if (letter == '^')
+{
+ if (TARGET_XTHEADVECTOR)
+ fputs ("th.", file);
+ return;
+}
For almost all patterns, you just simply append "th." in the ASM prefix.
like change "vmulh.vv" -> "th.vmulh.vv"
Almost all theadvector instructions are not new features, all same as RVV1.0.
Why do you invent the such ISA doesn't include any features that RVV1.0 doesn't
satisfy ?
I am not explicitly object this patch. But I should know the reason.
Btw, stage 1 will close soon. So I will review this patch on GCC-15 as long as
all other RISC-V maintainers agree.
juzhe.zh...@rivai.ai
RISC-V: Support XTheadVector extensions
90% theadvector extension reusing current RVV 1.0 instructions patterns:
Just change ASM, For example:
@@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh_scalar"
(match_operand:VFULLI_D 3 "register_operand" "vr,vr, vr, vr")]
VMULH)
(match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu, 0")))]
"TARGET_VECTOR"
- "vmulh.vx\t%0,%3,%z4%p1"
+ "%^vmulh.vx\t%0,%3,%z4%p1"
[(set_attr "type" "vimul")
(set_attr "mode" "")])
+ if (letter == '^')
+{
+ if (TARGET_XTHEADVECTOR)
+ fputs ("th.", file);
+ return;
+}
For almost all patterns, you just simply append "th." in the ASM prefix.
like change "vmulh.vv" -> "th.vmulh.vv"
Almost all theadvector instructions are not new features, all same as RVV1.0.
Why do you invent the such ISA doesn't include any features that RVV1.0 doesn't
satisfy ?
I am not explicitly object this patch. But I should know the reason.
Btw, stage 1 will close soon. So I will review this patch on GCC-15 as long as
all other RISC-V maintainers agree.
juzhe.zh...@rivai.ai
[PATCH 0/9] RISC-V: Support XTheadVector extensions
This patch series presents gcc implementation of the XTheadVector extension [1]. [1] https://github.com/T-head-Semi/thead-extension-spec/ Contributors: Jun Sha (Joshua) Jin Ma RISC-V: minimal support for xtheadvector RISC-V: Handle differences between xtheadvector and vector RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part1) RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part2) RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part3) RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part4) RISC-V: Tests for overlapping RVV and XTheadVector instructions (Part5) RISC-V: Add support for xtheadvector-specific load/store intrinsics RISC-V: Disable fractional type intrinsics for XTheadVector --- gcc/common/config/riscv/riscv-common.cc | 10 + gcc/config.gcc| 2 +- gcc/config/riscv/riscv-c.cc | 8 +- gcc/config/riscv/riscv-protos.h | 1 + .../riscv/riscv-vector-builtins-bases.cc | 122 +++ .../riscv/riscv-vector-builtins-bases.h | 30 + .../riscv/riscv-vector-builtins-functions.def | 2 + .../riscv/riscv-vector-builtins-shapes.cc | 122 +++ .../riscv/riscv-vector-builtins-shapes.h | 2 + .../riscv/riscv-vector-builtins-types.def | 120 +++ gcc/config/riscv/riscv-vector-builtins.cc | 300 ++- gcc/config/riscv/riscv-vector-switch.def | 144 ++-- gcc/config/riscv/riscv.cc | 13 +- gcc/config/riscv/riscv.opt| 2 + gcc/config/riscv/riscv_th_vector.h| 49 ++ .../riscv/thead-vector-builtins-functions.def | 30 + gcc/config/riscv/thead-vector.md | 235 ++ gcc/config/riscv/vector-iterators.md | 4 + gcc/config/riscv/vector.md| 778 +- .../riscv/predef-__riscv_th_v_intrinsic.c | 11 + .../gcc.target/riscv/rvv/base/pragma-1.c | 2 +- .../gcc.target/riscv/rvv/fractional-type.c| 79 ++ .../gcc.target/riscv/rvv/xtheadvector.c | 13 + .../rvv/xtheadvector/autovec/vadd-run-nofm.c | 4 + .../riscv/rvv/xtheadvector/autovec/vadd-run.c | 81 ++ .../xtheadvector/autovec/vadd-rv32gcv-nofm.c | 10 + .../rvv/xtheadvector/autovec/vadd-rv32gcv.c | 8 + .../xtheadvector/autovec/vadd-rv64gcv-nofm.c | 10 + .../rvv/xtheadvector/autovec/vadd-rv64gcv.c | 8 + .../rvv/xtheadvector/autovec/vadd-template.h | 70 ++ .../rvv/xtheadvector/autovec/vadd-zvfh-run.c | 54 ++ .../riscv/rvv/xtheadvector/autovec/vand-run.c | 75 ++ .../rvv/xtheadvector/autovec/vand-rv32gcv.c | 7 + .../rvv/xtheadvector/autovec/vand-rv64gcv.c | 7 + .../rvv/xtheadvector/autovec/vand-template.h | 61 ++ .../rvv/xtheadvector/binop_vv_constraint-1.c | 68 ++ .../rvv/xtheadvector/binop_vv_constraint-3.c | 27 + .../rvv/xtheadvector/binop_vv_constraint-4.c | 27 + .../rvv/xtheadvector/binop_vv_constraint-5.c | 29 + .../rvv/xtheadvector/binop_vv_constraint-6.c | 28 + .../rvv/xtheadvector/binop_vv_constraint-7.c | 29 + .../rvv/xtheadvector/binop_vx_constraint-1.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-10.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-11.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-12.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-13.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-14.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-15.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-16.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-17.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-18.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-19.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-2.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-20.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-21.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-22.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-23.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-24.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-25.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-26.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-27.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-28.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-29.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-3.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-30.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-31.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-32.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-33.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-34.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-35.c | 73 ++ .../rvv/xtheadvector/binop_vx_constraint-36.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-37.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-38.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-39.c | 68 ++ .../rvv/xtheadvector/binop_vx_constraint-4.c | 73 ++
