Re: [PATCH v5 12/14] ARC: Build Infrastructure

2020-04-17 Thread Vineet Gupta 
On 4/17/20 5:12 PM, Vineet Gupta via Libc-alpha wrote:
> On 4/17/20 4:10 PM, Joseph Myers wrote:
>> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
>>
>>> +  GLIBC_PRIVATE {
>>> +# A copy of sigaction lives in libpthread, and needs these.
>>> +__default_rt_sa_restorer;
>>> +  }
>>
>> Not a requirement for this port, given that this is GLIBC_PRIVATE so can 
>> always be changed later, but does sigaction actually need to live in 
>> libpthread?
> 
> From my old branches, I added this back in 2018 when working on some 
> cancellation
> failures.
> 
>>  Or given the work that's been done on moving functions from 
>> libpthread to libc (and the corresponding dynamic linker work that mean 
>> it's now possible to move versioned symbols like that), could the copy of 
>> sigaction in libpthread be removed?
> 
> I don't think there's any ARC specific code which requires a restorer copy in
> libpthread. I can remove this and give the build a spin to see if anything 
> breaks.

As of today, libpthread_pic.a includes sigaction.os hence the need for export as
well: removing it from Version file causes build errors.

But when sigaction.os dependency is removed, this won't be needed.


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v3] Make any 32-bit time based syscalls unavailable for TIMESIZE==64

2020-04-17 Thread Vineet Gupta 
On 4/13/20 2:12 PM, Vineet Gupta via Libc-alpha wrote:
> On 4/6/20 11:54 AM, Vineet Gupta wrote:
>> On 3/31/20 2:47 PM, Vineet Gupta via Libc-alpha wrote:
>>> From: Vineet Gupta via Libc-alpha 
>>>
>>> An older asm-generic syscall ABI may have kernel provide 32-bit
>>> time syscalls, so undef them to not mix 32/64 in 64-bit time regime.
>>>
>>> Signed-off-by: Vineet Gupta 
>>
>> ping !
> 
> ping ^2 !

If this is not suitable for common code, I'd still like to add this as part of 
ARC
port to safe guard against future snafus.

>>> ---
>>> Changes since v2
>>>   - Made x32 safe
>>>
>>> Changes since v1
>>>   - don't redirect these to 64-bit variants
>>> ---
>>>  sysdeps/unix/sysv/linux/generic/sysdep.h | 24 
>>>  1 file changed, 24 insertions(+)
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/generic/sysdep.h 
>>> b/sysdeps/unix/sysv/linux/generic/sysdep.h
>>> index 40b4b955ca1b..b83e17e1c9d1 100644
>>> --- a/sysdeps/unix/sysv/linux/generic/sysdep.h
>>> +++ b/sysdeps/unix/sysv/linux/generic/sysdep.h
>>> @@ -17,6 +17,7 @@
>>> .  */
>>>  
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -25,3 +26,26 @@
>>>  #ifdef __NR_llseek
>>>  # define __NR__llseek __NR_llseek
>>>  #endif
>>> +
>>> +#if (__TIMESIZE == 64 && __WORDSIZE == 32 \
>>> + && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
>>> +
>>> +/* Don't provide 32-bit time syscalls even if the kernel ABI provides
>>> +   them (Older variants of asm-generic ABIs e.g. ARC).  */
>>> +
>>> +# undef __NR_futex
>>> +# undef __NR_rt_sigtimedwait
>>> +# undef __NR_ppoll
>>> +# undef __NR_utimensat
>>> +# undef __NR_pselect6
>>> +# undef __NR_recvmmsg
>>> +# undef __NR_semtimedop
>>> +# undef __NR_mq_timedreceive
>>> +# undef __NR_mq_timedsend
>>> +# undef __NR_clock_getres
>>> +# undef __NR_timerfd_settime
>>> +# undef __NR_timerfd_gettime
>>> +# undef __NR_sched_rr_get_interval
>>> +# undef __NR_clock_adjtime
>>> +
>>> +#endif
>>>
> 

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 12/14] ARC: Build Infrastructure

2020-04-17 Thread Vineet Gupta 
On 4/17/20 4:10 PM, Joseph Myers wrote:
> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
> 
>> +  GLIBC_PRIVATE {
>> +# A copy of sigaction lives in libpthread, and needs these.
>> +__default_rt_sa_restorer;
>> +  }
> 
> Not a requirement for this port, given that this is GLIBC_PRIVATE so can 
> always be changed later, but does sigaction actually need to live in 
> libpthread?

