subject:"\[PATCH V4 07\/10\] powerpc, lib\: Add new branch instruction analysis support functions"

Re: [PATCH V4 07/10] powerpc, lib: Add new branch instruction analysis support functions

2013-12-20 Thread Anshuman Khandual

On 12/10/2013 11:39 AM, Anshuman Khandual wrote:
> On 12/09/2013 11:51 AM, Michael Ellerman wrote:
>> On Wed, 2013-04-12 at 10:32:39 UTC, Anshuman Khandual wrote:
>>> Generic powerpc branch instruction analysis support added in the code
>>> patching library which will help the subsequent patch on SW based
>>> filtering of branch records in perf. This patch also converts and
>>> exports some of the existing local static functions through the header
>>> file to be used else where.
>>>
>>> diff --git a/arch/powerpc/include/asm/code-patching.h 
>>> b/arch/powerpc/include/asm/code-patching.h
>>> index a6f8c7a..8bab417 100644
>>> --- a/arch/powerpc/include/asm/code-patching.h
>>> +++ b/arch/powerpc/include/asm/code-patching.h
>>> @@ -22,6 +22,36 @@
>>>  #define BRANCH_SET_LINK0x1
>>>  #define BRANCH_ABSOLUTE0x2
>>>  
>>> +#define XL_FORM_LR  0x4C20
>>> +#define XL_FORM_CTR 0x4C000420
>>> +#define XL_FORM_TAR 0x4C000460
>>> +
>>> +#define BO_ALWAYS0x0280
>>> +#define BO_CTR   0x0200
>>> +#define BO_CRBI_OFF  0x0080
>>> +#define BO_CRBI_ON   0x0180
>>> +#define BO_CRBI_HINT 0x0040
>>> +
>>> +/* Forms of branch instruction */
>>> +int instr_is_branch_iform(unsigned int instr);
>>> +int instr_is_branch_bform(unsigned int instr);
>>> +int instr_is_branch_xlform(unsigned int instr);
>>> +
>>> +/* Classification of XL-form instruction */
>>> +int is_xlform_lr(unsigned int instr);
>>> +int is_xlform_ctr(unsigned int instr);
>>> +int is_xlform_tar(unsigned int instr);
>>> +
>>> +/* Branch instruction is a call */
>>> +int is_branch_link_set(unsigned int instr);
>>> +
>>> +/* BO field analysis (B-form or XL-form) */
>>> +int is_bo_always(unsigned int instr);
>>> +int is_bo_ctr(unsigned int instr);
>>> +int is_bo_crbi_off(unsigned int instr);
>>> +int is_bo_crbi_on(unsigned int instr);
>>> +int is_bo_crbi_hint(unsigned int instr);
>>
>>
>> I think this is the wrong API.
>>
>> We end up with all these micro checks, which don't actually encapsulate much,
>> and don't implement the logic perf needs. If we had another user for this 
>> level
>> of detail then it might make sense, but for a single user I think we're 
>> better
>> off just implementing the semantics it wants.
>>
> 
> Having a comprehensive list of branch instruction analysis APIs which some 
> other
> user can also use in the future does not make it wrong. Being more elaborate 
> and
> detailed makes this one a better choice than the API you have suggested below.
> 
>> So that would be something more like:
>>
>> bool instr_is_return_branch(unsigned int instr);
>> bool instr_is_conditional_branch(unsigned int instr);
>> bool instr_is_func_call(unsigned int instr);
>> bool instr_is_indirect_func_call(unsigned int instr);
>>
>>
>> These would then encapsulate something like the logic in your 8/10 patch. You
>> can hopefully also optimise the checking logic in each routine because you 
>> know
>> the exact semantics you're implementing.

Any ways, here is the patch which is will supersede the present patch for adding
required library functions. Hope this works.

commit 9d9f11a6b778b51732aaa0e7c9dea4be3385df56
Author: Anshuman Khandual 
Date:   Fri Dec 20 13:46:15 2013 +0530

powerpc, lib: Add new branch analysis support functions

Generic powerpc branch analysis support added in the code patching
library which will help the subsequent patch on SW based filtering
of branch records in perf.

