[Patch] MIPS: Add atomic memory operations for QI and HI modes.

[David Daney]

Richard Sandiford wrote:
> Hi David,
> 
> David Daney <ddaney@avtrex.com> writes:
>> David Daney wrote:
>>> This patch adds support for QI and HI mode atomic memory operations to
>>> MIPS.  This is a minimal implementation of the QI and HI mode operations
>>> as it only implements sync_compare_and_swap for those modes.  A possible
>>> future enhancement would be to implement the full set of atomic insns
>>> for QI and HI modes, but this should be enough to get libstdc++ building
>>> again.
>>>
>>> The patch borrows from the sparc implementation.
>>>
>>> Currently testing on mipsel-linux with all languages,
>>>
>>> OK to commit if it tests good?
>> And it did:
>>
>> http://gcc.gnu.org/ml/gcc-testresults/2008-04/msg01887.html
>>
>> This gets us back to clean C++ and libstdc++ test results (except for 
>> pr31863.C, which times out occasionally when the system is heavily loaded).
> 
.
.
.
> 
> Finally, I thought it would be worth having a version of mips_emit_binary
> that creates a new register.  That should make it easier to see where a
> value is first initialised.

Thanks, that was bothering me too (but not enough to fix it).  Is there 
any reason not to use a new register each time we do an assignment?

See my questions below..

> 
> Here's a version of the patch with those nits picked.  I also added
> a few more comments.  (I'm sure some folk will think it's now very
> much over-commented...)
> 
> I won't have time to test this for a few days, as my machine is busy
> with other things.  But I tested that it produces the same code as
> your patch for the gcc.torture/compile/sync-1.c test for o32 and n32,
> and it makes that test work for n64.
> 

I will do a full bootstrap on mipsel-linux (should be done tomorrow).  I 
could test a multi-lib bootstrap on mips64-linux but on my O2 that takes 
about a week.



> +/* Expand a QI or HI mode compare_and_swap.  The operands are the same
> +   as for the generator function.  */
> +
> +void
> +mips_expand_compare_and_swap_12 (rtx result, rtx mem, rtx oldval, rtx newval)
> +{
> +  rtx orig_addr, memsi_addr, memsi, shift, shiftsi, unshifted_mask;
> +  rtx mask, inverted_mask, oldvalsi, old_shifted, newvalsi, new_shifted, res;
> +
> +  /* Compute the address of the containing SImode value.  */
> +  orig_addr = force_reg (Pmode, XEXP (mem, 0));
> +  memsi_addr = mips_force_binary (Pmode, AND, orig_addr,
> +				  force_reg (Pmode, GEN_INT (-4)));
> +
> +  /* Create a memory reference for it.  */
> +  memsi = gen_rtx_MEM (SImode, memsi_addr);
> +  set_mem_alias_set (memsi, ALIAS_SET_MEMORY_BARRIER);
> +  MEM_VOLATILE_P (memsi) = MEM_VOLATILE_P (mem);
> +
> +  /* Work out the byte offset of the QImode or HImode value,
> +     counting from the least significant byte.  */
> +  shift = mips_force_binary (Pmode, AND, orig_addr, GEN_INT (3));
> +  if (TARGET_BIG_ENDIAN)
> +    mips_emit_binary (XOR, shift, shift,
> +		      GEN_INT (GET_MODE (mem) == QImode ? 3 : 2));

Why not mips_force_binary here^^^?

> +
> +  /* Multiply by eight to convert the shift value from bytes to bits.  */
> +  mips_emit_binary (ASHIFT, shift, shift, GEN_INT (3));
> +

And here ^^^?

> +  /* Make the final shift an SImode value, so that it can be used in
> +     SImode operations.  */
> +  shiftsi = force_reg (SImode, gen_lowpart (SImode, shift));
> +
> +  /* Set MASK to an inclusive mask of the QImode or HImode value.  */
> +  unshifted_mask = GEN_INT (GET_MODE_MASK (GET_MODE (mem)));
> +  unshifted_mask = force_reg (SImode, unshifted_mask);
> +  mask = mips_force_binary (SImode, ASHIFT, unshifted_mask, shiftsi);
> +
> +  /* Compute the equivalent exclusive mask.  */
> +  inverted_mask = gen_reg_rtx (SImode);
> +  emit_insn (gen_rtx_SET (VOIDmode, inverted_mask,
> +			  gen_rtx_NOT (SImode, mask)));
> +
> +  /* Shift the old value into place.  */
> +  oldvalsi = force_reg (SImode, gen_lowpart (SImode, oldval));
> +  old_shifted = mips_force_binary (SImode, ASHIFT, oldvalsi, shiftsi);
> +
> +  /* Do the same for the new value.  */
> +  newvalsi = force_reg (SImode, gen_lowpart (SImode, newval));
> +  new_shifted = mips_force_binary (SImode, ASHIFT, newvalsi, shiftsi);
> +
> +  /* Do the SImode atomic access.  */
> +  res = gen_reg_rtx (SImode);
> +  emit_insn (gen_compare_and_swap_12 (res, memsi, mask, inverted_mask,
> +				      old_shifted, new_shifted));
> +
> +  /* Shift and convert the result.  */
> +  mips_emit_binary (AND, res, res, mask);

And here ^^^?

> +  mips_emit_binary (LSHIFTRT, res, res, shiftsi);

And here^^^?

> +  mips_emit_move (result, gen_lowpart (GET_MODE (result), res));
> +}
> +

I may bootstrap it changing those to mips_force_binary.

David Daney