riscv-software-src
add support for cbo.zero in CMO extensions
The zformat encoding does not have an imm_val field. The operation performed in the macro is right, but it should be based on rs1_val and not imm_val. The fix is just to replace imm_val with rs1_val and everything will work similarly. For completeness, the filed rs1_val_data should also be defined in the template.yaml for the instruction even if it is not being used (just so that a change in the cgf does not break CTG).
The reason I select imm_val instead of rs1_val is that rs1 is the whole address of cbo.zero, it doesn't have offset field to adjust the address to signature address. So rs1_val is relative stable, and what can be changed is only the offset in the cache block size (currently this is two time of xlen)
The
zformatencoding does not have animm_valfield. The operation performed in the macro is right, but it should be based onrs1_valand notimm_val. The fix is just to replaceimm_valwithrs1_valand everything will work similarly. For completeness, the filedrs1_val_datashould also be defined in thetemplate.yamlfor the instruction even if it is not being used (just so that a change in the cgf does not break CTG).
I have one question: is there anly way to make the signature data length for one instruction variable? The cache block size may be variable. I only found the stride field will affect the length. However it will break CTG if we want to change this.
The reason I select imm_val instead of rs1_val is that rs1 is the whole address of cbo.zero, it doesn't have offset field to adjust the address to signature address. So rs1_val is relative stable, and what can be changed is only the offset in the cache block size (currently this is two time of xlen)
Having it has imm_val will be confusing and architecturally incorrect for the zformat instructions. Its okay to use a relative value in rs1 i.e some post processing after the values are generated before the instruction uses it. It is always good to use only architecturally relevant fields in the source side as the same format may be applicable for multiple instructions. How the values are interpreted are controlled by the domain of each of the variables and the assembly macro itself.
The reason I select imm_val instead of rs1_val is that rs1 is the whole address of cbo.zero, it doesn't have offset field to adjust the address to signature address. So rs1_val is relative stable, and what can be changed is only the offset in the cache block size (currently this is two time of xlen)
Having it has
imm_valwill be confusing and architecturally incorrect for thezformatinstructions. Its okay to use a relative value in rs1 i.e some post processing after the values are generated before the instruction uses it. It is always good to use only architecturally relevant fields in the source side as the same format may be applicable for multiple instructions. How the values are interpreted are controlled by the domain of each of the variables and the assembly macro itself.
OK, I'll try to modify it.
I don't think performing the cbo.zero at some location and then loading the value from there to store it back into the signature region is really an efficient testing strategy. I think a parameter is needed in the yaml to configure the model, but a dynamic configuration of the test is not necessary. The value in
rs1is treated as the effective address which identifies a cache block. Hence the test should ideally test if non-zero bits in the LSB do identify the cache block correctly i.e the block index is the value over which coverpoints should be defined. To do so, we can assume a reasonably high cache block size(say 64 bytes) and write tests accordingly. The test will work without problems for any block size under 64 bytes.
@pawks Hi, Pawks. I don't quite understand how test work for any block size under 64 bytes. I can add blocksz field to every instruction in template.yaml, generate test cases for cbo.zero with different offset under 64 and directly zero the signature data with that size for test case. However, in this way, every size under 64 will generate different signature data for this generated test. Do I misunderstand anything?
I don't think a dynamic test is necessary, and yes, a different signature will be generated if a model is configured for different cacheblock sizes. This kind of issue is why the riscof framework was developed; it will run the tests with a reference model configured for whatever blocksize you want and compare it a vendor implementation. Dynamically configuring the test will make the test run faster, (e.g. it could avoid testing offset between 32..63 if the blocksize is 32f
- but if you didn't do that, the tests will still work, just be redundant.
On Tue, Jan 11, 2022 at 8:53 PM liweiwei90 @.***> wrote:
I don't think performing the cbo.zero at some location and then loading the value from there to store it back into the signature region is really an efficient testing strategy. I think a parameter is needed in the yaml to configure the model, but a dynamic configuration of the test is not necessary. The value in rs1 is treated as the effective address which identifies a cache block. Hence the test should ideally test if non-zero bits in the LSB do identify the cache block correctly i.e the block index is the value over which coverpoints should be defined. To do so, we can assume a reasonably high cache block size(say 64 bytes) and write tests accordingly. The test will work without problems for any block size under 64 bytes.
@pawks https://github.com/pawks Hi, Pawks. I don't quite understand how test work for any block size under 64 bytes. I can add blocksz field to every instruction in template.yaml, generate test cases for cbo.zero with different offset under 64 and directly zero the signature data with that size for test case. However, in this way, every size under 64 will generate different signature data for this generated test. Do I misunderstand anything?
— Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/22#issuecomment-1010633901, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJVYREJ22MMC22NGJDDUVUCNJANCNFSM5KKFIH2A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you are subscribed to this thread.Message ID: @.***>
if the blocksize is 32f
OK. I am not familiar with riscof currently. It seems not to transfer something to riscv-ctg. So what we should do is just generate a non-dynamic test with max-block-size support ? such as 4096B(it cannot cover pages)?
I don't find the question that triggers the test/build error. It works successfully in my local test.
These tests have been merged into riscv-arch-test. When I tried using them, I had to modify the RV_TEST_ISA and check ISA strings for cbo.zero in the rv32i_m/CMO and rv64i_m/CMO directories for riscof to accept them. I submitted a riscv-arch-test PR419 reflecting these changes, but it would be good to also fix the .cgf in this PR.
-RVTEST_ISA("RV32IZicbozZicsr") +RVTEST_ISA("RV32IZicsr_Zicboz") -RVTEST_CASE(0,"//check ISA:=regex(.*I.*Zicboz.Zicsr.);def TEST_CASE_1=True;",cbozero) +RVTEST_CASE(0,"//check ISA:=regex(.*32.*I.*Zicsr.Zicboz.);def TEST_CASE_1=True;",cbozero)
@liweiwei90 this PR is too old to be merged, it has many conflicts and versioning issues. Also it needs minor revisions too. Can you please add the changes proposed by @davidharrishmc, resolve the conflicts, and setup the correct version please? As you do these things, I will merge this one into master.
