> From: Pavan Nikhilesh [mailto:[email protected]] > Sent: Friday, January 12, 2018 4:44 PM > To: [email protected]; [email protected]; Van > Haaren, Harry <[email protected]>; Eads, Gage > <[email protected]>; [email protected]; [email protected]; Ma, > Liang J <[email protected]> > Cc: [email protected]; Pavan Nikhilesh <[email protected]> > Subject: [dpdk-dev] [PATCH v4 10/13] app/eventdev: add pipeline queue worker > functions > > Signed-off-by: Pavan Nikhilesh <[email protected]>
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 <[email protected]> > 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!
