On 07/26/2011 06:20 PM, Andrew MacLeod wrote: > * __sync_mem_compare_exchange has the skeleton in place, but not the > guts. There are some issues that rth and I will work out later, I > just don't want to hold up the rest of the patch for that. Right now > it will fail the compare_exchange tests.
Please disable the relevant tests too. > if ((icode != CODE_FOR_nothing) && (model == MEMMODEL_SEQ_CST || > model == MEMMODEL_ACQ_REL)) > + #ifdef HAVE_sync_mem_thread_fence > + emit_mem_thread_fence (model); > + #else > expand_builtin_sync_synchronize (); > + #endif Coding style requires braces here. Yes, only one of the two functions are called, but that's not immediately obvious to the eye. Lots of other instances in your new code. That said, why wouldn't emit_mem_thread_fence always exist and generate the expand_builtin_sync_synchronize as needed? > + #ifdef HAVE_sync_mem_thread_fence > + emit_mem_thread_fence (model); > + #else > + if (model != MEMMODEL_RELAXED) > + expand_builtin_sync_synchronize (); > + #endif > + > + target = expand_sync_fetch_operation (mem, val, code, false, target); > + > + #ifdef HAVE_sync_mem_thread_fence > + emit_mem_thread_fence (model); > + #else > + if (model != MEMMODEL_RELAXED) > + expand_builtin_sync_synchronize (); > + #endif > + return target; Over-zealous with your pattern. The sync_fetch_op is a full barrier. You don't need the extra stuff. > + static rtx > + maybe_convert_modes (tree exp, enum machine_mode mode) I think a better name might be expand_expr_force_mode. Otherwise it looks ok. r~
