core-v-verif icon indicating copy to clipboard operation
core-v-verif copied to clipboard

Published 20 hours ago • verified publisher icon openhwgroup

Cv32e40s/dev -> master

Open silabs-hfegran opened this issue 1 year ago • 7 comments

silabs-hfegran avatar Oct 23 '23 12:10 silabs-hfegran

Thanks for opennng this PR @silabs-hfegran. At this point, the merge conflicts don't look too bad:

$ git merge silabs-hfegran/cv32e40s/dev
Auto-merging bin/ci_check
Auto-merging cv32e40p/sim/ExternalRepos.mk
CONFLICT (modify/delete): cva6/tb/uvmt/uvmt_cva6_dut_wrap.sv deleted in HEAD and modified in silabs-hfegran/cv32e40s/dev.  Version silabs-hfegran/cv32e40s/dev of cva6/tb/uvmt/uvmt_cva6_dut_wrap.sv left in tree.
CONFLICT (modify/delete): cva6/tb/uvmt/uvmt_cva6_tb.sv deleted in HEAD and modified in silabs-hfegran/cv32e40s/dev.  Version silabs-hfegran/cv32e40s/dev of cva6/tb/uvmt/uvmt_cva6_tb.sv left in tree.
Auto-merging lib/uvm_agents/uvma_isacov/uvma_isacov_mon.sv
CONFLICT (content): Merge conflict in lib/uvm_agents/uvma_isacov/uvma_isacov_mon.sv
Auto-merging lib/uvm_agents/uvma_obi_memory/src/comps/uvma_obi_memory_cov_model.sv
Auto-merging lib/uvm_agents/uvma_rvfi/uvma_rvfi_instr_mon.sv
Auto-merging mk/Common.mk
Automatic merge failed; fix conflicts and then commit the result.

So, "only" three conflicts. I'll take a look and report back.

MikeOpenHWGroup avatar Oct 23 '23 13:10 MikeOpenHWGroup

Thanks for opennng this PR @silabs-hfegran. At this point, the merge conflicts don't look too bad:

$ git merge silabs-hfegran/cv32e40s/dev
Auto-merging bin/ci_check
Auto-merging cv32e40p/sim/ExternalRepos.mk
CONFLICT (modify/delete): cva6/tb/uvmt/uvmt_cva6_dut_wrap.sv deleted in HEAD and modified in silabs-hfegran/cv32e40s/dev.  Version silabs-hfegran/cv32e40s/dev of cva6/tb/uvmt/uvmt_cva6_dut_wrap.sv left in tree.
CONFLICT (modify/delete): cva6/tb/uvmt/uvmt_cva6_tb.sv deleted in HEAD and modified in silabs-hfegran/cv32e40s/dev.  Version silabs-hfegran/cv32e40s/dev of cva6/tb/uvmt/uvmt_cva6_tb.sv left in tree.
Auto-merging lib/uvm_agents/uvma_isacov/uvma_isacov_mon.sv
CONFLICT (content): Merge conflict in lib/uvm_agents/uvma_isacov/uvma_isacov_mon.sv
Auto-merging lib/uvm_agents/uvma_obi_memory/src/comps/uvma_obi_memory_cov_model.sv
Auto-merging lib/uvm_agents/uvma_rvfi/uvma_rvfi_instr_mon.sv
Auto-merging mk/Common.mk
Automatic merge failed; fix conflicts and then commit the result.

So, "only" three conflicts. I'll take a look and report back.

Two deleted files, and one actual conflict - shouldn't be too hard to fix, but my first attempt failed to run hello-world.

silabs-hfegran avatar Oct 23 '23 13:10 silabs-hfegran

Merge conflicts should be OK now, runs hello world, but I will need some time to run a larger regression

silabs-hfegran avatar Oct 23 '23 13:10 silabs-hfegran

Update on the three conflicts. The first two are easy: the cva6 directory has been deleted from CORE-V-VERIF, so cva6/tb/uvmt/uvmt_cva6_dut_wrap.sv and cva6/tb/uvmt/uvmt_cva6_tb.sv can safely be deleted here.

