On Sun, Jul 26, 2015 at 07:05:01PM +0200, Lars-Peter Clausen wrote:
Testing on ARM shows that the combined code size of the specialized functions is about 50% larger than the generlized function in relative numbers. But in absolute numbers its less than 200 bytes, which is still quite small. On the other hand the generalized function increases the execution time of dapm_power_one_widget() by 30%. Given that this function is one of the most often called functions of the DAPM framework the tradeoff seems to be worth it.
This reads like you mean to say that it's worth it to combine things in spite of the 30% speed hit... you're not clear which tradeoff you mean.
To avoid this still keep two versions of these functions around, one for input and one for output. But have a generic implementation of the algorithm which gets inlined by those two versions and then let the compiler take care of optimizing it into specialized versions.
...which the compiler presumably actually manages to do.
--- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h
+/**
- snd_soc_dapm_widget_for_each_sink_path - Iterates over all paths in the
- specified direction of a widget
- @w: The widget
- @dir: Whether to iterate over the paths where the specified widget is the
incoming or outgoing widgets
- @p: The path iterator variable
- */
+#define snd_soc_dapm_widget_for_each_path(w, dir, p) \
- list_for_each_entry(p, &w->edges[dir], list_node[dir])
This is a macro not a static inline? The other macros that use this are macros but that's more outdated code than anything I think, might be worth fixing that while we're at it.
#define DAPM_UPDATE_STAT(widget, val) widget->dapm->card->dapm_stats.val++;
+#define dapm_widget_for_each_path_safe(w, dir, p, next_p) \
- list_for_each_entry_safe(p, next_p, &w->edges[dir], list_node[dir])
It's a bit odd not to have this in the header next to the unsafe version.
+#define SND_SOC_DAPM_DIR_REVERSE(x) ((x == SND_SOC_DAPM_DIR_IN) ? \
- SND_SOC_DAPM_DIR_OUT : SND_SOC_DAPM_DIR_IN)
+#define dapm_for_each_direction(dir) \
- for ((dir) = SND_SOC_DAPM_DIR_IN; (dir) <= SND_SOC_DAPM_DIR_OUT; \
(dir)++)
Eew.
list_for_each_entry(w, &card->widgets, list) {
w->inputs = -1;w->outputs = -1;
w->endpoints[SND_SOC_DAPM_DIR_IN] = -1;w->endpoints[SND_SOC_DAPM_DIR_OUT] = -1;
Loop over the endpoints array in some of the users like this perhaps?
- dapm_for_each_direction(dir) {
rdir = SND_SOC_DAPM_DIR_REVERSE(dir);snd_soc_dapm_widget_for_each_path(w, dir, p) {if (p->connected && !p->connected(w, p->node[rdir]))continue;
Looks like dapm_for_each_direction should be snd_soc_dapm_for_each_direction - the above looks wrong.