All of lore.kernel.org
 help / color / mirror / Atom feed
* [cocci] Iterator macros
@ 2024-08-13  6:15 Peter Senna Tschudin
  2024-08-13  6:19 ` Julia Lawall
  2024-08-14  9:44 ` Julia Lawall
  0 siblings, 2 replies; 12+ messages in thread
From: Peter Senna Tschudin @ 2024-08-13  6:15 UTC (permalink / raw)
  To: cocci

Dear List, 

I am trying to make changes to a macro iterator from igt-gpu-tools*, and I am failing to use Coccinelle. My current attempt only captures about 1/2 of call sites, and I am not really sure how to ask Coccinelle to make the changes I want. Here is the pattern I am trying to change: 


From: 

const struct intel_execution_engine2 *e2; 
... 
for_each_ctx_engine(i915, ctx, e2) { 
    execbuf.flags = e2->flags; 
    gem_execbuf(i915, &execbuf); 
} 


To: 

struct intel_engine_data ied;
... 
for_each_ctx_engine(i915, ctx, ied) { 
    execbuf.flags = ied.current_engine->flags; 
    gem_execbuf(i915, &execbuf); 
} 

Changes I want: 
1. From `const struct intel_execution_engine2 *e2` to `struct intel_engine_data ied`. 
2. Replace the last argument of for_each_ctx_engine() following the change on previous step. 
3. Change references from `e2` to `ied.current_engine` inside the iterator statement. 

Here is my starting point that finds about half of the call sites: 

@r1@ 
iterator I =~ "for_each_ctx_engine"; 
statement S; 
@@ 
* I(...) 
S 

I assume that I should create other rules and import I from r1 as the core of the changes I want. Can you point me to an example to help me here?

Thank you,

Peter