Signed-off-by: Anshuman Khandual 

diff --git a/arch/powerpc/include/asm/code-patching.h 
b/arch/powerpc/include/asm/code-patching.h
index a6f8c7a..15700b5 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -22,6 +22,16 @@
 #define BRANCH_SET_LINK0x1
 #define BRANCH_ABSOLUTE0x2
 
+#define XL_FORM_LR  0x4C20
+#define XL_FORM_CTR 0x4C000420
+#define XL_FORM_TAR 0x4C000460
+
+#define BO_ALWAYS0x0280
+#define BO_CTR   0x0200
+#define BO_CRBI_OFF  0x0080
+#define BO_CRBI_ON   0x0180
+#define BO_CRBI_HINT 0x0040
+
 unsigned int create_branch(const unsigned int *addr,
   unsigned long target, int flags);
 unsigned int create_cond_branch(const unsigned int *addr,
@@ -49,4 +59,10 @@ static inline unsigned long ppc_function_entry(void *func)
 #endif
 }
 
+/* Perf branch filters */
+bool instr_is_return_branch(unsigned int instr);
+bool instr_is_conditional_branch(unsigned int instr);
+bool instr_is_func_call(unsigned int instr);
+bool instr_is_indirect_func_call(unsigned int instr);
+
 #endif /* _ASM_POWERPC_CODE_PATCHING_H */
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 17e5b23..ad39c58 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -77,6 +77,7 @@ static unsigned int branch_opcode(unsigned int instr)
return (instr >> 26) & 0x3F;
 }
 
