Re: [Valgrind-developers] vex: r2385: Fix a few issues as reported by the BEAM tool.

[Florian K. <br...@ac...>]

On 06/15/2012 11:43 PM, John Reiser wrote:
>>  
>> -      if ((e->Iex.Binop.op == Iop_CmpF64) |
>> -          (e->Iex.Binop.op == Iop_CmpD64) |
>> +      if ((e->Iex.Binop.op == Iop_CmpF64) ||
>> +          (e->Iex.Binop.op == Iop_CmpD64) ||
>>            (e->Iex.Binop.op == Iop_CmpD128)) {
> 
> This change produces equivalent-but-slower code on i686, x86_64,
> PowerPC, ARM, and any other architecture that can perform multiple
> tests without multiple branches (and thus avoid multiple
> pipeline stalls due to multiple missed branch predictions.)
> [x86: SETcc + bool; PowerPC: crAND, crOR etc; ARM: conditional
> execution (including if-then)]

In this case the compiler could replace || with | as an optimization as
all subexpressions are free of side effects.

> If BEAM insists, then BEAM needs to re-consider.

BEAM is not insisting on anything. We could easily switch that complaint
off if we wanted to. But I don't think we want to do that.
Mixing boolean and bitwise expressions is a known source of trouble. If
you look around VEX code, || is used across the board when boolean
expressions are combined. So we should be consistent with existing practice.

    Florian