<div class="__aliyun_email_body_block"><div style="line-height:1.7;font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;"><div style="clear:both;">Hi Felix,</div><div style="clear:both;"><br ></div><div style="clear:both;">Nice to reach a consensus!</div><div style="clear:both;"><br ></div><div style="clear:both;">Seems loom's backend code only needs to adapt with the post-call nop. So they are <span >orthogonal with just a little overlapping. Therefore, whichever merged secondly could do that minor adjustment based on the former one.</span></div><div style="clear:both;"><br ></div><div style="clear:both;">(Sorry for the late reply for being on holiday without opening my mailbox)</div><div style="clear:both;"><br ></div><div style="clear:both;">Regards,</div><div style="clear:both;">Xiaolin</div><div style="clear:both;"><br /></div><blockquote style="margin-right:0;margin-top:0;margin-bottom:0;font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;"><div style="clear:both;">------------------------------------------------------------------</div><div style="clear:both;">From:yangfei <yangfei@iscas.ac.cn></div><div style="clear:both;">Send Time:2022年10月4日(星期二) 15:39</div><div style="clear:both;">To:郑孝林(云矅) <yunyao.zxl@alibaba-inc.com></div><div style="clear:both;">Cc:riscv-port-dev <riscv-port-dev@openjdk.org></div><div style="clear:both;">Subject:Re: Re: Discuss the RVC implementation</div><div style="clear:both;"><br /></div><span style="font-family:Arial;">Hi Xiaolin,</span><br >
<span style="font-family:Arial;"><br >
</span><br >
<span style="font-family:Arial;">Thanks for the thorough consierations. Comments inlined.</span><br >
<span style="font-family:Arial;"><br >
</span><br >
<span style="font-family:Arial;">> Hi Felix,</span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">> Thank you for taking the time to consider this, and the discussions.</span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">> I think it's certainly a fairly good observation, regarding the three versions that can theoretically cover any case in combination, in an instruction-level granularity. But in reality, I may have some of my personal practices to share: such might be too fine-grained to implement a high-level control, please let me explain it.</span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">> Let alone correctness, there are also code styles and maintenance that we have to focus on for sure. For example, if we want to rewrite one piece of code[1] with a fixed length by removing the `IncompressibleRegion` thing, to an instruction-level granularity, it might become [2]. Please see my comments in that gist.</span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">> 1. From the code style aspect:</span><br >
<span style="font-family:Arial;">> We can see it is not looking so promising. In fact, my RVC prototype was in exactly the same way as your thought (so I guess it might be an intuitive and general thought :-) ), in an instruction-level granularity. And I sadly found the code style was messy even to myself. We have to overload lots of things such as _ld(Register, Address), _ld(Register, address), (see my comments) and so on to fulfill any usage in an incompressible piece of code: the overall API changes (like _ld in any form) are not convergent.</span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">> In the comments from the gist, we can see we certainly have to make incompressible all the callees, even the callees of the callees, and so on, in a transitive relation. For example, the 'la(Register, Address)' API itself must be incompressible if we are in an instruction granularity. So we have to make its callee, 'la(Register, address)' API incompressible as well, and so on. It might be indeed an inferno...</span><br >
<span style="font-family:Arial;"><br >
</span><br >
<span style="font-family:Arial;">Yes, I think the 'la' case here already demonstrates the complexity of my proposal. I agree an 'IncompressibleRegion' mark would be simpler and easier for the developers.</span><br >
<span style="font-family:Arial;"><br >
</span><br >
<span style="font-family:Arial;">> 2. From the compression rate aspect:</span><br >
<span style="font-family:Arial;">> Besides, we are just talking about la() here. If we directly mark la()s as incompressible, then the la()s called by actually safe and compressible code will be left as incompressible forever. The compression rate will be definitely lower: the main issue here is, of course, the granularity problem -- instruction-level granularity is too fine-grained, which cannot allow us to make high-level controls.</span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">> The current `CompressibleRegion` combined with `IncompressibleRegion` can implement a function-level granularity (neither too fine nor too coarse), which I think is very suitable for the current backend, that we can use them combined to mark everything without many efforts and with a concentration (like the current implementation: the unified relocate() with a lambda[3] and an IncompressibleRegion hidden inside). With them both, we can avoid the above problems with no effort, please see the first line of [1]: the incompressible region directly controls the current function, marking THE 'la' it currently uses incompressible, without affecting the 'la' definitions themselves (movptr, ld ... are as well). So we can avoid lots of invasions to the current backend code base. Nice, right?</span><br >
<span style="font-family:Arial;"><br >
</span><br >
<span style="font-family:Arial;">I have went through your local changes about unified relocate() with a lambda. And I think it looks better and we can go on with this solution if no objections.</span><br >
<span style="font-family:Arial;"><br >
</span><br >
<span style="font-family:Arial;">> 3. From the maintenance aspect:</span><br >
<span style="font-family:Arial;">Explicitly adding '_' to every compressible instruction might be a burden for developers and porters. One may say, just adding some '_'s, why burdens? In fact, considering we are porting code like [1] again from AArch64 port. We not only have to change instructions to RISC-V's, but also have to consider RVC... does one instruction have '_' or not? Do its callees, even its callees' callees, have an incompressible version? Even if to myself, it might be a heavy burden :-) I might feel very troublesome - I may just want to ctrl+c and ctrl+v some code without other confusion. So, why not directly throw an `IncompressionRegion` to that stub with a fixed length, so that programmers can normally write their code with the normal "ld", "la" and "addi"? Everything is easily solved without caring for the trifling :-)</span><br >
<span style="font-family:Arial;"><br >
</span><br >
<span style="font-family:Arial;">That makes sense :-)</span><br >
<span style="font-family:Arial;"><br >
</span><br >
<span style="font-family:Arial;">> Just sharing some practices from the same thought and might be verbose again -- there are things not easy to foresee at a glance. When implementing, the pitfalls might be obvious then. From my personal perspective, I may consider the CompressibleRegions plan looks better though, and I am looking forward to your views and suggestions.</span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">> Best,</span><br >
<span style="font-family:Arial;">> Xiaolin</span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">></span><br >
<span style="font-family:Arial;">> [1] <a href="https://github.com/zhengxiaolinX/jdk/blob/2ee3204ace5a7767482819be2240982cc0744f8c/src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp#L196-L275" target="_blank">https://github.com/zhengxiaolinX/jdk/blob/2ee3204ace5a7767482819be2240982cc0744f8c/src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp#L196-L275</a></span><br >
<span style="font-family:Arial;">> [2] <a href="https://gist.github.com/zhengxiaolinX/3151db356a9001f58827d272c8330bb7" target="_blank">https://gist.github.com/zhengxiaolinX/3151db356a9001f58827d272c8330bb7</a></span><br >
<span style="font-family:Arial;">> [3] <a href="https://github.com/zhengxiaolinX/jdk/blob/2ee3204ace5a7767482819be2240982cc0744f8c/src/hotspot/cpu/riscv/assembler_riscv.hpp#L2167-L2178" target="_blank">https://github.com/zhengxiaolinX/jdk/blob/2ee3204ace5a7767482819be2240982cc0744f8c/src/hotspot/cpu/riscv/assembler_riscv.hpp#L2167-L2178</a></span><br >
<br ></blockquote></div></div>