Bug 1195 - removing some f-strings and misleading log messages
Summary: removing some f-strings and misleading log messages
Status: IN_PROGRESS
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: PC Linux
: Normal enhancement
Assignee: Jacob Lifshay
URL:
Depends on: 589 939
Blocks:
  Show dependency treegraph
 
Reported: 2023-10-21 00:07 BST by Jacob Lifshay
Modified: 2023-11-29 19:57 GMT (History)
3 users (show)

See Also:
NLnet milestone: ---
total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for this task, excluding subtasks' budget: 0
parent task for budget allocation:
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jacob Lifshay 2023-10-21 00:07:37 BST
At luke's request, I rewrote the f-strings and str.format() calls in some files to use printf-style formatting, pushing the changes to
the programmerjake/remove-f-strings branch:

https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=6521a032629e493db091860c690b3487ea142a6c

commit f75d15aeaa4e321462d0e19a19e075c48ee38691
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Fri Oct 20 15:32:27 2023 -0700

    replace some f-strings and str.format calls with % as luke requested

I also changed the code to not generate log messages like the following:

setting spr TFHAR SelectableInt(value=0x0, bits=64)
setting spr TFIAR SelectableInt(value=0x0, bits=64)
setting spr TEXASR SelectableInt(value=0x0, bits=64)
setting spr TEXASRU SelectableInt(value=0x0, bits=32)

it turns out they were generated by the code that checks if all SPRs match in ExpectedState that was running caller.SPR.__getitem__, which calls __setitem__ for SPRs it doesn't have entries for:

commit 6521a032629e493db091860c690b3487ea142a6c
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Fri Oct 20 16:00:41 2023 -0700

    don't generate misleading log messages setting unimplemented SPRs to zero
    
    turns out they're generated by calling caller.SPR.__getitem__
Comment 1 Luke Kenneth Casson Leighton 2023-10-21 00:33:51 BST
one of the reasons for not adding every SPR is that
some of them are aliases for high and low 32-bit halves.
adding that is not necessary.

the reason for not using PHP string formatting has
been made clear multiple times systematically on
multiple occasions. so many that it was set over
two years ago as a hard requirement of the project's
HDL_workflow development practices.

there is now a massive technical debt thanks to ths
hard requirement being blatantly disregarded by multiple
contributors.
Comment 2 Luke Kenneth Casson Leighton 2023-10-21 00:36:00 BST
1540                 log(f"write OV/OV32 OV={ov} OV32={ov32}",
1541                     kind=LogKind.InstrInOuts)
1542             else:
1543                 # TODO: if 32-bit mode, set ca to ca32
1544                 self.spr['XER'][XER_bits['CA']] = ca
1545                 self.spr['XER'][XER_bits['CA32']] = ca32
1546                 log(f"write CA/CA32 CA={ca} CA32={ca32}",
1547                     kind=LogKind.InstrInOuts)
Comment 3 Luke Kenneth Casson Leighton 2023-10-21 00:37:22 BST
 253             line_bytes = mem_lines[line_index]
 254             line_str = f"0x{line_addr:08X}:"
 255             for col_chunk in range(0, line_size,
 256                                    column_chunk_size):
 257                 line_str += " "
 258                 for i in range(column_chunk_size):
 259                     line_str += f" {line_bytes[col_chunk + i]:02X}"
 260             line_str += "  |"
 261             for i in range(line_size):
 262                 if 0x20 <= line_bytes[i] <= 0x7E:
 263                     line_str += chr(line_bytes[i])
 264                 else:
 265                     line_str += "."
 266             line_str += "|"
 267             lines.append(line_str)
 268         lines = "\n".join(lines)
 269         log(f"\n{name}:\n{lines}\n", kind=kind)
 270
Comment 4 Luke Kenneth Casson Leighton 2023-10-21 00:37:51 BST
 343         if addr != expected:
 344             exc = MemException("not sign extended",
 345                                f"address not sign extended: 0x{addr:X} "
 346                                f"expected 0x{expected:X}")
 347             exc.dar = addr
 348             raise exc
 349         return page_idx
 350 
 351     def __reduce_ex__(self, protocol):
 352         raise PicklingError("MemMMap can't be deep-copied or pickled")
 353 
 354     def __access_addr_range_err(self, start_addr, size, needed_flag):
 355         assert needed_flag != _MMapPageFlags.W, \
 356             f"can't write to address 0x{start_addr:X} size 0x{size:X}"
Comment 5 Luke Kenneth Casson Leighton 2023-10-21 00:38:16 BST
 385         assert block is not None, \
 386             f"can't read from address 0x{start_addr:X} size 0x{size:X}"
 387         return (ctypes.c_ubyte * size).from_buffer(block, block_addr)
 388
Comment 6 Luke Kenneth Casson Leighton 2023-10-21 00:40:57 BST
  77         for first_old_fname in found:
  78             found_str = " ".join(f"'{i}'" for i in found)
  79             lines = [
  80                 f"found likely old generated file: {first_old_fname!r}",
  81                 f"new location: {first_fname!r}",
  82                 ("please remove the old generated files to avoid committing "
  83                  "them or add to:"),
  84                 ("openpower.decoder.pseudo.pywriter."
  85                  "OLD_GENERATED_FILES_EXCEPTIONS"),
  86                 "commands to remove all likely old generated files:",
  87                 f"(cd {isadir}; rm -v {found_str})",
  88             ]
Comment 7 Jacob Lifshay 2023-11-29 18:52:33 GMT
i'm setting this to lower priority, since cavatools needs to be finished first.
Comment 8 Luke Kenneth Casson Leighton 2023-11-29 19:57:12 GMT
(In reply to Jacob Lifshay from comment #7)
> i'm setting this to lower priority, since cavatools needs to be finished
> first.

bug #939. and the crypto-primitives bug #589.