+/* Forms of branch instruction */
 static int instr_is_branch_iform(unsigned

Re: [PATCH V4 07/10] powerpc, lib: Add new branch instruction analysis support functions

2013-12-09 Thread Anshuman Khandual

On 12/09/2013 11:51 AM, Michael Ellerman wrote:
> On Wed, 2013-04-12 at 10:32:39 UTC, Anshuman Khandual wrote:
>> Generic powerpc branch instruction analysis support added in the code
>> patching library which will help the subsequent patch on SW based
>> filtering of branch records in perf. This patch also converts and
>> exports some of the existing local static functions through the header
>> file to be used else where.
>>
>> diff --git a/arch/powerpc/include/asm/code-patching.h 
>> b/arch/powerpc/include/asm/code-patching.h
>> index a6f8c7a..8bab417 100644
>> --- a/arch/powerpc/include/asm/code-patching.h
>> +++ b/arch/powerpc/include/asm/code-patching.h
>> @@ -22,6 +22,36 @@
>>  #define BRANCH_SET_LINK 0x1
>>  #define BRANCH_ABSOLUTE 0x2
>>  
>> +#define XL_FORM_LR  0x4C20
>> +#define XL_FORM_CTR 0x4C000420
>> +#define XL_FORM_TAR 0x4C000460
>> +
>> +#define BO_ALWAYS0x0280
>> +#define BO_CTR   0x0200
>> +#define BO_CRBI_OFF  0x0080
>> +#define BO_CRBI_ON   0x0180
>> +#define BO_CRBI_HINT 0x0040
>> +
>> +/* Forms of branch instruction */
>> +int instr_is_branch_iform(unsigned int instr);
>> +int instr_is_branch_bform(unsigned int instr);
>> +int instr_is_branch_xlform(unsigned int instr);
>> +
>> +/* Classification of XL-form instruction */
>> +int is_xlform_lr(unsigned int instr);
>> +int is_xlform_ctr(unsigned int instr);
>> +int is_xlform_tar(unsigned int instr);
>> +
>> +/* Branch instruction is a call */
>> +int is_branch_link_set(unsigned int instr);
>> +
>> +/* BO field analysis (B-form or XL-form) */
>> +int is_bo_always(unsigned int instr);
>> +int is_bo_ctr(unsigned int instr);
>> +int is_bo_crbi_off(unsigned int instr);
>> +int is_bo_crbi_on(unsigned int instr);
>> +int is_bo_crbi_hint(unsigned int instr);
> 
> 
> I think this is the wrong API.
> 
> We end up with all these micro checks, which don't actually encapsulate much,
> and don't implement the logic perf needs. If we had another user for this 
> level
> of detail then it might make sense, but for a single user I think we're better
> off just implementing the semantics it wants.
> 

Having a comprehensive list of branch instruction analysis APIs which some other
user can also use in the future does not make it wrong. Being more elaborate and
detailed makes this one a better choice than the API you have suggested below.

> So that would be something more like:
> 
> bool instr_is_return_branch(unsigned int instr);
> bool instr_is_conditional_branch(unsigned int instr);
> bool instr_is_func_call(unsigned int instr);
> bool instr_is_indirect_func_call(unsigned int instr);
> 
> 
> These would then encapsulate something like the logic in your 8/10 patch. You
> can hopefully also optimise the checking logic in each routine because you 
> know
> the exact semantics you're implementing.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V4 07/10] powerpc, lib: Add new branch instruction analysis support functions

2013-12-08 Thread Michael Ellerman

On Wed, 2013-04-12 at 10:32:39 UTC, Anshuman Khandual wrote:
> Generic powerpc branch instruction analysis support added in the code
> patching library which will help the subsequent patch on SW based
> filtering of branch records in perf. This patch also converts and
> exports some of the existing local static functions through the header
> file to be used else where.
> 
> diff --git a/arch/powerpc/include/asm/code-patching.h 
> b/arch/powerpc/include/asm/code-patching.h
> index a6f8c7a..8bab417 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -22,6 +22,36 @@
>  #define BRANCH_SET_LINK  0x1
>  #define BRANCH_ABSOLUTE  0x2
>  
> +#define XL_FORM_LR  0x4C20
> +#define XL_FORM_CTR 0x4C000420
> +#define XL_FORM_TAR 0x4C000460
> +
> +#define BO_ALWAYS0x0280
> +#define BO_CTR   0x0200
> +#define BO_CRBI_OFF  0x0080
> +#define BO_CRBI_ON   0x0180
> +#define BO_CRBI_HINT 0x0040
> +
> +/* Forms of branch instruction */
> +int instr_is_branch_iform(unsigned int instr);
> +int instr_is_branch_bform(unsigned int instr);
> +int instr_is_branch_xlform(unsigned int instr);
> +
> +/* Classification of XL-form instruction */
> +int is_xlform_lr(unsigned int instr);
> +int is_xlform_ctr(unsigned int instr);
> +int is_xlform_tar(unsigned int instr);
> +
> +/* Branch instruction is a call */
> +int is_branch_link_set(unsigned int instr);
> +
> +/* BO field analysis (B-form or XL-form) */
> +int is_bo_always(unsigned int instr);
> +int is_bo_ctr(unsigned int instr);
> +int is_bo_crbi_off(unsigned int instr);
> +int is_bo_crbi_on(unsigned int instr);
> +int is_bo_crbi_hint(unsigned int instr);


I think this is the wrong API.

We end up with all these micro checks, which don't actually encapsulate much,
and don't implement the logic perf needs. If we had another user for this level
of detail then it might make sense, but for a single user I think we're better
off just implementing the semantics it wants.

So that would be something more like:

bool instr_is_return_branch(unsigned int instr);
bool instr_is_conditional_branch(unsigned int instr);
bool instr_is_func_call(unsigned int instr);
bool instr_is_indirect_func_call(unsigned int instr);


These would then encapsulate something like the logic in your 8/10 patch. You
can hopefully also optimise the checking logic in each routine because you know
the exact semantics you're implementing.

cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH V4 07/10] powerpc, lib: Add new branch instruction analysis support functions

2013-12-04 Thread Anshuman Khandual

Generic powerpc branch instruction analysis support added in the code
patching library which will help the subsequent patch on SW based
filtering of branch records in perf. This patch also converts and
exports some of the existing local static functions through the header
file to be used else where.

Signed-off-by: Anshuman Khandual 
---
 arch/powerpc/include/asm/code-patching.h | 30 ++
 arch/powerpc/lib/code-patching.c | 54 ++--
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h 
b/arch/powerpc/include/asm/code-patching.h
index a6f8c7a..8bab417 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -22,6 +22,36 @@
 #define BRANCH_SET_LINK0x1
 #define BRANCH_ABSOLUTE0x2
 
