Re: asm volatile statement reordering

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17/10/17 10:12, Segher Boessenkool wrote:
> Hi!
> 
> On Tue, Oct 17, 2017 at 12:27:08AM +0200, David Brown wrote:
>> There has been a discussion going on about asm volatile statement 
>> reordering, where there has been surprising code generated for the ARM 
>> with certain code and compiler flags.  The discussion has been in the 
>> comp.arch.embedded Usenet group, the gcc-arm-embedded project (the most 
>> common source of gcc for embedded ARM devices, both for individuals and 
>> for companies), and some other related projects.  I believe it is time 
>> to ask the gcc folks too!
>>
>> This is a link to the gcc-arm-embedded issue, which is perhaps the most 
>> complete version.
>>
>> <https://bugs.launchpad.net/gcc-arm-embedded/+bug/1722849?comments=all>
> 
> I cannot reproduce this problem (with trunk GCC).  What is different
> about the compiler used there / what else is different?
> 

I don't have a build of trunk gcc conveniently available (I am building
a gcc 7 at the moment).  If this is a bug, then perhaps it is fixed
since the latest version I tried (6.3.0).  It is quite easy to test
there using the marvellous godbolt online compiler:

<https://godbolt.org/g/cJJNar>

Changing the line

	__asm volatile ("cpsid i" :: );

to

	__asm volatile ("cpsid i" :: "" (status));

gives the desired generated code.

> ...
> 
> I now also tried with some GCC 5, and the problem does happen there.
> 
> Please open a PR!
> 
> This is similar to PR62642, which is about unspec_volatile -- essentially
> everything that applies to unspec_volatile also applies to volatile asm.
> 
> I did the following patch, which fixes the problem in the testcase.
> Please try it with the "real" code?  (And do open a PR please).
> 
> 
>> Much of this boils down to the question of when gcc is allowed to 
>> re-order "asm volatile" statements, with respect to other "asm volatile" 
>> statements, volatile memory accesses, and unknown functions (which may 
>> contain observable behaviour).
> 
> "asm volatile" means the asm has an (unknown) side effect when executed.
> It cannot be moved over any other side effects.
> 
>> My testing suggests that gcc will re-order "asm volatile" statements 
>> that have an output, such as the "save the PRIMASK into status" 
>> statement, but it will /not/ re-order "asm volatile" statements that 
>> have no outputs.
>>
>> Is that correct?
> 
> Nope, you found a bug :-)
> 

The gcc documentation specifically says that asm volatile statements
/can/ be moved.  It just does not make it clear what kind of movements
are allowed.  If the intention is to allow movement with respect to
ordinary code while restricting movement with respect to volatile code
(volatile memory accesses, and volatile asm), then it is a bug.  If the
intention is to allow any kind of movement (after considering
dependencies), then it is not a bug - just a very unexpected design
decision.


Is the patch below something you added to the trunk to fix this, or is
it something the trunk already had?  If it is already in the trunk then
there is no need for a PR for the trunk, but I will suggest it for
backporting to the gnu-arm-embedded builds (which are the main source of
embedded ARM compilers for small microcontroller development).

Thanks,

David




> 
> Segher
> 
> 
> Index: gcc/ira.c
> ===================================================================
> --- gcc/ira.c   (revision 251105)
> +++ gcc/ira.c   (working copy)
> @@ -4418,6 +4418,10 @@
>  	 for a reason.  */
>        return false;
>  
> +    case ASM_OPERANDS:
> +      if (MEM_VOLATILE_P (x))
> +	return false;
> +
>      default:
>        break;
>      }
> 




[Index of Archives]     [Linux C Programming]     [Linux Kernel]     [eCos]     [Fedora Development]     [Fedora Announce]     [Autoconf]     [The DWARVES Debugging Tools]     [Yosemite Campsites]     [Yosemite News]     [Linux GCC]

  Powered by Linux