> From: Pavan Nikhilesh [mailto:pbhagavat...@caviumnetworks.com]
> Sent: Friday, January 12, 2018 4:44 PM
> To: jerin.ja...@caviumnetworks.com; santosh.shu...@caviumnetworks.com; Van
> Haaren, Harry <harry.van.haa...@intel.com>; Eads, Gage
> <gage.e...@intel.com>; hemant.agra...@nxp.com; nipun.gu...@nxp.com; Ma,
> Liang J <liang.j...@intel.com>
> Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavat...@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH v4 10/13] app/eventdev: add pipeline queue worker
> functions
>
> Signed-off-by: Pavan Nikhilesh <pbhagavat...@caviumnetworks.com>
Some of the code feels like duplication - but there are differences in each
implementation as far as I can see. The common initialization parts are
"macro-ed" to ensure minimal duplication. In short, I'm not aware of a better
solution/implementation.
Please see note on branch-optimization in the code below.
Acked-by: Harry van Haaren <harry.van.haa...@intel.com>
> static int
> worker_wrapper(void *arg)
> {
> - RTE_SET_USED(arg);
> + struct worker_data *w = arg;
> + struct evt_options *opt = w->t->opt;
> + const bool burst = evt_has_burst_mode(w->dev_id);
> + const bool mt_safe = !w->t->mt_unsafe;
> + const uint8_t nb_stages = opt->nb_stages;
> + RTE_SET_USED(opt);
> +
> + /* allow compiler to optimize */
> + if (nb_stages == 1) {
> + if (!burst && mt_safe)
> + return pipeline_queue_worker_single_stage_tx(arg);
> + else if (!burst && !mt_safe)
> + return pipeline_queue_worker_single_stage_fwd(arg);
> + else if (burst && mt_safe)
> + return pipeline_queue_worker_single_stage_burst_tx(arg);
> + else if (burst && !mt_safe)
> + return pipeline_queue_worker_single_stage_burst_fwd(
> + arg);
> + } else {
> + if (!burst && mt_safe)
> + return pipeline_queue_worker_multi_stage_tx(arg);
> + else if (!burst && !mt_safe)
> + return pipeline_queue_worker_multi_stage_fwd(arg);
> + else if (burst && mt_safe)
> + return pipeline_queue_worker_multi_stage_burst_tx(arg);
> + else if (burst && !mt_safe)
> + return pipeline_queue_worker_multi_stage_burst_fwd(arg);
> +
> + }
I don't think the compiler will optimize the below, as the nb_stages value
comes from opt, which is a uint8 originating from a pointer-dereference...
As far as I know, an optimizing compiler will do "constant propagation" through
function calls, but I would be amazed if it found a pointer initialized to a
specific value, and optimized the branches away here based on the contents of
that pointer deref.
So I see 2 options;
1) accept as is - I'll ack this patch here
2) Rework so that the worker_wrapper() accepts nb_stages, burst and mt_safe as
function arguments, allowing constant propagation for optimizing away the
branches.
@Pavan, I'll leave that choice up to you!