* - https://gitlab.freedesktop.org/drm/igt-gpu-tools 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13  6:15 [cocci] Iterator macros Peter Senna Tschudin
@ 2024-08-13  6:19 ` Julia Lawall
  2024-08-13  6:27   ` Peter Senna Tschudin
  2024-08-14  9:44 ` Julia Lawall
  1 sibling, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2024-08-13  6:19 UTC (permalink / raw)
  To: Peter Senna Tschudin; +Cc: cocci



On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:

> Dear List,
>
> I am trying to make changes to a macro iterator from igt-gpu-tools*, and I am failing to use Coccinelle. My current attempt only captures about 1/2 of call sites, and I am not really sure how to ask Coccinelle to make the changes I want. Here is the pattern I am trying to change:
>
>
> From:
>
> const struct intel_execution_engine2 *e2;
> ...
> for_each_ctx_engine(i915, ctx, e2) {
>     execbuf.flags = e2->flags;
>     gem_execbuf(i915, &execbuf);
> }
>
>
> To:
>
> struct intel_engine_data ied;
> ...
> for_each_ctx_engine(i915, ctx, ied) {
>     execbuf.flags = ied.current_engine->flags;
>     gem_execbuf(i915, &execbuf);
> }
>
> Changes I want:
> 1. From `const struct intel_execution_engine2 *e2` to `struct intel_engine_data ied`.
> 2. Replace the last argument of for_each_ctx_engine() following the change on previous step.
> 3. Change references from `e2` to `ied.current_engine` inside the iterator statement.
>
> Here is my starting point that finds about half of the call sites:
>
> @r1@
> iterator I =~ "for_each_ctx_engine";
> statement S;
> @@
> * I(...)
> S
>
> I assume that I should create other rules and import I from r1 as the
> core of the changes I want. Can you point me to an example to help me
> here?

Are you just looking for

iterator r.I;

If you have only one iterator name, you don't need the regexp.  Just put

iterator name for_each_ctx_engine;

julia

>
> Thank you,
>
> Peter
>
> * - https://gitlab.freedesktop.org/drm/igt-gpu-tools
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13  6:19 ` Julia Lawall
@ 2024-08-13  6:27   ` Peter Senna Tschudin
  2024-08-13  6:34     ` Julia Lawall
  2024-08-14  6:22     ` Julia Lawall
  0 siblings, 2 replies; 12+ messages in thread
From: Peter Senna Tschudin @ 2024-08-13  6:27 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Peter Senna Tschudin, cocci

[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]

Hi Julia,



On Tue, Aug 13, 2024 at 8:19 AM Julia Lawall <julia.lawall@inria.fr> wrote:

>
>
> On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:
>
> > Dear List,
> >
> > I am trying to make changes to a macro iterator from igt-gpu-tools*, and
> I am failing to use Coccinelle. My current attempt only captures about 1/2
> of call sites, and I am not really sure how to ask Coccinelle to make the
> changes I want. Here is the pattern I am trying to change:
> >
> >
> > From:
> >
> > const struct intel_execution_engine2 *e2;
> > ...
> > for_each_ctx_engine(i915, ctx, e2) {
> >     execbuf.flags = e2->flags;
> >     gem_execbuf(i915, &execbuf);
> > }
> >
> >
> > To:
> >
> > struct intel_engine_data ied;
> > ...
> > for_each_ctx_engine(i915, ctx, ied) {
> >     execbuf.flags = ied.current_engine->flags;
> >     gem_execbuf(i915, &execbuf);
> > }
> >
> > Changes I want:
> > 1. From `const struct intel_execution_engine2 *e2` to `struct
> intel_engine_data ied`.
> > 2. Replace the last argument of for_each_ctx_engine() following the
> change on previous step.
> > 3. Change references from `e2` to `ied.current_engine` inside the
> iterator statement.
> >
> > Here is my starting point that finds about half of the call sites:
> >
> > @r1@
> > iterator I =~ "for_each_ctx_engine";
> > statement S;
> > @@
> > * I(...)
> > S
> >
> > I assume that I should create other rules and import I from r1 as the
> > core of the changes I want. Can you point me to an example to help me
> > here?
>
> Are you just looking for
>
> iterator r.I;
>
> If you have only one iterator name, you don't need the regexp.  Just put
>
> iterator name for_each_ctx_engine;
>
Thank you! This is much faster but captures one instance less.


>
> julia
>
> >
> > Thank you,
> >
> > Peter
> >
> > * - https://gitlab.freedesktop.org/drm/igt-gpu-tools
> >
>


-- 
                         Peter

[-- Attachment #2: Type: text/html, Size: 2983 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13  6:27   ` Peter Senna Tschudin
@ 2024-08-13  6:34     ` Julia Lawall
  2024-08-13 11:05       ` Andrzej Hajda
  2024-08-14  6:22     ` Julia Lawall
  1 sibling, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2024-08-13  6:34 UTC (permalink / raw)
  To: Peter Senna Tschudin; +Cc: Peter Senna Tschudin, cocci

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]



On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:

> Hi Julia,
>
>
>
> On Tue, Aug 13, 2024 at 8:19 AM Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>       On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:
>
>       > Dear List,
>       >
>       > I am trying to make changes to a macro iterator from
>       igt-gpu-tools*, and I am failing to use Coccinelle. My current
>       attempt only captures about 1/2 of call sites, and I am not
>       really sure how to ask Coccinelle to make the changes I want.
>       Here is the pattern I am trying to change:
>       >
>       >
>       > From:
>       >
>       > const struct intel_execution_engine2 *e2;
>       > ...
>       > for_each_ctx_engine(i915, ctx, e2) {
>       >     execbuf.flags = e2->flags;
>       >     gem_execbuf(i915, &execbuf);
>       > }
>       >
>       >
>       > To:
>       >
>       > struct intel_engine_data ied;
>       > ...
>       > for_each_ctx_engine(i915, ctx, ied) {
>       >     execbuf.flags = ied.current_engine->flags;
>       >     gem_execbuf(i915, &execbuf);
>       > }
>       >
>       > Changes I want:
>       > 1. From `const struct intel_execution_engine2 *e2` to `struct
>       intel_engine_data ied`.
>       > 2. Replace the last argument of for_each_ctx_engine()
>       following the change on previous step.
>       > 3. Change references from `e2` to `ied.current_engine` inside
>       the iterator statement.
>       >
>       > Here is my starting point that finds about half of the call
>       sites:
>       >
>       > @r1@
>       > iterator I =~ "for_each_ctx_engine";
>       > statement S;
>       > @@
>       > * I(...)
>       > S
>       >
>       > I assume that I should create other rules and import I from r1
>       as the
>       > core of the changes I want. Can you point me to an example to
>       help me
>       > here?
>
>       Are you just looking for
>
>       iterator r.I;
>
>       If you have only one iterator name, you don't need the regexp. 
>       Just put
>
>       iterator name for_each_ctx_engine;
>
> Thank you! This is much faster but captures one instance less.

You can use multiple names like

(name1@I|name2@I|name3@I)(...) S

The you can use r.I in your other rules.  name1 etc would be iterator name
and I would be iterator.

julia

>  
>
>       julia
>
>       >
>       > Thank you,
>       >
>       > Peter
>       >
>       > * - https://gitlab.freedesktop.org/drm/igt-gpu-tools
>       >
>
>
>
> --
>                          Peter
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13  6:34     ` Julia Lawall
@ 2024-08-13 11:05       ` Andrzej Hajda
  2024-08-13 11:19         ` Julia Lawall
                           ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Andrzej Hajda @ 2024-08-13 11:05 UTC (permalink / raw)
  To: Julia Lawall, Peter Senna Tschudin; +Cc: Peter Senna Tschudin, cocci



