search

riscv-software-src / riscof

BSD 3-Clause "New" or "Revised" License
61 stars 39 forks source link

riscof run: IndexError: list index out of range - from riscof/framework/test.py #27

Open nadiga-ventana opened 2 years ago

nadiga-ventana commented 2 years ago

Getting this error when running RISCOF run for riscv-test-suite/rv64i_m/I/src/sll-01.S which has RVTEST_ISA("RV64i")

  File "/release/ventana/vt1/venv/cb1d784/lib/python3.8/site-packages/riscof/framework/main.py", line 200, in run
    results = test.run_tests(dut, base, ispec['hart0'], pspec, work_dir, cntr_args)
  File "/release/ventana/vt1/venv/cb1d784/lib/python3.8/site-packages/riscof/framework/test.py", line 412, in run_tests
    test_list, test_pool = generate_test_pool(ispec, pspec, work_dir, cntr_args[0])
  File "/release/ventana/vt1/venv/cb1d784/lib/python3.8/site-packages/riscof/framework/test.py", line 341, in generate_test_pool
    isa = prod_isa(ispec['ISA'],db[file]['isa'])
  File "/release/ventana/vt1/venv/cb1d784/lib/python3.8/site-packages/riscof/framework/test.py", line 285, in prod_isa
    prefix = match[0][0]
IndexError: list index out of range

We probably need to make these two regexp case insensitive

        match = re.findall("(?P<prefix>RV(64|128|32)(I|E))",entry)
        prefix = match[0][0]
        exts = isa_set(re.sub("RV(64|128|32)(I|E)","",entry))
pdonahue-ventana commented 2 years ago

The "ISA Extension Naming Conventions" chapter of the unprivileged spec says: "The ISA naming strings are case insensitive." So it should be case-insensitive in the python (probably via re.IGNORECASE).

pawks commented 2 years ago

This error in the test has been fixed on the master of riscv-arch-test. All the tests so far have the ISA strings in upper case. Should a case insensitive match be allowed? What do you think @neelgala @allenjbaum ?

neelgala commented 2 years ago

well.. when introduced the RVTEST_ISA macro wasn't intended to be compliant with the "ISA Extension Naming Conventions". It's major usecase now is to derive the march like arguments for each test. This becomes easy to do if we follow a case sensitive approach, where all standard extensions are upper-case (alphabets A-Z) and the Z extensions are lower-case.

so unless there is real value in forcing RVTEST_ISA to be compliant with "ISA Extension Naming Conventions" - I wouldn't suggest changing it. Following the case-sensitive approach should probably be put in the test-format spec to make it clear. RISCOF can additionally include checks to ensure that the tests follow this convention.

Also I believe the actual error reported in this issue has been fixed in the riscv-arch-test.