>From my old branches, I added this back in 2018 when working on some 
>cancellation
failures.

>  Or given the work that's been done on moving functions from 
> libpthread to libc (and the corresponding dynamic linker work that mean 
> it's now possible to move versioned symbols like that), could the copy of 
> sigaction in libpthread be removed?

I don't think there's any ARC specific code which requires a restorer copy in
libpthread. I can remove this and give the build a spin to see if anything 
breaks.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2] provide y2038 safe socket constants for default/asm-generic ABI

2020-04-17 Thread Vineet Gupta 
On 4/17/20 4:20 PM, Joseph Myers wrote:
> This patch is OK.

For commit now ?
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 14/14] Documentation for ARC port

2020-04-17 Thread Vineet Gupta 
On 4/17/20 4:13 PM, Joseph Myers wrote:
> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
> 
>> +* Support for Synpsys ARC HS cores (ARCv2 ISA) running Linux.
> 
> I think there's a missing 'o' there in Synopsys.

oops, fixed now.

> 
>> +  Port requires atleast
> 
> And a missing space in 'at least'.

Fixed.


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 13/14] build-many-glibcs.py: Enable ARC builds

2020-04-17 Thread Vineet Gupta 
On 4/17/20 4:12 PM, Joseph Myers wrote:
> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
> 
>> diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
>> index 64a836c52ea9..77b686d34cea 100755
>> --- a/scripts/build-many-glibcs.py
>> +++ b/scripts/build-many-glibcs.py
>> @@ -1248,6 +1248,7 @@ def install_linux_headers(policy, cmdlist):
>>  """Install Linux kernel headers."""
>>  arch_map = {'aarch64': 'arm64',
>>  'alpha': 'alpha',
>> +'arc': 'arc',
>>  'arm': 'arm',
>>  'csky': 'csky',
>>  'hppa': 'parisc',
> 
> The description of this patch seems a bit confused (the actual enabling of 
> the ARC builds is in a previous patch, this one is just adding some 
> configuration required to do so).  Anyway, this patch is OK with a better 
> description.

Sorry that was a snafu in moving code around, so some bits from this moved into
build patch. Fixed now.


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 11/14] ARC: Update syscall-names.list for ARC specific syscalls

2020-04-17 Thread Vineet Gupta 
On 4/17/20 4:05 PM, Joseph Myers wrote:
> This patch is OK.

So when the rest of port is ready, this will be part of main commit ?


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 08/14] ARC: Linux ABI

2020-04-17 Thread Vineet Gupta 
On 4/17/20 4:02 PM, Joseph Myers wrote:
> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
> 
>> +  for (i = 0; i < stack_args; i++)
>> +*r++ = va_arg(vl, unsigned long int);
> 
> Missing space before '(' in call to va_arg.
> 

Fixed.

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 06/14] ARC: hardware floating point support

2020-04-17 Thread Vineet Gupta 
On 4/17/20 3:59 PM, Joseph Myers wrote:
> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
> 
>> +#  define _FPU_SETS(cw) __asm__ volatile ("bset %0, %0, 31  \r\n" \
>> +  "sr   %0, [0x301] \r\n" \
>> +  : : "r" (cw))
> 
> This asm doesn't look safe; it's modifying an input operand.  I think the 
> compiler will assume the register it puts %0 in is unmodified by the asm, 
> because it's listed as an input.

Indeed this is a bug, as wiggling bit 31 was not present in inotial code.
I changed it to "+r"

> As an architecture-specific interface, it's not very clear if the 
> interface for _FPU_SETS should be that it modifies the variable passed as 
> an argument, but I'd guess not.

But does it not depend on whether the value is used after the fact. And with +r 
we
enable the compiler to DTRT.

>  My suggestion would be to define the 
> macro (using do { ... } while (0)) to copy the argument to a temporary 
> variable, and do the bit-set operation in C code on that temporary 
> variable rather than as part of the asm.

I can certainly do that if you think so.


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 03/14] ARC: Thread Local Storage support