On 13.08.2024 08:34, Julia Lawall wrote:
>
> On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:
>
>> Hi Julia,
>>
>>
>>
>> On Tue, Aug 13, 2024 at 8:19 AM Julia Lawall <julia.lawall@inria.fr> wrote:
>>
>>
>>        On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:
>>
>>        > Dear List,
>>        >
>>        > I am trying to make changes to a macro iterator from
>>        igt-gpu-tools*, and I am failing to use Coccinelle. My current
>>        attempt only captures about 1/2 of call sites, and I am not
>>        really sure how to ask Coccinelle to make the changes I want.
>>        Here is the pattern I am trying to change:
>>        >
>>        >
>>        > From:
>>        >
>>        > const struct intel_execution_engine2 *e2;
>>        > ...
>>        > for_each_ctx_engine(i915, ctx, e2) {
>>        >     execbuf.flags = e2->flags;
>>        >     gem_execbuf(i915, &execbuf);
>>        > }
>>        >
>>        >
>>        > To:
>>        >
>>        > struct intel_engine_data ied;
>>        > ...
>>        > for_each_ctx_engine(i915, ctx, ied) {
>>        >     execbuf.flags = ied.current_engine->flags;
>>        >     gem_execbuf(i915, &execbuf);
>>        > }
>>        >
>>        > Changes I want:
>>        > 1. From `const struct intel_execution_engine2 *e2` to `struct
>>        intel_engine_data ied`.
>>        > 2. Replace the last argument of for_each_ctx_engine()
>>        following the change on previous step.
>>        > 3. Change references from `e2` to `ied.current_engine` inside
>>        the iterator statement.
>>        >
>>        > Here is my starting point that finds about half of the call
>>        sites:
>>        >
>>        > @r1@
>>        > iterator I =~ "for_each_ctx_engine";
>>        > statement S;
>>        > @@
>>        > * I(...)
>>        > S
>>        >
>>        > I assume that I should create other rules and import I from r1
>>        as the
>>        > core of the changes I want. Can you point me to an example to
>>        help me
>>        > here?
>>
>>        Are you just looking for
>>
>>        iterator r.I;
>>
>>        If you have only one iterator name, you don't need the regexp.
>>        Just put
>>
>>        iterator name for_each_ctx_engine;
>>
>> Thank you! This is much faster but captures one instance less.
> You can use multiple names like
>
> (name1@I|name2@I|name3@I)(...) S
>
> The you can use r.I in your other rules.  name1 etc would be iterator name
> and I would be iterator.

