search

riscv / riscv-control-transfer-records

This repo contains a RISC-V ISA extension (proposal) to allow recording of control transfer history to on-chip registers, to support usages associated with profiling and debug.
https://jira.riscv.org/browse/RVG-62
Creative Commons Attribution 4.0 International
14 stars 4 forks source link

Renaming NTBREN to NTBRINH #2

Closed rajnesh-kanwal closed 11 months ago

rajnesh-kanwal commented 11 months ago

Same as all other filters bit I think we should keep Not taken branch filter bit as Inhibit bit rather than enable bit. This can be easily missed by software developers leading to bugs.

@bcstrongx @atishp04

bcstrongx commented 11 months ago

I made the not-taken branch bit, as well as the external trap bit, enables since, unlike the other transfer types, they are intended to be off by default. The default behavior is to record all (and only) control flow transfers within enabled privilege modes, which should not require any of the transfer type inhibit/enable bits to be implemented, much less set.

rajnesh-kanwal commented 11 months ago

That makes sense but this breaks compatibility with other bits. In both scenarios (EN/INH), when CTR is being used, developer is supposed to selectively program the filters, which will be applicable for both inhibit and enable bits. We can not free the programmer from his job of carefully programming the filters.

I am fine with both cases but I think the Enable bit creates confusion.

atishp04 commented 11 months ago

I agree that enable bits may lead to confusion if somebody is not paying attention carefully. However, we end up specifying non-zero reset value if we intend to keep the not taken branch and external trap off by default. That is not ideal either. I checked perf record command line and there is no way to selectively filter external trap/not taken branches. @rajnesh-kanwal let me know if I missed something.

I would suggest just add an application note highlighting the fact that these two bits are enable not inhibit with appropriate reasoning.

bcstrongx commented 11 months ago

I agree that neither approach is ideal, but I prefer not to have a non-zero reset value. I will add a non-normative note highlighting the differing polarities.

rajnesh-kanwal commented 11 months ago

Sounds good.