On 2/28/19 5:26 AM, JiangNing OS wrote:
> To solve BZ89430 the followings are needed,
>
> (1) The code below in noce_try_cmove_arith needs to be fixed.
>
> /* ??? We could handle this if we knew that a load from A or B could
> not trap or fault. This is also true if we've already loaded
> from the address along the path from ENTRY. */
> else if (may_trap_or_fault_p (a) || may_trap_or_fault_p (b))
> return FALSE;
>
> Finding dominating memory access could help to decide whether the memory
> access following the conditional move is valid or not.
> * If there is a dominating memory write to the same memory address in test_bb
> as the one from x=a, it is always safe.
> * When the dominating memory access to the same memory address in test_bb is
> a read, for the load out of x=a, it is always safe. For the store out of x=a,
> if the memory is on stack, it is still safe.
>
> Besides, there is a bug around rearranging the then_bb and else_bb layout,
> which needs to be fixed.
>
> (2) The fix by https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02998.html is
> an overkill. If the write target following conditional move is a memory
> access, it exits shortly.
>
> if (!set_b && MEM_P (orig_x))
> /* We want to avoid store speculation to avoid cases like
> if (pthread_mutex_trylock(mutex))
> ++global_variable;
> Rather than go to much effort here, we rely on the SSA optimizers,
> which do a good enough job these days. */
> return FALSE;
>
> It looks a bit hard for compiler to know the program semantic is related to
> mutex and avoid store speculation. Without removing this overkill, the fix
> mentioned by (1) would not work. Any idea? An alternative solution is to
> detect the following pattern conservatively,
>
> if (a_function_call(...))
> ++local_variable;
>
> If the local_variable doesn't have address taken at all in current function,
> it mustn't be a pthread mutex lock semantic, and then the following patch
> would work to solve (1) and pass the last case as described in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89430.
Just a note. I'm deferring this to gcc-10.
jeff