The last one, lib/uvm_agents/uvma_isacov/uvma_isacov_mon.sv is more challenging. Below is a snipit of the conflict. I believe we want to deleted the HEAD section (everything between <<<<<<< HEAD and =======) and keep silabs-hfegran/cv32e40s/dev (everything from ======= and >>>>>>> silabs-hfegran/cv32e40s/dev).

Comments?

<<<<<<< HEAD
  // Disassemble the instruction using Spike (via DPI)
  if (mon_trn.instr.ext == C_EXT) begin
    mon_trn.instr.rs1     = dasm_rvc_rs1(instr);
    mon_trn.instr.rs2     = dasm_rvc_rs2(instr);
    mon_trn.instr.rd      = dasm_rvc_rd(instr);
    mon_trn.instr.c_rdrs1 = dasm_rvc_rd(instr);
    mon_trn.instr.c_rd    = mon_trn.instr.decode_rd_c(instr);
    mon_trn.instr.c_rs1   = mon_trn.instr.decode_rs1_c(instr);
    mon_trn.instr.c_rs2   = mon_trn.instr.decode_rs2_c(instr);
  end
  else begin
    mon_trn.instr.rs1  = dasm_rs1(instr);
    mon_trn.instr.rs2  = dasm_rs2(instr);
    mon_trn.instr.rd   = dasm_rd(instr);
    mon_trn.instr.immi = dasm_i_imm(instr);
    mon_trn.instr.imms = dasm_s_imm(instr);
    mon_trn.instr.immb = dasm_sb_imm(instr) >> 1;  // Because dasm gives [12:0], not [12: 1]
    mon_trn.instr.immu = dasm_u_imm(instr) >> 12;  // Because dasm gives [31:0], not [31:12]
    mon_trn.instr.immj = dasm_uj_imm(instr) >> 1;  // Because dasm gives [20:0], not [20: 1]
=======
    //Disassemble the instruction using Spike (via DPI)
    if (mon_trn.instr.ext == C_EXT) begin
      mon_trn.instr.rs1     = dasm_rvc_rs1(instr);
      mon_trn.instr.rs2     = dasm_rvc_rs2(instr);
      mon_trn.instr.rd      = dasm_rvc_rd(instr);
      mon_trn.instr.c_rdrs1 = dasm_rvc_rd(instr);
      mon_trn.instr.c_rdp   = dasm_rvc_rs1s(instr);
      mon_trn.instr.c_rs1s  = dasm_rvc_rs1s(instr);
      mon_trn.instr.c_rs2s  = dasm_rvc_rs2s(instr);
    end
    else begin
      mon_trn.instr.rs1  = dasm_rs1(instr);
      mon_trn.instr.rs2  = dasm_rs2(instr);
      mon_trn.instr.rd   = dasm_rd(instr);
      mon_trn.instr.immi = dasm_i_imm(instr);
      mon_trn.instr.imms = dasm_s_imm(instr);
      mon_trn.instr.immb = dasm_sb_imm(instr) >> 1;  // Because dasm gives [12:0], not [12: 1]
      mon_trn.instr.immu = dasm_u_imm(instr) >> 12;  // Because dasm gives [31:0], not [31:12]
      mon_trn.instr.immj = dasm_uj_imm(instr) >> 1;  // Because dasm gives [20:0], not [20: 1]
    end

    // Make instructions as illegal,
    // 1. If a CSR instruction is not targeted to a valid CSR
    if (mon_trn.instr.group == CSR_GROUP) begin
      mon_trn.instr.csr_val = dasm_csr(instr);
      if (!$cast(mon_trn.instr.csr, mon_trn.instr.csr_val) ||
           cfg.core_cfg.unsupported_csr_mask[mon_trn.instr.csr_val]) begin
        mon_trn.instr.illegal = 1;
      end
    end

  end else if (cfg.decoder == ISA_SUPPORT) begin
    // Attempt to decode instruction with isa_support

    // TODO: silabs-hefegran, isa decoder representation changed for compressed 'rx registers,
    // we supply the old (non-translated) value to avoid having to rewrite that logic now, which
    // might also interfere with the spike implementation.
    // the "get_rx"-functions should no longer be needed if we supply the translated values to
    // the coverage model.
    mon_trn.instr.c_rdrs1 = instr_asm.rd.valid_gpr_rvc  ? instr_asm.rd.gpr_rvc  : instr_asm.rd.gpr;
    mon_trn.instr.c_rdp   = instr_asm.rd.valid_gpr_rvc  ? instr_asm.rd.gpr_rvc  : instr_asm.rd.gpr;
    mon_trn.instr.c_rs1s  = instr_asm.rs1.valid_gpr_rvc ? instr_asm.rs1.gpr_rvc : instr_asm.rs1.gpr;
    mon_trn.instr.c_rs2s  = instr_asm.rs2.valid_gpr_rvc ? instr_asm.rs2.gpr_rvc : instr_asm.rs2.gpr;
    mon_trn.instr.rs1     = instr_asm.rs1.valid_gpr_rvc ? instr_asm.rs1.gpr_rvc : instr_asm.rs1.gpr;
    mon_trn.instr.rs2     = instr_asm.rs2.valid_gpr_rvc ? instr_asm.rs2.gpr_rvc : instr_asm.rs1.gpr;
    mon_trn.instr.rd      = instr_asm.rd.valid_gpr_rvc  ? instr_asm.rd.gpr_rvc  : instr_asm.rd.gpr;
    mon_trn.instr.immi    = instr_asm.imm.imm_raw_sorted;
    mon_trn.instr.imms    = instr_asm.imm.imm_raw_sorted;
    mon_trn.instr.immb    = instr_asm.imm.imm_raw_sorted;
    mon_trn.instr.immu    = instr_asm.imm.imm_raw_sorted;
    mon_trn.instr.immj    = instr_asm.imm.imm_raw_sorted;

    // Make instructions as illegal,
    // 1. If a CSR instruction is not targeted to a valid CSR
    if (mon_trn.instr.group == CSR_GROUP) begin
      mon_trn.instr.csr_val = instr_asm.csr.address;
      if (!$cast(mon_trn.instr.csr, mon_trn.instr.csr_val) ||
           cfg.core_cfg.unsupported_csr_mask[mon_trn.instr.csr_val]) begin
        mon_trn.instr.illegal = 1;
      end
    end
