Re: [GCC, ARM] armv8 linux toolchain asan testcase fail due to stl missing conditional code

[Shiva Chen <shiva0217 at gmail dot com>]

Hi, Ramana

Currently, I work for Marvell and the company have copyright assignment on file.

Hi, all

After adding the attribute and rebuild gcc, I got the assembler error message

load_n.s:39: Error: bad instruction `ldrbeq r0,[r0]'

When i look into armv8 ISA document, it seems ldrb Encoding A1 have
conditional code field.

Does it mean we should also patch assembler or I just miss
understanding something ?

Following command use to generate load_n.s:

/home/shivac/build-system-trunk/Release/build/armv8-marvell-linux-gnueabihf-hard/gcc-final/./gcc/cc1
-fpreprocessed load_n.i -quiet -dumpbase load_n.c -march=armv8-a
-mfloat-abi=hard -mfpu=fp-armv8  -mtls-dialect=gnu -auxbase-strip
.libs/load_1_.o -g3 -O2 -Wall -Werror -version -fPIC -funwind-tables
-o load_n.s


The test.c is a simple test case to reproduce missing conditional code
in mmap.c.

Any suggestion ?


Shiva

2015-06-03 17:29 GMT+08:00 Shiva Chen <shiva0217@gmail.com>:
> Hi, Ramana
>
> I'm not sure what copyright assignment means ?
>
> Does it mean the patch have copyright assignment or not ?
>
> I update the patch to add "predicable" and  "predicable_short_it"
> attribute as suggestion.
>
> However, I don't have svn write access yet.
>
> Shiva
>
> 2015-06-03 16:36 GMT+08:00 Kyrill Tkachov <kyrylo.tkachov@arm.com>:
>>
>> On 03/06/15 09:32, Ramana Radhakrishnan wrote:
>>>>
>>>> This pattern is not predicable though, i.e. it doesn't have the
>>>> "predicable" attribute set to "yes".
>>>> Therefore the compiler should be trying to branch around here rather than
>>>> try to do a cond_exec.
>>>> Why does the generated code above look like it's converted to conditional
>>>> execution?
>>>> Could you produce a self-contained reduced testcase for this?
>>>
>>> CCFSM state machine in ARM state.
>>>
>>> arm.c (final_prescan_insn).
>>
>>
>> Ah ok.
>> This patch makes sense then.
>> As Ramana mentioned, please mark the pattern with "predicable" and also set
>> the "predicable_short_it" attribute to "no" so that it will not be
>> conditionalised in Thumb2 mode or when -mrestrict-it is enabled.
>>
>> Thanks,
>> Kyrill
>>
>>
>>
>>>
>>> Ramana
>>>
>>>> Thanks,
>>>> Kyrill
>>>>
>>>>> @@ -91,9 +91,9 @@
>>>>>       {
>>>>>         enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
>>>>>         if (is_mm_relaxed (model) || is_mm_consume (model) ||
>>>>> is_mm_acquire (model))
>>>>> -      return \"str<sync_sfx>\t%1, %0\";
>>>>> +      return \"str<sync_sfx>%?\t%1, %0\";
>>>>>         else
>>>>> -      return \"stl<sync_sfx>\t%1, %0\";
>>>>> +      return \"stl<sync_sfx>%?\t%1, %0\";
>>>>>       }
>>>>>     )
>>>>>
>>

Attachment: load_n.i
Description: Binary data

struct backtrace_state
{
  int threaded;
  int lock_alloc;
};

void foo (struct backtrace_state *state)
{
  if (state->threaded)
    __sync_lock_release (&state->lock_alloc);
}