2020-04-17 Thread Vineet Gupta 
On 4/17/20 3:44 PM, Joseph Myers wrote:
> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
> 
>> +/* Code to initially initialize the thread pointer.  */
>> +# define TLS_INIT_TP(tcbp)  \
>> +  ({\
>> +long result_var;\
>> +__builtin_set_thread_pointer(tcbp); \
> 
> Missing space before '(' in call to __builtin_set_thread_pointer.

Fixed.


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 02/14] ARC: startup and dynamic linking code

2020-04-17 Thread Vineet Gupta 
On 4/17/20 3:42 PM, Joseph Myers wrote:
> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
> 
>> +/* What this code does:
>> +-ldso starts execution here when kernel returns from execve()
>> +-calls into generic ldso entry point _dl_start( )
> 
> Avoid use of parentheses to indicate that a name is a function.

Fixed.

> 
>> +auto inline void
>> +__attribute__ ((always_inline))
>> +elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
>> +   void *const reloc_addr_arg)
>> +{
>> +  ElfW(Addr) *const reloc_addr = reloc_addr_arg;
>> +  *reloc_addr += l_addr; // + reloc->r_addend;
> 
> This comment seems unhelpful.  If you want to comment on the addend not 
> being added, write a comment that says *why* it's not being added, not 
> just a commented-out addition.

That's just leftover code from initial bringup, removed now.


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 01/14] ARC: ABI Implementation

2020-04-17 Thread Vineet Gupta 
On 4/17/20 3:37 PM, Joseph Myers wrote:
> On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:
> 
>> +#define reloc_index \
>> +({  \
>> +  unsigned long int plt0 = D_PTR (l, l_info[DT_PLTGOT]);\
>> +  unsigned long int pltn = reloc_arg;   \
>> +  /* Exclude PLT0 and PLT1.  */ \
>> +  unsigned long int idx = ((pltn - plt0) / 16 ) - 2;\
> 
> There's a stray space between '16' and ')' here.

Fixed.


>> +/* Helper for generic longjmp_chk().  */
>> +#define JB_FRAME_ADDRESS(buf) ((void *) (unsigned long int) (buf[JB_SP]))
> 
> This comment should not use '()' (see the GNU Coding Standards, '()' 
> should not be appended to a function name to indicate that it's a 
> function, only to indicate a function call with no arguments).

Fixed.


>> +  __mcount_internal ((unsigned long int) frompc,\
>> + (unsigned long int) __builtin_return_address(0));  \
> 
> Missing space before '(' in call to __builtin_return_address.

Fixed.


>> +#define TLS_LE(x)   \
>> +  ({ int *__result; \
>> + void *tp = __builtin_thread_pointer(); \
>> + __asm__ ("add %0, %1, @" #x "@tpoff   \n"  \
>> +  : "=r" (__result) : "r"(tp)); \
>> + __result; })
>> +
>> +#define TLS_IE(x)   \
>> +  ({ int *__result; \
>> + void *tp = __builtin_thread_pointer(); \
>> + __asm__ ("ld %0, [pcl, @" #x "@tlsie]  \n" \
>> +  "add %0, %1, %0   \n" \
>> +  : "=" (__result) : "r" (tp));   \
>> + __result; })
> 
> Missing spaces before '(' in calls to __builtin_thread_pointer.

Fixed.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2] provide y2038 safe socket constants for default/asm-generic ABI

2020-04-17 Thread Joseph Myers 
This patch is OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 14/14] Documentation for ARC port

2020-04-17 Thread Joseph Myers 
On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:

> +* Support for Synpsys ARC HS cores (ARCv2 ISA) running Linux.

I think there's a missing 'o' there in Synopsys.

> +  Port requires atleast

And a missing space in 'at least'.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 13/14] build-many-glibcs.py: Enable ARC builds

2020-04-17 Thread Joseph Myers 
On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:

> diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
> index 64a836c52ea9..77b686d34cea 100755
> --- a/scripts/build-many-glibcs.py
> +++ b/scripts/build-many-glibcs.py
> @@ -1248,6 +1248,7 @@ def install_linux_headers(policy, cmdlist):
>  """Install Linux kernel headers."""
>  arch_map = {'aarch64': 'arm64',
>  'alpha': 'alpha',
> +'arc': 'arc',
>  'arm': 'arm',
>  'csky': 'csky',
>  'hppa': 'parisc',

The description of this patch seems a bit confused (the actual enabling of 
the ARC builds is in a previous patch, this one is just adding some 
configuration required to do so).  Anyway, this patch is OK with a better 
description.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 12/14] ARC: Build Infrastructure

2020-04-17 Thread Joseph Myers 
On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:

> +  GLIBC_PRIVATE {
> +# A copy of sigaction lives in libpthread, and needs these.
> +__default_rt_sa_restorer;
> +  }

