[PATCH 2/8] opensm/complib: define "if" statements with branch prediction hints

[PATCH 2/8] opensm/complib: define "if" statements with branch prediction hints

[Yevgeny Kliteynik]

Defined "if_PT" and "if_PF" for "predict true"
and "predict false" respectively.

Signed-off-by: Yevgeny Kliteynik <kliteyn-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
---
 include/complib/cl_types_osd.h | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/complib/cl_types_osd.h b/include/complib/cl_types_osd.h
index ce1a452..f9abb28 100644
--- a/include/complib/cl_types_osd.h
+++ b/include/complib/cl_types_osd.h
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
- * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
+ * Copyright (c) 2002-2012 Mellanox Technologies LTD. All rights reserved.
  * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
  *
  * This software is available to you under a choice of one of two
@@ -64,6 +64,21 @@ BEGIN_C_DECLS
 #include <inttypes.h>
 #include <assert.h>
 #include <string.h>
+
+/*
+ * Branch prediction hints
+ */
+#if defined(HAVE_BUILTIN_EXPECT)
+#define CL_PREDICT_TRUE(exp)	__builtin_expect( ((uintptr_t)(exp)), 1 )
+#define CL_PREDICT_FALSE(exp)	__builtin_expect( ((uintptr_t)(exp)), 0 )
+#else
+#define CL_PREDICT_TRUE(exp)	(exp)
+#define CL_PREDICT_FALSE(exp)	(exp)
+#endif
+
+#define if_PF(cond)		if(CL_PREDICT_FALSE(cond))
+#define if_PT(cond)		if(CL_PREDICT_TRUE(cond))
+
 #if defined (_DEBUG_)
 #define CL_ASSERT	assert
 #else				/* _DEBUG_ */
-- 
1.7.11.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 2/8] opensm/complib: define "if" statements with branch prediction hints

[Hefty, Sean]

> +#define if_PF(cond)		if(CL_PREDICT_FALSE(cond))
> +#define if_PT(cond)		if(CL_PREDICT_TRUE(cond))

If CL_PREDICT_TRUE/FALSE are too long, why not just shorten those, rather than abstract if statements behind a macro?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] opensm/complib: define "if" statements with branch prediction hints

[Jason Gunthorpe]

On Tue, Aug 14, 2012 at 09:39:23PM +0000, Hefty, Sean wrote:
> > +#define if_PF(cond)		if(CL_PREDICT_FALSE(cond))
> > +#define if_PT(cond)		if(CL_PREDICT_TRUE(cond))
> 
> If CL_PREDICT_TRUE/FALSE are too long, why not just shorten those,
> rather than abstract if statements behind a macro?

If you are typing these so often (ie more than in some datastructure
kernels) that the length is a problem then you really should be using
profile guided optimization instead...

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] opensm/complib: define "if" statements with branch prediction hints

[Yevgeny Kliteynik]

Hi,

Sorry for the delay with a response - just got back from a long vacation.

On 8/15/2012 12:54 AM, Jason Gunthorpe wrote:
> On Tue, Aug 14, 2012 at 09:39:23PM +0000, Hefty, Sean wrote:
>>> +#define if_PF(cond)		if(CL_PREDICT_FALSE(cond))
>>> +#define if_PT(cond)		if(CL_PREDICT_TRUE(cond))
>>
>> If CL_PREDICT_TRUE/FALSE are too long, why not just shorten those,
>> rather than abstract if statements behind a macro?

It's not the size - it's just the readability of the macro.
First macro (CL_PREDICT_*) is close to the GCC's way to define
it. Second macro (if_PF/T) is close to the usual 'if' statement
syntax.

If you think it's a deal breaker, I can lose the first macro and
have everything in one macro.

> If you are typing these so often (ie more than in some datastructure
> kernels) that the length is a problem then you really should be using
> profile guided optimization instead...

I'm trying to optimize all the 'if' conditions in the SM.
As I described in patch 3/8 (next patch in these series),
I don't touch all the non-fatal cases. The goal is not to
help SM to perform better in general (in which case I agree
than PGO would be the right way to go), but to make sure
that the fatal cases will never be predicted wrong, so any
fatal check (existing or a newly added one) won't affect
the SM performance.

Also, the plan is to define a new ASSERT macro that will
use this if_PF thing to make sure that ASSERT statement
doesn't cost much (if any).

-- YK

> Jason
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] opensm/complib: define "if" statements with branch prediction hints

[Yevgeny Kliteynik]

On 8/26/2012 4:43 PM, Yevgeny Kliteynik wrote:
> 
>> If you are typing these so often (ie more than in some datastructure
>> kernels) that the length is a problem then you really should be using
>> profile guided optimization instead...
> 
> I'm trying to optimize all the 'if' conditions in the SM.

Obviously, what I meant is that I'm *not* trying to optimize
all the 'if' conditions in the SM...  :)

-- YK

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 2/8] opensm/complib: define "if" statements with branch prediction hints

[Hefty, Sean]

> On 8/15/2012 12:54 AM, Jason Gunthorpe wrote:
> > On Tue, Aug 14, 2012 at 09:39:23PM +0000, Hefty, Sean wrote:
> >>> +#define if_PF(cond)		if(CL_PREDICT_FALSE(cond))
> >>> +#define if_PT(cond)		if(CL_PREDICT_TRUE(cond))
> >>
> >> If CL_PREDICT_TRUE/FALSE are too long, why not just shorten those,
> >> rather than abstract if statements behind a macro?
> 
> It's not the size - it's just the readability of the macro.
> First macro (CL_PREDICT_*) is close to the GCC's way to define
> it. Second macro (if_PF/T) is close to the usual 'if' statement
> syntax.

I would not abstract the 'if' statement.  If CL_PREDICT_FALSE/TRUE are not readable, then shorten those.

if (PF(...))

is just as readable as

if_PF(...)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] opensm/complib: define "if" statements with branch prediction hints

[Yevgeny Kliteynik]

> 
> I would not abstract the 'if' statement.  If CL_PREDICT_FALSE/TRUE are not readable, then shorten those.
> 
> if (PF(...))
> 
> is just as readable as
> 
> if_PF(...)

OK, agree.
I'll issue a v2 shortly - the only difference would be
change in this macro and rebase to the updated trunk.

-- YK

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html