I guess the issue here could be a problem with parsing igt code (it is 
also an issue with kernel drivers using iterators).
Cocci quite often does not recognize automatically iterators or other 
constructs, and does not scan them at all.
Try run for example:
$ spatch --parse-c tests/intel/gem_ctx_exec.c
...
NB total files = 1; perfect = 0; pbs = 1; timeout = 0; =========> 0%
nb good = 385,  nb passed = 19 =========> 3.12% passed
nb good = 385,  nb bad = 204 =========> 66.45% good or passed

As I understand, lines marked as "bad" are skipped during cocci pass, am 
I right?
Some workaround is to define iterators by hand in the script or include 
files.

Regards
Andrzej

>
> julia
>
>>   
>>
>>        julia
>>
>>        >
>>        > Thank you,
>>        >
>>        > Peter
>>        >
>>        > * - https://gitlab.freedesktop.org/drm/igt-gpu-tools
>>        >
>>
>>
>>
>> --
>>                           Peter
>>
> >


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13 11:05       ` Andrzej Hajda
@ 2024-08-13 11:19         ` Julia Lawall
  2024-08-14  7:19         ` Julia Lawall
  2024-08-14  7:21         ` Julia Lawall
  2 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2024-08-13 11:19 UTC (permalink / raw)
  To: Andrzej Hajda; +Cc: Peter Senna Tschudin, Peter Senna Tschudin, cocci