+#define XL_FORM_LR  0x4C20
+#define XL_FORM_CTR 0x4C000420
+#define XL_FORM_TAR 0x4C000460
+
+#define BO_ALWAYS0x0280
+#define BO_CTR   0x0200
+#define BO_CRBI_OFF  0x0080
+#define BO_CRBI_ON   0x0180
+#define BO_CRBI_HINT 0x0040
+
+/* Forms of branch instruction */
+int instr_is_branch_iform(unsigned int instr);
+int instr_is_branch_bform(unsigned int instr);
+int instr_is_branch_xlform(unsigned int instr);
+
+/* Classification of XL-form instruction */
+int is_xlform_lr(unsigned int instr);
+int is_xlform_ctr(unsigned int instr);
+int is_xlform_tar(unsigned int instr);
+
+/* Branch instruction is a call */
+int is_branch_link_set(unsigned int instr);
+
+/* BO field analysis (B-form or XL-form) */
+int is_bo_always(unsigned int instr);
+int is_bo_ctr(unsigned int instr);
+int is_bo_crbi_off(unsigned int instr);
+int is_bo_crbi_on(unsigned int instr);
+int is_bo_crbi_hint(unsigned int instr);
+
 unsigned int create_branch(const unsigned int *addr,
   unsigned long target, int flags);
 unsigned int create_cond_branch(const unsigned int *addr,
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 17e5b23..cb62bd8 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -77,16 +77,66 @@ static unsigned int branch_opcode(unsigned int instr)
return (instr >> 26) & 0x3F;
 }
 
-static int instr_is_branch_iform(unsigned int instr)
+int instr_is_branch_iform(unsigned int instr)
 {
return branch_opcode(instr) == 18;
 }
 
-static int instr_is_branch_bform(unsigned int instr)
+int instr_is_branch_bform(unsigned int instr)
 {
return branch_opcode(instr) == 16;
 }
 
+int instr_is_branch_xlform(unsigned int instr)
+{
+   return branch_opcode(instr) == 19;
+}
+
+int is_xlform_lr(unsigned int instr)
+{
+   return (instr & XL_FORM_LR) == XL_FORM_LR;
+}
+
+int is_xlform_ctr(unsigned int instr)
+{
+   return (instr & XL_FORM_CTR) == XL_FORM_CTR;
+}
+
+int is_xlform_tar(unsigned int instr)
+{
+   return (instr & XL_FORM_TAR) == XL_FORM_TAR;
+}
+
+int is_branch_link_set(unsigned int instr)
+{
+   return (instr & BRANCH_SET_LINK) == BRANCH_SET_LINK;
+}
+
+int is_bo_always(unsigned int instr)
+{
+   return (instr & BO_ALWAYS) == BO_ALWAYS;
+}
+
+int is_bo_ctr(unsigned int instr)
+{
+   return (instr & BO_CTR) == BO_CTR;
+}
+
+int is_bo_crbi_off(unsigned int instr)
+{
+   return (instr & BO_CRBI_OFF) == BO_CRBI_OFF;
+}
+
+int is_bo_crbi_on(unsigned int instr)
+{
+   return (instr & BO_CRBI_ON) == BO_CRBI_ON;
+}
+
+int is_bo_crbi_hint(unsigned int instr)
+{
+   return (instr & BO_CRBI_HINT) == BO_CRBI_HINT;
+}
+
 int instr_is_relative_branch(unsigned int instr)
 {
if (instr & BRANCH_ABSOLUTE)
-- 
1.7.11.7

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V4 07/10] powerpc, lib: Add new branch instruction analysis support functions

Re: [PATCH V4 07/10] powerpc, lib: Add new branch instruction analysis support functions

Re: [PATCH V4 07/10] powerpc, lib: Add new branch instruction analysis support functions

[PATCH V4 07/10] powerpc, lib: Add new branch instruction analysis support functions

4 matches

Site Navigation

Mail list logo

Footer information