[Mesa-dev] [PATCH v2] nvc0: simplify the way percentage metrics are computed

[Ilia Mirkin]

Is that worth it? Now you're getting potentially imprecise return
results for int64 counters, just to remove a few *100?

On Mon, Oct 31, 2016 at 10:37 AM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
> v2: - forgot to return double instead of uint64_t
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  .../drivers/nouveau/nvc0/nvc0_query_hw_metric.c    | 26 +++++++++++++---------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_metric.c b/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_metric.c
> index 36534ba..86c4923 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_metric.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_metric.c
> @@ -439,19 +439,19 @@ nvc0_hw_metric_end_query(struct nvc0_context *nvc0, struct nvc0_hw_query *hq)
>        hmq->queries[i]->funcs->end_query(nvc0, hmq->queries[i]);
>  }
>
> -static uint64_t
> +static double
>  sm20_hw_metric_calc_result(struct nvc0_hw_query *hq, uint64_t res64[8])
>  {
>     switch (hq->base.type - NVC0_HW_METRIC_QUERY(0)) {
>     case NVC0_HW_METRIC_QUERY_ACHIEVED_OCCUPANCY:
>        /* ((active_warps / active_cycles) / max. number of warps on a MP) * 100 */
>        if (res64[1])
> -         return ((res64[0] / (double)res64[1]) / 48) * 100;
> +         return ((res64[0] / (double)res64[1]) / 48);
>        break;
>     case NVC0_HW_METRIC_QUERY_BRANCH_EFFICIENCY:
>        /* (branch / (branch + divergent_branch)) * 100 */
>        if (res64[0] + res64[1])
> -         return (res64[0] / (double)(res64[0] + res64[1])) * 100;
> +         return (res64[0] / (double)(res64[0] + res64[1]));
>        break;
>     case NVC0_HW_METRIC_QUERY_INST_PER_WRAP:
>        /* inst_executed / warps_launched */
> @@ -471,7 +471,7 @@ sm20_hw_metric_calc_result(struct nvc0_hw_query *hq, uint64_t res64[8])
>     case NVC0_HW_METRIC_QUERY_ISSUE_SLOT_UTILIZATION:
>        /* ((inst_issued / 2) / active_cycles) * 100 */
>        if (res64[1])
> -         return ((res64[0] / 2) / (double)res64[1]) * 100;
> +         return ((res64[0] / 2) / (double)res64[1]);
>        break;
>     case NVC0_HW_METRIC_QUERY_IPC:
>        /* inst_executed / active_cycles */
> @@ -486,7 +486,7 @@ sm20_hw_metric_calc_result(struct nvc0_hw_query *hq, uint64_t res64[8])
>     return 0;
>  }
>
> -static uint64_t
> +static double
>  sm21_hw_metric_calc_result(struct nvc0_hw_query *hq, uint64_t res64[8])
>  {
>     switch (hq->base.type - NVC0_HW_METRIC_QUERY(0)) {
> @@ -520,7 +520,7 @@ sm21_hw_metric_calc_result(struct nvc0_hw_query *hq, uint64_t res64[8])
>        /* ((metric-issue_slots / 2) / active_cycles) * 100 */
>        if (res64[4])
>           return (((res64[0] + res64[1] + res64[2] + res64[3]) / 2) /
> -                 (double)res64[4]) * 100;
> +                 (double)res64[4]);
>        break;
>     case NVC0_HW_METRIC_QUERY_IPC:
>        return sm20_hw_metric_calc_result(hq, res64);
> @@ -532,14 +532,14 @@ sm21_hw_metric_calc_result(struct nvc0_hw_query *hq, uint64_t res64[8])
>     return 0;
>  }
>
> -static uint64_t
> +static double
>  sm30_hw_metric_calc_result(struct nvc0_hw_query *hq, uint64_t res64[8])
>  {
>     switch (hq->base.type - NVC0_HW_METRIC_QUERY(0)) {
>     case NVC0_HW_METRIC_QUERY_ACHIEVED_OCCUPANCY:
>        /* ((active_warps / active_cycles) / max. number of warps on a MP) * 100 */
>        if (res64[1])
> -         return ((res64[0] / (double)res64[1]) / 64) * 100;
> +         return ((res64[0] / (double)res64[1]) / 64);
>        break;
>     case NVC0_HW_METRIC_QUERY_BRANCH_EFFICIENCY:
>        return sm20_hw_metric_calc_result(hq, res64);
> @@ -564,7 +564,7 @@ sm30_hw_metric_calc_result(struct nvc0_hw_query *hq, uint64_t res64[8])
>     case NVC0_HW_METRIC_QUERY_ISSUE_SLOT_UTILIZATION:
>        /* ((metric-issue_slots / 2) / active_cycles) * 100 */
>        if (res64[2])
> -         return (((res64[0] + res64[1]) / 2) / (double)res64[2]) * 100;
> +         return (((res64[0] + res64[1]) / 2) / (double)res64[2]);
>        break;
>     case NVC0_HW_METRIC_QUERY_IPC:
>        return sm20_hw_metric_calc_result(hq, res64);
> @@ -589,12 +589,15 @@ nvc0_hw_metric_get_query_result(struct nvc0_context *nvc0,
>     struct nvc0_hw_metric_query *hmq = nvc0_hw_metric_query(hq);
>     struct nvc0_screen *screen = nvc0->screen;
>     struct nouveau_device *dev = screen->base.device;
> +   const struct nvc0_hw_metric_cfg *cfg = NULL;
>     union pipe_query_result results[8] = {};
>     uint64_t res64[8] = {};
> -   uint64_t value = 0;
> +   double value = 0;
>     boolean ret = false;
>     unsigned i;
>
> +   cfg = nvc0_hw_metric_get_cfg(hq->base.type - NVC0_HW_METRIC_QUERY(0));
> +
>     for (i = 0; i < hmq->num_queries; i++) {
>        ret = hmq->queries[i]->funcs->get_query_result(nvc0, hmq->queries[i],
>                                                       wait, &results[i]);
> @@ -616,6 +619,9 @@ nvc0_hw_metric_get_query_result(struct nvc0_context *nvc0,
>        break;
>     }
>
> +   if (cfg->type == PIPE_DRIVER_QUERY_TYPE_PERCENTAGE)
> +      value *= 100;
> +
>     *(uint64_t *)result = value;
>     return ret;
>  }
> --
> 2.10.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev