Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

[Stefan Monnier]

> --- a/src/lisp.h	2014-09-07 07:04:01 +0000
> +++ b/src/lisp.h	2014-09-09 03:44:06 +0000
> @@ -298,6 +298,13 @@
>  # endif
>  #endif
> 
> +/* Stolen from gnulib.  */
> +#if (__GNUC__ || __HP_cc || __HP_aCC || __IBMC__	\
> +     || __IBMCPP__ || __ICC || 0x5110 <= __SUNPRO_C)
> +#define GCALIGNED __attribute__ ((aligned (GCALIGNMENT)))
> +#else
> +#define GCALIGNED /* empty */
> +#endif

Any reason you don't use the `alignas' we already use/check just above?

> +#if (__GNUC__ || __HP_cc || __HP_aCC || __IBMC__	\
> +     || __IBMCPP__ || __ICC || 0x5110 <= __SUNPRO_C)

Yuck!  Why not check USE_LSB_TAG?
- USE_LSB_TAG is only set when we have alignas defined.
- When USE_LSB_TAG is not set, you don't need special alignment.


        Stefan

Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

[Dmitry Antipov]

On 09/09/2014 05:13 PM, Stefan Monnier wrote:

> Any reason you don't use the `alignas' we already use/check just above?

An existing code uses alignas (X) for variable declaration,
not for type declaration.  If I try the latter, I see:

=== modified file 'src/lisp.h'
--- src/lisp.h	2014-09-09 11:43:22 +0000
+++ src/lisp.h	2014-09-09 14:09:45 +0000
@@ -1023,7 +1023,7 @@

  typedef struct interval *INTERVAL;

-struct GCALIGNED Lisp_Cons
+struct alignas (GCALIGNMENT) Lisp_Cons
    {
      /* Car of this cons cell.  */
      Lisp_Object car;

==>

../../trunk/src/lisp.h:1026:8: error: expected ‘{’ before ‘_Alignas’
  struct alignas (GCALIGNMENT) Lisp_Cons
         ^

No ideas why alignas (X) is designed in such a way.  Paul?

Dmitry

Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

[Paul Eggert]

Dmitry Antipov wrote:
> No ideas why alignas (X) is designed in such a way.

You'll have to ask the committee. :-)  For C11, the C standardization 
committee decided to allow alignas only when declaring objects; alignas 
cannot be used in typedefs or in the definitions of structures.  Emacs 
prefers alignas when it works, since alignas is standardized and is more 
likely to be portable, but for this particular feature (an aligned 
structure type) Emacs uses a GCC extension instead, since there's no way 
to do it in the standard.

I'll try to review the code carefully soon.

Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

[Stefan Monnier]

>> Any reason you don't use the `alignas' we already use/check just above?
> An existing code uses alignas (X) for variable declaration,
> not for type declaration.  If I try the latter, I see:

Any chance `alignas' can be used where we create the stack-allocated
object, maybe?


        Stefan

Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

[Paul Eggert]

Stefan Monnier wrote:
> Any chance `alignas' can be used where we create the stack-allocated
> object, maybe?

Although I think we could do that, there would be quite a cost in terms 
of expressiveness.  The macro to create the stack-allocated object would 
have to expand to a declaration of a local variable, which would mean 
that it could not be a function-like macro scoped_cons (a, b) but 
instead would have to be a declaration-like macro SCOPED_NAMED_CONS 
(name, a, b), where NAME would give the name of the newly-allocated 
struct Lisp_Cons object.

I'm finding it a bit hard to think about all this, by the way, as it the 
new definitions aren't used anywhere, which makes it hard to see some of 
the motivation here.

Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

[Paul Eggert]

Paul Eggert wrote:
> I'll try to review the code carefully soon.

I installed some changes that I hope improve things.  The biggest was to 
change the new string and vector API to look more like the existing 
functional API, e.g., instead of "Lisp_Object obj; build_local_string 
(obj, s); v = f (obj);" one now says "v = f (build_local_string (s));".

Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

[Dmitry Antipov]

On 09/10/2014 10:43 AM, Paul Eggert wrote:

> I installed some changes that I hope improve things.

| (pointer_valid_for_lisp_object): Remove.  This check is not
| necessary, since make_lisp_ptr is already doing it.  All uses removed.

This is still in use in verify_alloca:

../../trunk/src/alloc.c: In function ‘verify_alloca’:
../../trunk/src/alloc.c:7179:7: error: implicit declaration of function ‘pointer_valid_for_lisp_object’ [-Werror=implicit-function-declaration]
        eassert (pointer_valid_for_lisp_object (ptr));
        ^
../../trunk/src/alloc.c:7179:7: error: nested extern declaration of ‘pointer_valid_for_lisp_object’ [-Werror=nested-externs]
cc1: all warnings being treated as errors

If you think that this function is redundant, feel free to remove it; otherwise let's use make_lisp_ptr.

Dmitry

Re: [Emacs-diffs] trunk r117846: Add macros to allocate temporary Lisp objects with alloca.

[Paul Eggert]

Dmitry Antipov wrote:
> If you think that this function is redundant, feel free to remove it;
> otherwise let's use make_lisp_ptr.

Thanks, I did the latter.