[cfe-dev] Two questions about address_space attribute

[Liu, Yaxun (Sam) via cfe-dev]

+ John Matt

My comments are below.

Sam

-----Original Message-----
From: Y Song <ys114321 at gmail.com> 
Sent: Tuesday, November 12, 2019 10:15 AM
To: cfe-dev at lists.llvm.org
Cc: Andrii Nakryiko <andrii.nakryiko at gmail.com>; Alexei Starovoitov <alexei.starovoitov at gmail.com>; richard at metafoo.co.uk; Liu, Yaxun (Sam) <Yaxun.Liu at amd.com>
Subject: Re: Two questions about address_space attribute

[CAUTION: External Email]

Hi, Richard Smith and Yaxun Liu,

You probably have expertise on the two questions below related to address space. Could you comment on my questions below?

Thanks!

Yonghong

On Thu, Nov 7, 2019 at 10:29 AM Y Song <ys114321 at gmail.com> wrote:
>
> Hi,
>
> Currently, I am thinking to add address_space attribute support for
> x86_64 and BPF to compile linux kernel. The main goal is to get 
> address_space information into debug_info and then later to dwarf/BTF.
> But address_space enforcement itself can also help with better code 
> for the kernel.
>
> The RFC patch is here:
>     https://reviews.llvm.org/D69393
>
> During experiments, we found two issues which I would like to get 
> expert opinion:
>
> issue 1: address_space attributes on function pointers 
> =========================================
>
> clang does not allow address_space attribute for function pointers, 
> specifically, in clang/lib/Sema/SemaType.cpp,
>
> // ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "A function type shall 
> not be // qualified by an address-space qualifier."
> if (Type->isFunctionType()) {
>   S.Diag(Attr.getLoc(), diag::err_attribute_address_function_type);
>   Attr.setInvalid();
>   return;
> }
>
> But linux kernel tries to annotate signal handling function pointer 
> with address space to indicate it is accessing user space.
>
> typedef __signalfn_t __user *__sighandler_t; typedef __restorefn_t 
> __user *__sigrestore_t;
>
> Such attribute makes sense for linux since indeed the signal handler 
> code resides in user space and the kernel pointer merely points to 
> user memory here.
>
> What is the rationale for this restriction in standard? How we could 
> make clang tolerate such restrictions for X86/BPF/...?
>

I think we may allow function to be in a non-default address space. The issue is that there may be lots of places in clang or llvm assuming function in default address space, which needs to be fixed.

> issue 2: typeof(*ptr) where ptr has address_space attribute.
>
> -bash-4.4$ cat t2.c
> int test(int __attribute__((address_space(1))) *p) { #ifdef NO_TYPEOF
>   int t = *p;
> #else
>   typeof(*p) t = *p;
> #endif
>   return t;
> }
> -bash-4.4$ clang -c t2.c
> t2.c:5:14: error: automatic variable qualified with an address space
>   typeof(*p) t = *p;
>              ^
> 1 error generated.

Llvm assumes stack address space (alloca address space) is determined by data layout. By default it is 0 but it can be changed by specify a different value in data layout. E.g. amdgcn has alloca address space to be 5. It cannot be explicitly specified in source.

> -bash-4.4$ clang -DNO_TYPEOF -c t2.c
> -bash-4.4$
>
> The IR generated without NO_TYPEOF macro:
>   %p.addr = alloca i32 addrspace(1)*, align 8
>   %t = alloca i32, align 4
>   store i32 addrspace(1)* %p, i32 addrspace(1)** %p.addr, align 8
>   %0 = load i32 addrspace(1)*, i32 addrspace(1)** %p.addr, align 8
>   %1 = load i32, i32 addrspace(1)* %0, align 4
>   store i32 %1, i32* %t, align 4
>   %2 = load i32, i32* %t, align 4
>   ret i32 %2
>
> So we can directly load a i32 addrspacee(1)* to an i32. Maybe
> typeof(*p) should just i32 and we should not emit the error at all?
>
> Thanks,
>
> Yonghong