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

[Shiva Chen]

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.tkac...@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\";
>>>>       }
>>>>     )
>>>>
>

diff --git a/gcc/config/arm/sync.md b/gcc/config/arm/sync.md
index 44cda61..cf8f3a3 100644
--- a/gcc/config/arm/sync.md
+++ b/gcc/config/arm/sync.md
@@ -75,11 +75,12 @@
   {
     enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
     if (is_mm_relaxed (model) || is_mm_consume (model) || is_mm_release 
(model))
-      return \"ldr<sync_sfx>\\t%0, %1\";
+      return \"ldr<sync_sfx>%?\\t%0, %1\";
     else
-      return \"lda<sync_sfx>\\t%0, %1\";
+      return \"lda<sync_sfx>%?\\t%0, %1\";
   }
-)
+  [(set_attr "predicable" "yes")
+   (set_attr "predicable_short_it" "no")])
 
 (define_insn "atomic_store<mode>"
   [(set (match_operand:QHSI 0 "memory_operand" "=Q")
@@ -91,11 +92,12 @@
   {
     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\";
   }
-)
+  [(set_attr "predicable" "yes")
+   (set_attr "predicable_short_it" "no")])
 
 ;; Note that ldrd and vldr are *not* guaranteed to be single-copy atomic,
 ;; even for a 64-bit aligned address.  Instead we use a ldrexd unparied