[Mesa-dev] glsl: Small optimization for constant conditionals

Submitted by Iago Toral Quiroga on April 15, 2014, 10:30 a.m.

Details

Message ID 1397557840-17838-2-git-send-email-itoral@igalia.com
State Accepted
Commit cda5e0c25ea5f8ab38b69f1f04099acfa3f0ced2
Headers show

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga April 15, 2014, 10:30 a.m.
Once the relevant branch has been identified do not iterate over the
instructions in the branch, do a linked list insertion instead to avoid the
loop.
---
 src/glsl/opt_if_simplification.cpp | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/glsl/opt_if_simplification.cpp b/src/glsl/opt_if_simplification.cpp
index 2bec825..e05f031 100644
--- a/src/glsl/opt_if_simplification.cpp
+++ b/src/glsl/opt_if_simplification.cpp
@@ -90,15 +90,9 @@  ir_if_simplification_visitor::visit_leave(ir_if *ir)
        * that matters out.
        */
       if (condition_constant->value.b[0]) {
-	 foreach_list_safe(n, &ir->then_instructions) {
-	    ir_instruction *then_ir = (ir_instruction *) n;
-	    ir->insert_before(then_ir);
-	 }
+         ir->insert_before(&ir->then_instructions);
       } else {
-	 foreach_list_safe(n, &ir->else_instructions) {
-	    ir_instruction *else_ir = (ir_instruction *) n;
-	    ir->insert_before(else_ir);
-	 }
+         ir->insert_before(&ir->else_instructions);
       }
       ir->remove();
       this->made_progress = true;

Comments

On 04/15/2014 03:30 AM, Iago Toral Quiroga wrote:
> Once the relevant branch has been identified do not iterate over the
> instructions in the branch, do a linked list insertion instead to avoid the
> loop.
> ---
>  src/glsl/opt_if_simplification.cpp | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)

Oh, this is nicer.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

and pushed.

By the way, usually people send cover letters for patches when there's a
large series, but not for a single patch.  If you have extra commentary
about a patch, you can put that here, below the "---" and the diffstat.
 (Anything below the "---" won't be part of the commit message when
applied with "git am".)

> diff --git a/src/glsl/opt_if_simplification.cpp b/src/glsl/opt_if_simplification.cpp
> index 2bec825..e05f031 100644
> --- a/src/glsl/opt_if_simplification.cpp
> +++ b/src/glsl/opt_if_simplification.cpp
> @@ -90,15 +90,9 @@ ir_if_simplification_visitor::visit_leave(ir_if *ir)
>         * that matters out.
>         */
>        if (condition_constant->value.b[0]) {
> -	 foreach_list_safe(n, &ir->then_instructions) {
> -	    ir_instruction *then_ir = (ir_instruction *) n;
> -	    ir->insert_before(then_ir);
> -	 }
> +         ir->insert_before(&ir->then_instructions);
>        } else {
> -	 foreach_list_safe(n, &ir->else_instructions) {
> -	    ir_instruction *else_ir = (ir_instruction *) n;
> -	    ir->insert_before(else_ir);
> -	 }
> +         ir->insert_before(&ir->else_instructions);
>        }
>        ir->remove();
>        this->made_progress = true;
>
Hi Kenneth,

El 2014-04-17 08:57, Kenneth Graunke escribió:
> On 04/15/2014 03:30 AM, Iago Toral Quiroga wrote:
>> Once the relevant branch has been identified do not iterate over the
>> instructions in the branch, do a linked list insertion instead to 
>> avoid the
>> loop.
>> ---
>>  src/glsl/opt_if_simplification.cpp | 10 ++--------
>>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> Oh, this is nicer.
> 
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
> 
> and pushed.
> 
> By the way, usually people send cover letters for patches when there's 
> a
> large series, but not for a single patch.  If you have extra commentary
> about a patch, you can put that here, below the "---" and the diffstat.
>  (Anything below the "---" won't be part of the commit message when
> applied with "git am".)

Thanks for the review and for the tip! I will do this next time.

>> diff --git a/src/glsl/opt_if_simplification.cpp 
>> b/src/glsl/opt_if_simplification.cpp
>> index 2bec825..e05f031 100644
>> --- a/src/glsl/opt_if_simplification.cpp
>> +++ b/src/glsl/opt_if_simplification.cpp
>> @@ -90,15 +90,9 @@ ir_if_simplification_visitor::visit_leave(ir_if 
>> *ir)
>>         * that matters out.
>>         */
>>        if (condition_constant->value.b[0]) {
>> -	 foreach_list_safe(n, &ir->then_instructions) {
>> -	    ir_instruction *then_ir = (ir_instruction *) n;
>> -	    ir->insert_before(then_ir);
>> -	 }
>> +         ir->insert_before(&ir->then_instructions);
>>        } else {
>> -	 foreach_list_safe(n, &ir->else_instructions) {
>> -	    ir_instruction *else_ir = (ir_instruction *) n;
>> -	    ir->insert_before(else_ir);
>> -	 }
>> +         ir->insert_before(&ir->else_instructions);
>>        }
>>        ir->remove();
>>        this->made_progress = true;
>>