Re: [dev] [st] [PATCH] Explicit cast in CEIL macro

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.com>
Date: Tue, 24 Jun 2014 21:16:58 +0200

> But who'd specify a scaling factor of 1.99999 or sth.?
> BTW: Casting to int yields the exact same results for both CEIL and
> ceilf in my setup.

There is an implicit casting because xw.cw and xw.ch are integers,
so the explicit casting is redundant. This macro is used only in one place,
xloadfonts, that is out of the main loop (usually is only called in
the init process), so it is a waste of time try to optimize it for speed.
A macro like:

        CEIL(x) ((int)(x) + ((x) > 0 && (int)(x) != (x)))

Can take more bytes that the direct call to ceilf because it has
only one parameter, so the calling sequence is short, and in case of
being and inlined function, the compiler version will be better
that our version, so my proposal for the patch is:



diff --git a/config.mk b/config.mk
index 97afa2c..6d90f86 100644
--- a/config.mk
+++ b/config.mk
_AT_@ -14,7 +14,7 @@ X11LIB = /usr/X11R6/lib
 INCS = -I. -I/usr/include -I${X11INC} \
        `pkg-config --cflags fontconfig` \
        `pkg-config --cflags freetype2`
-LIBS = -L/usr/lib -lc -L${X11LIB} -lX11 -lutil -lXext -lXft \
+LIBS = -L/usr/lib -lc -L${X11LIB} -lm -lX11 -lutil -lXext -lXft \
        `pkg-config --libs fontconfig` \
        `pkg-config --libs freetype2`
 
diff --git a/st.c b/st.c
index 26053b0..e5c8a3f 100644
--- a/st.c
+++ b/st.c
_AT_@ -77,7 +77,6 @@ char *argv0;
 #define ATTRCMP(a, b) ((a).mode != (b).mode || (a).fg != (b).fg || (a).bg != (b).bg)
 #define IS_SET(flag) ((term.mode & (flag)) != 0)
 #define TIMEDIFF(t1, t2) ((t1.tv_sec-t2.tv_sec)*1000 + (t1.tv_usec-t2.tv_usec)/1000)
-#define CEIL(x) (((x) != (int) (x)) ? (x) + 1 : (x))
 #define MODBIT(x, set, bit) ((set) ? ((x) |= (bit)) : ((x) &= ~(bit)))
 
 #define TRUECOLOR(r,g,b) (1 << 24 | (r) << 16 | (g) << 8 | (b))
_AT_@ -2892,6 +2891,7 @@ xloadfonts(char *fontstr, double fontsize) {
         FcPattern *pattern;
         FcResult r_sz, r_psz;
         double fontval;
+ float ceilf(float);
 
         if(fontstr[0] == '-') {
                 pattern = XftXlfdParse(fontstr, False, False);
_AT_@ -2937,8 +2937,8 @@ xloadfonts(char *fontstr, double fontsize) {
         }
 
         /* Setting character width and height. */
- xw.cw = CEIL(dc.font.width * cwscale);
- xw.ch = CEIL(dc.font.height * chscale);
+ xw.cw = ceilf(dc.font.width * cwscale);
+ xw.ch = ceilf(dc.font.height * chscale);
 
         FcPatternDel(pattern, FC_SLANT);
         FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_ITALIC);

----
See that if we use a macro for:
	xw.ch = CEIL(dc.font.height * chscale);
then "dc.font.height * chscale" will be computed several times. I
know that moderm compiler will reuse the value, but come on, the
poor compiler has too much work for solving our stupid code.
Regards,
-- 
Roberto E. Vargas Caballero
Received on Tue Jun 24 2014 - 21:16:58 CEST

This archive was generated by hypermail 2.3.0 : Tue Jun 24 2014 - 21:24:06 CEST