Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 28 Nov 2017 11:30:23 +0000
From:   Mark Rutland <mark.rutland@arm.com>
To:     Li Zhijian <lizhijian@cn.fujitsu.com>
Cc:     acme@redhat.com, linux-kernel@vger.kernel.org, philip.li@intel.com,
        paulmck@linux.vnet.ibm.com, mingo@kernel.org
Subject: Re: [PATCH] tools include: define READ_ONCE/WRITE_ONCE for C++
Message-ID: <20171128113023.43bxgqfflawrlnih@lakrids.cambridge.arm.com>
References: <1511849622-3683-1-git-send-email-lizhijian@cn.fujitsu.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1511849622-3683-1-git-send-email-lizhijian@cn.fujitsu.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

[Fixed acme's address and added more Ccs]

On Tue, Nov 28, 2017 at 02:13:42PM +0800, Li Zhijian wrote:
> since 6aa7de0, we failed to build perf with C++(clang)
> to fix the following compile errors

Sorry about this.

With llvm-config and libclang from the LLVM 5.0.0 release, I can
reproduce this.

> --------------
> lizhijian@haswell-OptiPlex-9020:~/lkp/linux/tools/perf$ make LLVM_CONFIG=/usr/bin/llvm-config-3.9 LIBCLANGLLVM=1
> ...
>   CC       util/probe-finder.o
> In file included from /home/lizj/linux/tools/perf/util/util.h:13:0,
>                  from /home/lizj/linux/tools/perf/util/util-cxx.h:20,
>                  from util/c++/clang-c.h:5,
>                  from util/c++/clang-test.cpp:2:
> /home/lizj/linux/tools/include/asm/../../arch/x86/include/asm/atomic.h: In function ‘int atomic_read(const atomic_t*)’:
> /home/lizj/linux/tools/include/linux/compiler.h:157:45: error: use of deleted function ‘atomic_read(const atomic_t*)::<anonymous union>::<constructor>()’
>   ({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
>                                              ^
> /home/lizj/linux/tools/include/asm/../../arch/x86/include/asm/atomic.h:28:9: note: in expansion of macro ‘READ_ONCE’
>   return READ_ONCE((v)->counter);
>          ^
> /home/lizj/linux/tools/include/linux/compiler.h:157:11: note: ‘atomic_read(const atomic_t*)::<anonymous union>::<constructor>()’ is implicitly deleted because the default definition would be ill-formed:
>   ({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
>            ^
> /home/lizj/linux/tools/include/asm/../../arch/x86/include/asm/atomic.h:28:9: note: in expansion of macro ‘READ_ONCE’
>   return READ_ONCE((v)->counter);
>          ^
> /home/lizj/linux/tools/include/linux/compiler.h:157:11: error: uninitialized const member in ‘union atomic_read(const atomic_t*)::<anonymous>’
>   ({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
>            ^
> /home/lizj/linux/tools/include/asm/../../arch/x86/include/asm/atomic.h:28:9: note: in expansion of macro ‘READ_ONCE’
>   return READ_ONCE((v)->counter);
>          ^
> /home/lizj/linux/tools/include/linux/compiler.h:157:23: note: ‘const int atomic_read(const atomic_t*)::<anonymous union>::__val’ should be initialized
>   ({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
>                        ^
> /home/lizj/linux/tools/include/asm/../../arch/x86/include/asm/atomic.h:28:9: note: in expansion of macro ‘READ_ONCE’
>   return READ_ONCE((v)->counter);
>          ^
>   LD       tests/perf-in.o

So IIUC, g++ doesn't like the union aliasing trick that we use here, and
doesn't believe that __val has been initialised.

I haven't found a way around that yet. Ideally we'd just have a
non-const temporary (and a __builtin_memcpy) rather than a union, but I
can't find a way to strip const from typeof() prior to c++11.

> ...
> --------------
> 
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
>  tools/include/linux/compiler.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h
> index 07fd03c..e4c8a0d 100644
> --- a/tools/include/linux/compiler.h
> +++ b/tools/include/linux/compiler.h
> @@ -86,6 +86,10 @@
>  
>  #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>  
> +#ifdef __cplusplus
> +#define READ_ONCE(x) ACCESS_ONCE(x)
> +#define WRITE_ONCE(x, val) ACCESS_ONCE(x) = val
> +#else
>  #include <linux/types.h>

This is somewhat unfortunate, since we want to get rid of ACCESS_ONCE().

I'll see if I can come up with something to make the existing
{READ,WRITE}_ONCE() work, so that we can avoid issues with non-scalar
types.

Otherwise, could we please make these:

#define READ_ONCE(x) 	(*(volatile typeof(x) *)&(x))
#define WRITE_ONCE(x)	(*(volatile typeof(x) *)&(x)) = (val)

Thanks,
Mark.

>  /*
> @@ -160,6 +164,7 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
>  #define WRITE_ONCE(x, val) \
>  	({ union { typeof(x) __val; char __c[1]; } __u = { .__val = (val) }; __write_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })
>  
> +#endif /* __cplusplus */
>  
>  #ifndef __fallthrough
>  # define __fallthrough
> -- 
> 2.7.4
> 

From 1585289467202223313@xxx Tue Nov 28 06:17:28 +0000 2017
X-GM-THRID: 1585289320077919715
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread