March 2021 Bump

[zitaofang]

Part of ucb-bar/chipyard#837.

[a0u]

Could you explain why renaming vsetvl to hvsetvl is needed this time? The collision with the V extension has not been problematic before unless one is attempting to mix Hwacha and RVV code, as the -march string should be sufficient to disambiguate which instruction the assembler ought to emit in each situation.

While there is a naming conflict with the generated MATCH_VSETVL/MASK_VSETVL macros, this can be fixed in esp-opcodes and is orthogonal to what mnemonic the assembler accepts and what the disassemblers from binutils and spike print.

I'm fine with this change if @colinschmidt is, but we should not be breaking source compatibility if it is not functionally necessary.

[zitaofang]

Could you explain why renaming vsetvl to hvsetvl is needed this time? The collision with the V extension has not been problematic before unless one is attempting to mix Hwacha and RVV code, as the -march string should be sufficient to disambiguate which instruction the assembler ought to emit in each situation.

While there is a naming conflict with the generated MATCH_VSETVL/MASK_VSETVL macros, this can be fixed in esp-opcodes and is orthogonal to what mnemonic the assembler accepts and what the disassemblers from binutils and spike print.

I'm fine with this change if @colinschmidt is, but we should not be breaking source compatibility if it is not functionally necessary.

Actually the conflicting names are creating problem in Spike. The vsetvl instruction definition in hwacha/opcodes_hwacha.h in that repo is causing the build system to override the upstream (RVV) implementation during build if we don't change the name. The build system will generate a source file containing the handler for every instruction, and Hwacha's vsetvl.c will overwrite RVV's implementation, causing link errors. If you want to reproduce this error, check out this commit: ucb-bar/esp-isa-sim@1969ab2.

I mentioned this to @colinschmidt in Slack DM back in March, and he said we can rename the instruction to solve this conflict and try to control the impact on other sources file. And I think if we are changing instruction name in Spike, we should also try to keep its name the same across the entire toolchains to avoid problems. It would just be a search/replace for the code anyway.

[a0u]

Actually the conflicting names are creating problem in Spike. The vsetvl instruction definition in hwacha/opcodes_hwacha.h in that repo is causing the build system to override the upstream (RVV) implementation during build if we don't change the name. The build system will generate a source file containing the handler for every instruction, and Hwacha's vsetvl.c will overwrite RVV's implementation, causing link errors. If you want to reproduce this error, check out this commit: ucb-bar/esp-isa-sim@1969ab2.

But the fix can be isolated to esp-isa-sim and need not involve esp-opcodes at all. Only the header file has to be renamed, not the instruction itself. The rest of the toolchain is not affected since the instruction encoding remains the same.

diff --git a/hwacha/hwacha_disasm.cc b/hwacha/hwacha_disasm.cc
index d2f4947..d7c091e 100644
--- a/hwacha/hwacha_disasm.cc
+++ b/hwacha/hwacha_disasm.cc
@@ -278,7 +278,7 @@ std::vector<disasm_insn_t*> hwacha_t::get_disasms()
     insns.push_back(new disasm_insn_t(name, match_##code, mask_##code | (extra), __VA_ARGS__));
 
   DISASM_INSN("vsetcfg", vsetcfg, 0, {&npregs, &nxregs});
-  DISASM_INSN("vsetvl", vsetvl, 0, {&xrd, &xrs1});
+  DISASM_INSN("vsetvl", vsetvlh, 0, {&xrd, &xrs1});
   DISASM_INSN("vgetcfg", vgetcfg, 0, {&xrd});
   DISASM_INSN("vgetvl", vgetvl, 0, {&xrd});
 
diff --git a/hwacha/insns/vsetvl.h b/hwacha/insns/vsetvlh.h
similarity index 100%
rename from hwacha/insns/vsetvl.h
rename to hwacha/insns/vsetvlh.h
diff --git a/hwacha/opcodes_hwacha.h b/hwacha/opcodes_hwacha.h
index ab73e46..d6ec06a 100644
--- a/hwacha/opcodes_hwacha.h
+++ b/hwacha/opcodes_hwacha.h
@@ -9,7 +9,7 @@ DECLARE_INSN(vgetvl, 0x200400b, 0xfffff07f)
 DECLARE_INSN(vmca, 0x600202b, 0xfff0707f)
 DECLARE_INSN(vmcs, 0x400202b, 0xff80707f)
 DECLARE_INSN(vsetcfg, 0x200b, 0x7fff)
-DECLARE_INSN(vsetvl, 0x600b, 0xfff0707f)
+DECLARE_INSN(vsetvlh, 0x600b, 0xfff0707f)
 DECLARE_INSN(vuncfg, 0x400000b, 0xffffffff)
 DECLARE_INSN(vxcptaux, 0x200402b, 0xfffff07f)
 DECLARE_INSN(vxcptcause, 0x402b, 0xfffff07f)

In any case, even if hvsetvl is now the official mnemonic, esp-binutils should still accept vsetvl as an alias to avoid breaking existing Hwacha code outside of esp-tests.

[zitaofang]

I see. I will try to minimize the changes and hopefully get it done tomorrow or Thursday. I am doing internship right now and might not have a lot of time working on this,

[a0u]

This looks fine to me. CI appears to have encountered a transient error with the submodule clone.