On 15 August 2017 at 16:56, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> Hi Richard,
>
> On 08/15/2017 11:57 AM, Richard Henderson wrote:
>>
>> From: Alistair Francis <alistair.fran...@xilinx.com>
>>
>> Acording to the ARM ARM exclusive loads require the same allignment as
>> exclusive stores. Let's update the memops used for the load to match
>> that of the store. This adds the alignment requirement to the memops.
>>
>> Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
>> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com>
>> [rth: Require 16-byte alignment for 64-bit LDXP.]
>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>> ---
>> target/arm/translate-a64.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
>> index eac545e4f2..2200e25be0 100644
>> --- a/target/arm/translate-a64.c
>> +++ b/target/arm/translate-a64.c
>> @@ -1861,7 +1861,7 @@ static void gen_load_exclusive(DisasContext *s, int
>> rt, int rt2,
>> g_assert(size >= 2);
>> if (size == 2) {
>> /* The pair must be single-copy atomic for the doubleword.
>> */
>> - memop |= MO_64;
>> + memop |= MO_64 | MO_ALIGN;
>
>
> isn't MO_ALIGN_8 enough?
MO_ALIGN is "align to size of access", ie to 8 bytes in this case.
MO_ALIGN_8 is "align to a specified size (8 bytes) which isn't the
size of the access".
In this case the access size is 8 bytes so we don't need MO_ALIGN_8.
Alignments to something other than the access size are the oddball
case, so I think it makes sense to stick with MO_ALIGN for the
common case of "just aligned to the access size" so you can
spot the odd cases when you're reading the source.
thanks
-- PMM