Not a requirement for this port, given that this is GLIBC_PRIVATE so can 
always be changed later, but does sigaction actually need to live in 
libpthread?  Or given the work that's been done on moving functions from 
libpthread to libc (and the corresponding dynamic linker work that mean 
it's now possible to move versioned symbols like that), could the copy of 
sigaction in libpthread be removed?

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 11/14] ARC: Update syscall-names.list for ARC specific syscalls

2020-04-17 Thread Joseph Myers 
This patch is OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 08/14] ARC: Linux ABI

2020-04-17 Thread Joseph Myers 
On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:

> +  for (i = 0; i < stack_args; i++)
> +*r++ = va_arg(vl, unsigned long int);

Missing space before '(' in call to va_arg.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 06/14] ARC: hardware floating point support

2020-04-17 Thread Joseph Myers 
On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:

> +#  define _FPU_SETS(cw) __asm__ volatile ("bset %0, %0, 31   \r\n" \
> +   "sr   %0, [0x301] \r\n" \
> +  : : "r" (cw))

This asm doesn't look safe; it's modifying an input operand.  I think the 
compiler will assume the register it puts %0 in is unmodified by the asm, 
because it's listed as an input.

As an architecture-specific interface, it's not very clear if the 
interface for _FPU_SETS should be that it modifies the variable passed as 
an argument, but I'd guess not.  My suggestion would be to define the 
macro (using do { ... } while (0)) to copy the argument to a temporary 
variable, and do the bit-set operation in C code on that temporary 
variable rather than as part of the asm.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 03/14] ARC: Thread Local Storage support

2020-04-17 Thread Joseph Myers 
On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:

> +/* Code to initially initialize the thread pointer.  */
> +# define TLS_INIT_TP(tcbp)   \
> +  ({ \
> + long result_var;\
> + __builtin_set_thread_pointer(tcbp); \

Missing space before '(' in call to __builtin_set_thread_pointer.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 02/14] ARC: startup and dynamic linking code

2020-04-17 Thread Joseph Myers 
On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:

> +/* What this code does:
> +-ldso starts execution here when kernel returns from execve()
> +-calls into generic ldso entry point _dl_start( )

Avoid use of parentheses to indicate that a name is a function.

> +auto inline void
> +__attribute__ ((always_inline))
> +elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
> +void *const reloc_addr_arg)
> +{
> +  ElfW(Addr) *const reloc_addr = reloc_addr_arg;
> +  *reloc_addr += l_addr; // + reloc->r_addend;

This comment seems unhelpful.  If you want to comment on the addend not 
being added, write a comment that says *why* it's not being added, not 
just a commented-out addition.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v5 01/14] ARC: ABI Implementation

2020-04-17 Thread Joseph Myers 
On Wed, 8 Apr 2020, Vineet Gupta via Libc-alpha wrote:

> +#define reloc_index  \
> +({   \
> +  unsigned long int plt0 = D_PTR (l, l_info[DT_PLTGOT]); \
> +  unsigned long int pltn = reloc_arg;\
> +  /* Exclude PLT0 and PLT1.  */  \
> +  unsigned long int idx = ((pltn - plt0) / 16 ) - 2; \

There's a stray space between '16' and ')' here.

> +/* Helper for generic longjmp_chk().  */
> +#define JB_FRAME_ADDRESS(buf) ((void *) (unsigned long int) (buf[JB_SP]))

This comment should not use '()' (see the GNU Coding Standards, '()' 
should not be appended to a function name to indicate that it's a 
function, only to indicate a function call with no arguments).

> +  __mcount_internal ((unsigned long int) frompc, \
> +  (unsigned long int) __builtin_return_address(0));  \

Missing space before '(' in call to __builtin_return_address.

> +#define TLS_LE(x)\
> +  ({ int *__result;  \
> + void *tp = __builtin_thread_pointer();  \
> + __asm__ ("add %0, %1, @" #x "@tpoff   \n"   \
> +   : "=r" (__result) : "r"(tp)); \
> + __result; })
> +
> +#define TLS_IE(x)\
> +  ({ int *__result;  \
> + void *tp = __builtin_thread_pointer();  \
> + __asm__ ("ld %0, [pcl, @" #x "@tlsie]  \n" \
> +   "add %0, %1, %0   \n" \
> +   : "=" (__result) : "r" (tp));   \
> + __result; })

Missing spaces before '(' in calls to __builtin_thread_pointer.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc