diff mbox series

[27/40] sim/ppc: Add extra parenthesis to avoid ambiguity

Message ID 24f4d89aafc682360eb2b37005096f4b5b31124f.1666258361.git.research_trasio@irq.a4lg.com
State Committed
Headers
Series sim+gdb: Suppress warnings if built with Clang (big batch 1) |

Commit Message

Tsukasa OI Oct. 20, 2022, 9:32 a.m. UTC 
  Clang generates a warning if it considers that an expression is misleading
due to a lack of parenthesis ("-Wparentheses").
On the default configuration, it causes a build failure
(unless "--disable-werror" is specified).

This commit adds extra parenthesis to avoid ambiguity.
---
 sim/ppc/altivec.igen | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mike Frysinger Oct. 23, 2022, 3:05 p.m. UTC | #1 
On 20 Oct 2022 09:32, Tsukasa OI wrote:
> Clang generates a warning if it considers that an expression is misleading
> due to a lack of parenthesis ("-Wparentheses").
> On the default configuration, it causes a build failure
> (unless "--disable-werror" is specified).
> 
> This commit adds extra parenthesis to avoid ambiguity.
> ---
>  sim/ppc/altivec.igen | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sim/ppc/altivec.igen b/sim/ppc/altivec.igen
> index 63fe95a53d5..eda7af9dd6a 100644
> --- a/sim/ppc/altivec.igen
> +++ b/sim/ppc/altivec.igen
> @@ -231,7 +231,7 @@ void::model-function::ppc_insn_vr_vscr:itable_index index, model_data *model_ptr
>  	busy_ptr->vscr_busy = 1;
>  
>  	if (out_vmask)
> -	  busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2;
> +	  busy_ptr->nr_writebacks = (1 + (PPC_ONE_BIT_SET_P(out_vmask))) ? 1 : 2;

fairly certain this is incorrect.  this pretty much guarantees that "1" will
always be used and never "2".

Andrew posted a diff fix that i think is correct.
-mike
Tsukasa OI Oct. 24, 2022, 10:59 a.m. UTC | #2 
On 2022/10/24 0:05, Mike Frysinger wrote:
> On 20 Oct 2022 09:32, Tsukasa OI wrote:
>> Clang generates a warning if it considers that an expression is misleading
>> due to a lack of parenthesis ("-Wparentheses").
>> On the default configuration, it causes a build failure
>> (unless "--disable-werror" is specified).
>>
>> This commit adds extra parenthesis to avoid ambiguity.
>> ---
>>  sim/ppc/altivec.igen | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sim/ppc/altivec.igen b/sim/ppc/altivec.igen
>> index 63fe95a53d5..eda7af9dd6a 100644
>> --- a/sim/ppc/altivec.igen
>> +++ b/sim/ppc/altivec.igen
>> @@ -231,7 +231,7 @@ void::model-function::ppc_insn_vr_vscr:itable_index index, model_data *model_ptr
>>  	busy_ptr->vscr_busy = 1;
>>  
>>  	if (out_vmask)
>> -	  busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2;
>> +	  busy_ptr->nr_writebacks = (1 + (PPC_ONE_BIT_SET_P(out_vmask))) ? 1 : 2;
> 
> fairly certain this is incorrect.  this pretty much guarantees that "1" will
> always be used and never "2".
> 
> Andrew posted a diff fix that i think is correct.
> -mike

I agree that.  I kept the original semantics but this is definitely a
bug (I think Andrew's fix is correct).  I will leave it to Andrew (PATCH
05/10).

Thanks,
Tsukasa
diff mbox series

Patch

diff --git a/sim/ppc/altivec.igen b/sim/ppc/altivec.igen
index 63fe95a53d5..eda7af9dd6a 100644
--- a/sim/ppc/altivec.igen
+++ b/sim/ppc/altivec.igen
@@ -231,7 +231,7 @@  void::model-function::ppc_insn_vr_vscr:itable_index index, model_data *model_ptr
 	busy_ptr->vscr_busy = 1;
 
 	if (out_vmask)
-	  busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2;
+	  busy_ptr->nr_writebacks = (1 + (PPC_ONE_BIT_SET_P(out_vmask))) ? 1 : 2;
 
 	if (WITH_TRACE && ppc_trace[trace_model])
 	  model_trace_altivec_make_busy(model_ptr, vr_mask, 0);