>>>>>>> silabs-hfegran/cv32e40s/dev
  end

MikeOpenHWGroup avatar Oct 23 '23 14:10 MikeOpenHWGroup

Update on the three conflicts. The first two are easy: the cva6 directory has been deleted from CORE-V-VERIF, so cva6/tb/uvmt/uvmt_cva6_dut_wrap.sv and cva6/tb/uvmt/uvmt_cva6_tb.sv can safely be deleted here.

The last one, lib/uvm_agents/uvma_isacov/uvma_isacov_mon.sv is more challenging. Below is a snipit of the conflict. I believe we want to deleted the HEAD section (everything between <<<<<<< HEAD and =======) and keep silabs-hfegran/cv32e40s/dev (everything from ======= and >>>>>>> silabs-hfegran/cv32e40s/dev).

Comments?

Close, there are some changes from HEAD that needs to be included, for example this part:

    mon_trn.instr.c_rd    = mon_trn.instr.decode_rd_c(instr);
    mon_trn.instr.c_rs1   = mon_trn.instr.decode_rs1_c(instr);
    mon_trn.instr.c_rs2   = mon_trn.instr.decode_rs2_c(instr);

There are also a couple of naming changes where rdp, rs1s and rs2s are renamed to rd, rs1 and rs2 respectively. c.f. my latest fix. I ran this through full regressions overnight and it appears to give me more or less same results as for the 40s/dev branch, additionally isacov coverage looks as expected. I will go ahead and convert this to a proper PR as it looks good from our point of view.

silabs-hfegran avatar Oct 24 '23 07:10 silabs-hfegran

@MikeOpenHWGroup: Are we waiting for someone/something before merging this?

silabs-hfegran avatar Oct 30 '23 09:10 silabs-hfegran

@MikeOpenHWGroup: Are we waiting for someone/something before merging this?

I had the same question this morning. :-) I'm running a few tests and will approve (or not) based on their outcome.

MikeOpenHWGroup avatar Oct 30 '23 13:10 MikeOpenHWGroup