| Message ID | f177ef9c929fc69d3bfb27cd1a78fda6eaf4f927.1440681672.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote: > Add a common function to return "yes" or "no" string based on the > argument, and drop the local versions of it. Purely out of curiosity, gcc is able to amalgamate the constant strings (I remember reading that it is intelligent enough to do so), right? i.e. size i915.ko doesn't change (at least .data, we may see .text differences for gcc having different ideas about inlines)? -Chris
On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote: >> Add a common function to return "yes" or "no" string based on the >> argument, and drop the local versions of it. > > Purely out of curiosity, gcc is able to amalgamate the constant strings > (I remember reading that it is intelligent enough to do so), right? i.e. > size i915.ko doesn't change (at least .data, we may see .text > differences for gcc having different ideas about inlines)? I admit to giving GCC the benefit of the doubt. I may be naïve that way, trusting the tools to do what seems like the obviously right thing to do. If GCC lets us down, we could try something like the yesno version in drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the right thing with that would be violating the standard. BR, Jani. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
On Thu, 27 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote: > On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote: >> On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote: >>> Add a common function to return "yes" or "no" string based on the >>> argument, and drop the local versions of it. >> >> Purely out of curiosity, gcc is able to amalgamate the constant strings >> (I remember reading that it is intelligent enough to do so), right? i.e. >> size i915.ko doesn't change (at least .data, we may see .text >> differences for gcc having different ideas about inlines)? > > I admit to giving GCC the benefit of the doubt. I may be naïve that way, > trusting the tools to do what seems like the obviously right thing to > do. > > If GCC lets us down, we could try something like the yesno version in > drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the > right thing with that would be violating the standard. AFAICT GCC does the right thing with the patch. BR, Jani. > > BR, > Jani. > >> -Chris >> >> -- >> Chris Wilson, Intel Open Source Technology Centre > > -- > Jani Nikula, Intel Open Source Technology Center
On Mon, Aug 31, 2015 at 05:23:27PM +0300, Jani Nikula wrote: > On Thu, 27 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote: > > On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote: > >> On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote: > >>> Add a common function to return "yes" or "no" string based on the > >>> argument, and drop the local versions of it. > >> > >> Purely out of curiosity, gcc is able to amalgamate the constant strings > >> (I remember reading that it is intelligent enough to do so), right? i.e. > >> size i915.ko doesn't change (at least .data, we may see .text > >> differences for gcc having different ideas about inlines)? > > > > I admit to giving GCC the benefit of the doubt. I may be naïve that way, > > trusting the tools to do what seems like the obviously right thing to > > do. > > > > If GCC lets us down, we could try something like the yesno version in > > drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the > > right thing with that would be violating the standard. > > AFAICT GCC does the right thing with the patch. Fwiw, I didn't see any harm in the series, so Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Mon, Aug 31, 2015 at 03:33:13PM +0100, Chris Wilson wrote: > On Mon, Aug 31, 2015 at 05:23:27PM +0300, Jani Nikula wrote: > > On Thu, 27 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote: > > > On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > >> On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote: > > >>> Add a common function to return "yes" or "no" string based on the > > >>> argument, and drop the local versions of it. > > >> > > >> Purely out of curiosity, gcc is able to amalgamate the constant strings > > >> (I remember reading that it is intelligent enough to do so), right? i.e. > > >> size i915.ko doesn't change (at least .data, we may see .text > > >> differences for gcc having different ideas about inlines)? > > > > > > I admit to giving GCC the benefit of the doubt. I may be naïve that way, > > > trusting the tools to do what seems like the obviously right thing to > > > do. > > > > > > If GCC lets us down, we could try something like the yesno version in > > > drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the > > > right thing with that would be violating the standard. > > > > AFAICT GCC does the right thing with the patch. > > Fwiw, I didn't see any harm in the series, so > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Merged just this patch (due to conflict fun for now) to dinq, thanks. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 4088cb1b95d9..de7ae1c398ec 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -46,11 +46,6 @@ enum { PINNED_LIST, }; -static const char *yesno(int v) -{ - return v ? "yes" : "no"; -} - /* As the drm_debugfs_init() routines are called before dev->dev_private is * allocated we need to hook into the minor for release. */ static int diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8c938451a05e..118223a3e155 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -106,6 +106,11 @@ unlikely(__ret_warn_on); \ }) +static inline const char *yesno(bool v) +{ + return v ? "yes" : "no"; +} + enum pipe { INVALID_PIPE = -1, PIPE_A = 0, diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 493e9b294a69..3379f9c1ef88 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -30,11 +30,6 @@ #include <generated/utsrelease.h> #include "i915_drv.h" -static const char *yesno(int v) -{ - return v ? "yes" : "no"; -} - static const char *ring_str(int ring) { switch (ring) {
Add a common function to return "yes" or "no" string based on the argument, and drop the local versions of it. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 5 ----- drivers/gpu/drm/i915/i915_drv.h | 5 +++++ drivers/gpu/drm/i915/i915_gpu_error.c | 5 ----- 3 files changed, 5 insertions(+), 10 deletions(-)