[-- Attachment #1: Type: text/plain, Size: 4437 bytes --]



On Tue, 13 Aug 2024, Andrzej Hajda wrote:

>
>
> On 13.08.2024 08:34, Julia Lawall wrote:
> >
> > On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:
> >
> > > Hi Julia,
> > >
> > >
> > >
> > > On Tue, Aug 13, 2024 at 8:19 AM Julia Lawall <julia.lawall@inria.fr>
> > > wrote:
> > >
> > >
> > >        On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:
> > >
> > >        > Dear List,
> > >        >
> > >        > I am trying to make changes to a macro iterator from
> > >        igt-gpu-tools*, and I am failing to use Coccinelle. My current
> > >        attempt only captures about 1/2 of call sites, and I am not
> > >        really sure how to ask Coccinelle to make the changes I want.
> > >        Here is the pattern I am trying to change:
> > >        >
> > >        >
> > >        > From:
> > >        >
> > >        > const struct intel_execution_engine2 *e2;
> > >        > ...
> > >        > for_each_ctx_engine(i915, ctx, e2) {
> > >        >     execbuf.flags = e2->flags;
> > >        >     gem_execbuf(i915, &execbuf);
> > >        > }
> > >        >
> > >        >
> > >        > To:
> > >        >
> > >        > struct intel_engine_data ied;
> > >        > ...
> > >        > for_each_ctx_engine(i915, ctx, ied) {
> > >        >     execbuf.flags = ied.current_engine->flags;
> > >        >     gem_execbuf(i915, &execbuf);
> > >        > }
> > >        >
> > >        > Changes I want:
> > >        > 1. From `const struct intel_execution_engine2 *e2` to `struct
> > >        intel_engine_data ied`.
> > >        > 2. Replace the last argument of for_each_ctx_engine()
> > >        following the change on previous step.
> > >        > 3. Change references from `e2` to `ied.current_engine` inside
> > >        the iterator statement.
> > >        >
> > >        > Here is my starting point that finds about half of the call
> > >        sites:
> > >        >
> > >        > @r1@
> > >        > iterator I =~ "for_each_ctx_engine";
> > >        > statement S;
> > >        > @@
> > >        > * I(...)
> > >        > S
> > >        >
> > >        > I assume that I should create other rules and import I from r1
> > >        as the
> > >        > core of the changes I want. Can you point me to an example to
> > >        help me
> > >        > here?
> > >
> > >        Are you just looking for
> > >
> > >        iterator r.I;
> > >
> > >        If you have only one iterator name, you don't need the regexp.
> > >        Just put
> > >
> > >        iterator name for_each_ctx_engine;
> > >
> > > Thank you! This is much faster but captures one instance less.
> > You can use multiple names like
> >
> > (name1@I|name2@I|name3@I)(...) S
> >
> > The you can use r.I in your other rules.  name1 etc would be iterator name
> > and I would be iterator.
>
> I guess the issue here could be a problem with parsing igt code (it is also an
> issue with kernel drivers using iterators).
> Cocci quite often does not recognize automatically iterators or other
> constructs, and does not scan them at all.
> Try run for example:
> $ spatch --parse-c tests/intel/gem_ctx_exec.c
> ...
> NB total files = 1; perfect = 0; pbs = 1; timeout = 0; =========> 0%
> nb good = 385,  nb passed = 19 =========> 3.12% passed
> nb good = 385,  nb bad = 204 =========> 66.45% good or passed
>
> As I understand, lines marked as "bad" are skipped during cocci pass, am I
> right?
> Some workaround is to define iterators by hand in the script or include files.

Coccinelle has some heuristics for detecting iterators, typically tarting
with "for" and having the indentation of a for loop.

In a .h file provided with --macro-file-builtins, it seems that you can
use YACFE_ITERATOR, but there is currently no example of that.

Finally, if you declare an iterator name in your semantic patch, that
information will be propagated to the parsing of the C code.  On the other
hand, Peter's initial regexp solution would not have triggered that.
Perhaps his performance improvement was actually due to being able to
parse the code immediately rather than iterating over some heuristics.

julia

>
> Regards
> Andrzej
>
> >
> > julia
> >
> > >
> > >        julia
> > >
> > >        >
> > >        > Thank you,
> > >        >
> > >        > Peter
> > >        >
> > >        > * - https://gitlab.freedesktop.org/drm/igt-gpu-tools
> > >        >
> > >
> > >
> > >
> > > --
> > >                           Peter
> > >
> > >
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13  6:27   ` Peter Senna Tschudin
  2024-08-13  6:34     ` Julia Lawall
@ 2024-08-14  6:22     ` Julia Lawall
  1 sibling, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2024-08-14  6:22 UTC (permalink / raw)
  To: Peter Senna Tschudin; +Cc: Peter Senna Tschudin, cocci

[-- Attachment #1: Type: text/plain, Size: 2501 bytes --]



On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:

> Hi Julia,
>
>
>
> On Tue, Aug 13, 2024 at 8:19 AM Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>       On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:
>
>       > Dear List,
>       >
>       > I am trying to make changes to a macro iterator from igt-gpu-tools*, and I am failing to use Coccinelle. My current attempt only captures about 1/2 of call sites, and I am not really sure how
>       to ask Coccinelle to make the changes I want. Here is the pattern I am trying to change:
>       >
>       >
>       > From:
>       >
>       > const struct intel_execution_engine2 *e2;
>       > ...
>       > for_each_ctx_engine(i915, ctx, e2) {
>       >     execbuf.flags = e2->flags;
>       >     gem_execbuf(i915, &execbuf);
>       > }
>       >
>       >
>       > To:
>       >
>       > struct intel_engine_data ied;
>       > ...
>       > for_each_ctx_engine(i915, ctx, ied) {
>       >     execbuf.flags = ied.current_engine->flags;
>       >     gem_execbuf(i915, &execbuf);
>       > }
>       >
>       > Changes I want:
>       > 1. From `const struct intel_execution_engine2 *e2` to `struct intel_engine_data ied`.
>       > 2. Replace the last argument of for_each_ctx_engine() following the change on previous step.
>       > 3. Change references from `e2` to `ied.current_engine` inside the iterator statement.
>       >
>       > Here is my starting point that finds about half of the call sites:
>       >
>       > @r1@
>       > iterator I =~ "for_each_ctx_engine";
>       > statement S;
>       > @@
>       > * I(...)
>       > S
>       >
>       > I assume that I should create other rules and import I from r1 as the
>       > core of the changes I want. Can you point me to an example to help me
>       > here?
>
>       Are you just looking for
>
>       iterator r.I;
>
>       If you have only one iterator name, you don't need the regexp.  Just put
>
>       iterator name for_each_ctx_engine;
>
> Thank you! This is much faster but captures one instance less.

Which one are you missing?  I downloaded the code and tried it, and I got
exactly the same results with both semantic patches, with the one with
iterator name being 3x faster (19 seconds -> 6 seconds).

julia

>  
>
>       julia
>
>       >
>       > Thank you,
>       >
>       > Peter
>       >
>       > * - https://gitlab.freedesktop.org/drm/igt-gpu-tools
>       >
>
>
>
> --
>                          Peter
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13 11:05       ` Andrzej Hajda
  2024-08-13 11:19         ` Julia Lawall
@ 2024-08-14  7:19         ` Julia Lawall
  2024-08-14  7:57           ` Andrzej Hajda
  2024-08-14  7:21         ` Julia Lawall
  2 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2024-08-14  7:19 UTC (permalink / raw)
  To: Andrzej Hajda
  Cc: Julia Lawall, Peter Senna Tschudin, Peter Senna Tschudin, cocci

[-- Attachment #1: Type: text/plain, Size: 1576 bytes --]

> I guess the issue here could be a problem with parsing igt code (it is also an
> issue with kernel drivers using iterators).
> Cocci quite often does not recognize automatically iterators or other
> constructs, and does not scan them at all.
> Try run for example:
> $ spatch --parse-c tests/intel/gem_ctx_exec.c
> ...
> NB total files = 1; perfect = 0; pbs = 1; timeout = 0; =========> 0%
> nb good = 385,  nb passed = 19 =========> 3.12% passed
> nb good = 385,  nb bad = 204 =========> 66.45% good or passed
>
> As I understand, lines marked as "bad" are skipped during cocci pass, am I
> right?
> Some workaround is to define iterators by hand in the script or include files.

The following file:

#define igt_main int main ()
#define igt_simple_main int main ()

#define igt_fixture
#define igt_subtest_group
#define igt_dynamic(x) YACFE_ITERATOR
#define igt_dynamic_f(x) YACFE_ITERATOR
#define igt_subtest(x) YACFE_ITERATOR
#define igt_subtest_with_dynamic(x) YACFE_ITERATOR
#define igt_subtest_with_dynamic_f(x) YACFE_ITERATOR
#define igt_subtest_f(x) YACFE_ITERATOR
#define igt_until_timeout(x) YACFE_ITERATOR
#define igt_while_interruptible(x) YACFE_ITERATOR
#define igt_list_for_each_entry_safe_reverse(x) YACFE_ITERATOR
#define igt_list_for_each_entry_safe(x) YACFE_ITERATOR
#define igt_list_for_each_entry(x) YACFE_ITERATOR
#define _Atomic(x) x

managed to parse over 98% of the lines in

https://gitlab.freedesktop.org/drm/igt-gpu-tools

Command line spatch --parse-c . --macro-file-builtins igt.h

I'll check on the driver file you mentioned.  Thanks.

julia

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13 11:05       ` Andrzej Hajda
  2024-08-13 11:19         ` Julia Lawall
  2024-08-14  7:19         ` Julia Lawall
@ 2024-08-14  7:21         ` Julia Lawall
  2 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2024-08-14  7:21 UTC (permalink / raw)
  To: Andrzej Hajda; +Cc: Peter Senna Tschudin, Peter Senna Tschudin, cocci

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

> Try run for example:
> $ spatch --parse-c tests/intel/gem_ctx_exec.c
> ...
> NB total files = 1; perfect = 0; pbs = 1; timeout = 0; =========> 0%
> nb good = 385,  nb passed = 19 =========> 3.12% passed
> nb good = 385,  nb bad = 204 =========> 66.45% good or passed
>
> As I understand, lines marked as "bad" are skipped during cocci pass, am I
> right?
> Some workaround is to define iterators by hand in the script or include files.

With the definitions I mentioned, this file is at 100%.

julia

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-14  7:19         ` Julia Lawall
@ 2024-08-14  7:57           ` Andrzej Hajda
  2024-08-14  8:02             ` Julia Lawall
  0 siblings, 1 reply; 12+ messages in thread
From: Andrzej Hajda @ 2024-08-14  7:57 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Peter Senna Tschudin, Peter Senna Tschudin, cocci



On 14.08.2024 09:19, Julia Lawall wrote:
>> I guess the issue here could be a problem with parsing igt code (it is also an
>> issue with kernel drivers using iterators).
>> Cocci quite often does not recognize automatically iterators or other
>> constructs, and does not scan them at all.
>> Try run for example:
>> $ spatch --parse-c tests/intel/gem_ctx_exec.c
>> ...
>> NB total files = 1; perfect = 0; pbs = 1; timeout = 0; =========> 0%
>> nb good = 385,  nb passed = 19 =========> 3.12% passed
>> nb good = 385,  nb bad = 204 =========> 66.45% good or passed
>>
>> As I understand, lines marked as "bad" are skipped during cocci pass, am I
>> right?
>> Some workaround is to define iterators by hand in the script or include files.
> The following file:
>
> #define igt_main int main ()
> #define igt_simple_main int main ()
>
> #define igt_fixture
> #define igt_subtest_group
> #define igt_dynamic(x) YACFE_ITERATOR
> #define igt_dynamic_f(x) YACFE_ITERATOR
> #define igt_subtest(x) YACFE_ITERATOR
> #define igt_subtest_with_dynamic(x) YACFE_ITERATOR
> #define igt_subtest_with_dynamic_f(x) YACFE_ITERATOR
> #define igt_subtest_f(x) YACFE_ITERATOR
> #define igt_until_timeout(x) YACFE_ITERATOR
> #define igt_while_interruptible(x) YACFE_ITERATOR
> #define igt_list_for_each_entry_safe_reverse(x) YACFE_ITERATOR
> #define igt_list_for_each_entry_safe(x) YACFE_ITERATOR
> #define igt_list_for_each_entry(x) YACFE_ITERATOR
> #define _Atomic(x) x
>
> managed to parse over 98% of the lines in
>
> https://gitlab.freedesktop.org/drm/igt-gpu-tools
>
> Command line spatch --parse-c . --macro-file-builtins igt.h
>
> I'll check on the driver file you mentioned.  Thanks.

Wow, great.

I wonder if it would be possible to issue warning if spatch encounters 
lines which it is not able to parse, to avoid confusion.

Regards
Andrzej


>
> julia


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-14  7:57           ` Andrzej Hajda
@ 2024-08-14  8:02             ` Julia Lawall
  0 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2024-08-14  8:02 UTC (permalink / raw)
  To: Andrzej Hajda; +Cc: Peter Senna Tschudin, Peter Senna Tschudin, cocci

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]



On Wed, 14 Aug 2024, Andrzej Hajda wrote:

>
>
> On 14.08.2024 09:19, Julia Lawall wrote:
> > > I guess the issue here could be a problem with parsing igt code (it is
> > > also an
> > > issue with kernel drivers using iterators).
> > > Cocci quite often does not recognize automatically iterators or other
> > > constructs, and does not scan them at all.
> > > Try run for example:
> > > $ spatch --parse-c tests/intel/gem_ctx_exec.c
> > > ...
> > > NB total files = 1; perfect = 0; pbs = 1; timeout = 0; =========> 0%
> > > nb good = 385,  nb passed = 19 =========> 3.12% passed
> > > nb good = 385,  nb bad = 204 =========> 66.45% good or passed
> > >
> > > As I understand, lines marked as "bad" are skipped during cocci pass, am I
> > > right?
> > > Some workaround is to define iterators by hand in the script or include
> > > files.
> > The following file:
> >
> > #define igt_main int main ()
> > #define igt_simple_main int main ()
> >
> > #define igt_fixture
> > #define igt_subtest_group
> > #define igt_dynamic(x) YACFE_ITERATOR
> > #define igt_dynamic_f(x) YACFE_ITERATOR
> > #define igt_subtest(x) YACFE_ITERATOR
> > #define igt_subtest_with_dynamic(x) YACFE_ITERATOR
> > #define igt_subtest_with_dynamic_f(x) YACFE_ITERATOR
> > #define igt_subtest_f(x) YACFE_ITERATOR
> > #define igt_until_timeout(x) YACFE_ITERATOR
> > #define igt_while_interruptible(x) YACFE_ITERATOR
> > #define igt_list_for_each_entry_safe_reverse(x) YACFE_ITERATOR
> > #define igt_list_for_each_entry_safe(x) YACFE_ITERATOR
> > #define igt_list_for_each_entry(x) YACFE_ITERATOR
> > #define _Atomic(x) x
> >
> > managed to parse over 98% of the lines in
> >
> > https://gitlab.freedesktop.org/drm/igt-gpu-tools
> >
> > Command line spatch --parse-c . --macro-file-builtins igt.h
> >
> > I'll check on the driver file you mentioned.  Thanks.
>
> Wow, great.
>
> I wonder if it would be possible to issue warning if spatch encounters lines
> which it is not able to parse, to avoid confusion.

The theory is that generally you don't want that, because lost of macros
will trigger warnings, but are not relevant to the change that you want to
make.

But there is an option --verbose-parsing that should provide more
information.

julia

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [cocci] Iterator macros
  2024-08-13  6:15 [cocci] Iterator macros Peter Senna Tschudin
  2024-08-13  6:19 ` Julia Lawall
@ 2024-08-14  9:44 ` Julia Lawall
  1 sibling, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2024-08-14  9:44 UTC (permalink / raw)
  To: Peter Senna Tschudin; +Cc: cocci



On Tue, 13 Aug 2024, Peter Senna Tschudin wrote:

> Dear List,
>
> I am trying to make changes to a macro iterator from igt-gpu-tools*, and I am failing to use Coccinelle. My current attempt only captures about 1/2 of call sites, and I am not really sure how to ask Coccinelle to make the changes I want. Here is the pattern I am trying to change:
>
>
> From:
>
> const struct intel_execution_engine2 *e2;
> ...
> for_each_ctx_engine(i915, ctx, e2) {
>     execbuf.flags = e2->flags;
>     gem_execbuf(i915, &execbuf);
> }
>
>
> To:
>
> struct intel_engine_data ied;
> ...
> for_each_ctx_engine(i915, ctx, ied) {
>     execbuf.flags = ied.current_engine->flags;
>     gem_execbuf(i915, &execbuf);
> }
>
> Changes I want:
> 1. From `const struct intel_execution_engine2 *e2` to `struct intel_engine_data ied`.
> 2. Replace the last argument of for_each_ctx_engine() following the change on previous step.
> 3. Change references from `e2` to `ied.current_engine` inside the iterator statement.

I think that this gets at what you want.  It gives many warnings, but you
can ignore them:

@@
iterator name for_each_ctx_engine;
identifier e2;
symbol ied;
@@
- const struct intel_execution_engine2 *e2;
+ struct intel_engine_data ied;
  <+...
  for_each_ctx_engine(...,
-                      e2
+                      ied.current_engine
  ) { <...
-       e2
+       ied
      ...>
    }
  ...+>

julia

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-08-14  9:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-13  6:15 [cocci] Iterator macros Peter Senna Tschudin
2024-08-13  6:19 ` Julia Lawall
2024-08-13  6:27   ` Peter Senna Tschudin
2024-08-13  6:34     ` Julia Lawall
2024-08-13 11:05       ` Andrzej Hajda
2024-08-13 11:19         ` Julia Lawall
2024-08-14  7:19         ` Julia Lawall
2024-08-14  7:57           ` Andrzej Hajda
2024-08-14  8:02             ` Julia Lawall
2024-08-14  7:21         ` Julia Lawall
2024-08-14  6:22     ` Julia Lawall
2024-08-14  9:44 ` Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.