[PATCH] Use __attribute__((malloc)) for gcc 3.2

[PATCH] Use __attribute__((malloc)) for gcc 3.2

[Andi Kleen]

gcc 3.2 has an __attribute__((malloc)) function attribute. It tells gcc
that a function returns newly allocated memory and that the return pointer
cannot alias with any other pointer in the parent function. This often
allows gcc to generate better code because the optimizer doesn't need take
pointer aliasing in account.

This patch implements it for 2.5.39 and some common allocation functions.

Also added an noinline macro to wrap __attribute__((noinline)). That's 
not used yet. It tells the compiler that it should never inline, which
may be useful to prevent some awful code generation for those misguided
folks who use -O3 (gcc often screws up the register allocation of a 
function completely when bigger functions are inlined). 

Patch for 2.5.39.

Please consider applying.

-Andi

diff -X ../KDIFX -burp linux/include/asm-i386/pgalloc.h linux-2.5.39-work/include/asm-i386/pgalloc.h
--- linux/include/asm-i386/pgalloc.h	2002-09-25 00:59:27.000000000 +0200
+++ linux-2.5.39-work/include/asm-i386/pgalloc.h	2002-09-29 17:07:07.000000000 +0200
@@ -20,10 +20,10 @@ static inline void pmd_populate(struct m
  * Allocate and free page tables.
  */
 
-extern pgd_t *pgd_alloc(struct mm_struct *);
+extern pgd_t *pgd_alloc(struct mm_struct *) malloc_function;
 extern void pgd_free(pgd_t *pgd);
 
-extern pte_t *pte_alloc_one_kernel(struct mm_struct *, unsigned long);
+extern pte_t *pte_alloc_one_kernel(struct mm_struct *, unsigned long) malloc_function;
 extern struct page *pte_alloc_one(struct mm_struct *, unsigned long);
 
 static inline void pte_free_kernel(pte_t *pte)
diff -X ../KDIFX -burp linux/include/linux/bootmem.h linux-2.5.39-work/include/linux/bootmem.h
--- linux/include/linux/bootmem.h	2002-09-25 00:59:27.000000000 +0200
+++ linux-2.5.39-work/include/linux/bootmem.h	2002-09-29 17:03:39.000000000 +0200
@@ -37,7 +37,7 @@ typedef struct bootmem_data {
 extern unsigned long __init bootmem_bootmap_pages (unsigned long);
 extern unsigned long __init init_bootmem (unsigned long addr, unsigned long memend);
 extern void __init free_bootmem (unsigned long addr, unsigned long size);
-extern void * __init __alloc_bootmem (unsigned long size, unsigned long align, unsigned long goal);
+extern void * __init __alloc_bootmem (unsigned long size, unsigned long align, unsigned long goal) malloc_function;
 #ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE
 extern void __init reserve_bootmem (unsigned long addr, unsigned long size);
 #define alloc_bootmem(x) \
@@ -55,7 +55,7 @@ extern unsigned long __init init_bootmem
 extern void __init reserve_bootmem_node (pg_data_t *pgdat, unsigned long physaddr, unsigned long size);
 extern void __init free_bootmem_node (pg_data_t *pgdat, unsigned long addr, unsigned long size);
 extern unsigned long __init free_all_bootmem_node (pg_data_t *pgdat);
-extern void * __init __alloc_bootmem_node (pg_data_t *pgdat, unsigned long size, unsigned long align, unsigned long goal);
+extern void * __init __alloc_bootmem_node (pg_data_t *pgdat, unsigned long size, unsigned long align, unsigned long goal) malloc_function;
 #ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE
 #define alloc_bootmem_node(pgdat, x) \
 	__alloc_bootmem_node((pgdat), (x), SMP_CACHE_BYTES, __pa(MAX_DMA_ADDRESS))
diff -X ../KDIFX -burp linux/include/linux/dcache.h linux-2.5.39-work/include/linux/dcache.h
--- linux/include/linux/dcache.h	2002-09-29 16:54:34.000000000 +0200
+++ linux-2.5.39-work/include/linux/dcache.h	2002-09-29 17:03:57.000000000 +0200
@@ -173,8 +173,8 @@ extern void d_instantiate(struct dentry 
 extern void d_delete(struct dentry *);
 
 /* allocate/de-allocate */
-extern struct dentry * d_alloc(struct dentry *, const struct qstr *);
-extern struct dentry * d_alloc_anon(struct inode *);
+extern struct dentry * d_alloc(struct dentry *, const struct qstr *) malloc_function;
+extern struct dentry * d_alloc_anon(struct inode *) malloc_function;
 extern struct dentry * d_splice_alias(struct inode *, struct dentry *);
 extern void shrink_dcache_sb(struct super_block *);
 extern void shrink_dcache_parent(struct dentry *);
diff -X ../KDIFX -burp linux/include/linux/gfp.h linux-2.5.39-work/include/linux/gfp.h
--- linux/include/linux/gfp.h	2002-09-25 00:59:27.000000000 +0200
+++ linux-2.5.39-work/include/linux/gfp.h	2002-09-29 17:08:55.000000000 +0200
@@ -39,8 +39,8 @@
  * can allocate highmem pages, the *get*page*() variants return
  * virtual kernel addresses to the allocated page(s).
  */
-extern struct page * FASTCALL(__alloc_pages(unsigned int gfp_mask, unsigned int order, struct zonelist *zonelist));
-extern struct page * alloc_pages_node(int nid, unsigned int gfp_mask, unsigned int order);
+extern struct page * FASTCALL(__alloc_pages(unsigned int gfp_mask, unsigned int order, struct zonelist *zonelist)) malloc_function;
+extern struct page * alloc_pages_node(int nid, unsigned int gfp_mask, unsigned int order) malloc_function;
 
 /*
  * We get the zone list from the current node and the gfp_mask.
@@ -49,7 +49,8 @@ extern struct page * alloc_pages_node(in
  * For the normal case of non-DISCONTIGMEM systems the NODE_DATA() gets
  * optimized to &contig_page_data at compile-time.
  */
-static inline struct page * alloc_pages(unsigned int gfp_mask, unsigned int order)
+static inline malloc_function struct page * 
+alloc_pages(unsigned int gfp_mask, unsigned int order)
 {
 	pg_data_t *pgdat = NODE_DATA(numa_node_id());
 	unsigned int idx = (gfp_mask & GFP_ZONEMASK);
@@ -62,8 +63,8 @@ static inline struct page * alloc_pages(
 
 #define alloc_page(gfp_mask) alloc_pages(gfp_mask, 0)
 
-extern unsigned long FASTCALL(__get_free_pages(unsigned int gfp_mask, unsigned int order));
-extern unsigned long FASTCALL(get_zeroed_page(unsigned int gfp_mask));
+extern unsigned long FASTCALL(__get_free_pages(unsigned int gfp_mask, unsigned int order)) malloc_function; 
+extern unsigned long FASTCALL(get_zeroed_page(unsigned int gfp_mask)) malloc_function;
 
 #define __get_free_page(gfp_mask) \
 		__get_free_pages((gfp_mask),0)
diff -X ../KDIFX -burp linux/include/linux/jbd.h linux-2.5.39-work/include/linux/jbd.h
--- linux/include/linux/jbd.h	2002-09-25 00:59:27.000000000 +0200
+++ linux-2.5.39-work/include/linux/jbd.h	2002-09-29 17:04:40.000000000 +0200
@@ -54,7 +54,7 @@ extern int journal_enable_debug;
 #define jbd_debug(f, a...)	/**/
 #endif
 
-extern void * __jbd_kmalloc (char *where, size_t size, int flags, int retry);
+extern void * __jbd_kmalloc (char *where, size_t size, int flags, int retry) malloc_function;
 #define jbd_kmalloc(size, flags) \
 	__jbd_kmalloc(__FUNCTION__, (size), (flags), journal_oom_retry)
 #define jbd_rep_kmalloc(size, flags) \
diff -X ../KDIFX -burp linux/include/linux/kernel.h linux-2.5.39-work/include/linux/kernel.h
--- linux/include/linux/kernel.h	2002-09-29 16:54:34.000000000 +0200
+++ linux-2.5.39-work/include/linux/kernel.h	2002-09-29 17:20:52.000000000 +0200
@@ -47,6 +47,16 @@ void __might_sleep(char *file, int line)
 #define might_sleep() do {} while(0)
 #endif
 
+#if __GNUC__ >= 3 && __GNUC_MINOR__ >= 1
+/* Function allocates new memory and return cannot alias with anything */
+#define malloc_function __attribute__((malloc))
+/* Never inline */
+#define noinline __attribute__((noinline))
+#else
+#define malloc_function
+#define noinline
+#endif
+
 extern struct notifier_block *panic_notifier_list;
 NORET_TYPE void panic(const char * fmt, ...)
 	__attribute__ ((NORET_AND format (printf, 1, 2)));
diff -X ../KDIFX -burp linux/include/linux/mempool.h linux-2.5.39-work/include/linux/mempool.h
--- linux/include/linux/mempool.h	2002-09-25 00:59:27.000000000 +0200
+++ linux-2.5.39-work/include/linux/mempool.h	2002-09-29 17:05:04.000000000 +0200
@@ -21,10 +21,10 @@ typedef struct mempool_s {
 	wait_queue_head_t wait;
 } mempool_t;
 extern mempool_t * mempool_create(int min_nr, mempool_alloc_t *alloc_fn,
-				 mempool_free_t *free_fn, void *pool_data);
+				 mempool_free_t *free_fn, void *pool_data) malloc_function;
 extern int mempool_resize(mempool_t *pool, int new_min_nr, int gfp_mask);
 extern void mempool_destroy(mempool_t *pool);
-extern void * mempool_alloc(mempool_t *pool, int gfp_mask);
+extern void * mempool_alloc(mempool_t *pool, int gfp_mask) malloc_function;
 extern void mempool_free(void *element, mempool_t *pool);
 
 #endif /* _LINUX_MEMPOOL_H */
diff -X ../KDIFX -burp linux/include/linux/mm.h linux-2.5.39-work/include/linux/mm.h
--- linux/include/linux/mm.h	2002-09-29 16:54:35.000000000 +0200
+++ linux-2.5.39-work/include/linux/mm.h	2002-09-29 17:09:14.000000000 +0200
@@ -417,7 +417,8 @@ static inline int set_page_dirty(struct 
  * inlining and the symmetry break with pte_alloc_map() that does all
  * of this out-of-line.
  */
-static inline pmd_t *pmd_alloc(struct mm_struct *mm, pgd_t *pgd, unsigned long address)
+static inline malloc_function pmd_t *
+pmd_alloc(struct mm_struct *mm, pgd_t *pgd, unsigned long address) 
 {
 	if (pgd_none(*pgd))
 		return __pmd_alloc(mm, pgd, address);
diff -X ../KDIFX -burp linux/include/linux/skbuff.h linux-2.5.39-work/include/linux/skbuff.h
--- linux/include/linux/skbuff.h	2002-09-25 00:59:27.000000000 +0200
+++ linux-2.5.39-work/include/linux/skbuff.h	2002-09-29 17:02:37.000000000 +0200
@@ -259,11 +259,11 @@ struct sk_buff {
 #include <asm/system.h>
 
 extern void	       __kfree_skb(struct sk_buff *skb);
-extern struct sk_buff *alloc_skb(unsigned int size, int priority);
+extern struct sk_buff *alloc_skb(unsigned int size, int priority) malloc_function;
 extern void	       kfree_skbmem(struct sk_buff *skb);
-extern struct sk_buff *skb_clone(struct sk_buff *skb, int priority);
-extern struct sk_buff *skb_copy(const struct sk_buff *skb, int priority);
-extern struct sk_buff *pskb_copy(struct sk_buff *skb, int gfp_mask);
+extern struct sk_buff *skb_clone(struct sk_buff *skb, int priority) malloc_function;
+extern struct sk_buff *skb_copy(const struct sk_buff *skb, int priority) malloc_function;
+extern struct sk_buff *pskb_copy(struct sk_buff *skb, int gfp_mask) malloc_function;
 extern int	       pskb_expand_head(struct sk_buff *skb,
 					int nhead, int ntail, int gfp_mask);
 extern struct sk_buff *skb_realloc_headroom(struct sk_buff *skb,
diff -X ../KDIFX -burp linux/include/linux/slab.h linux-2.5.39-work/include/linux/slab.h
--- linux/include/linux/slab.h	2002-09-25 00:59:27.000000000 +0200
+++ linux-2.5.39-work/include/linux/slab.h	2002-09-29 16:56:49.000000000 +0200
@@ -55,11 +55,11 @@ extern kmem_cache_t *kmem_cache_create(c
 				       void (*)(void *, kmem_cache_t *, unsigned long));
 extern int kmem_cache_destroy(kmem_cache_t *);
 extern int kmem_cache_shrink(kmem_cache_t *);
-extern void *kmem_cache_alloc(kmem_cache_t *, int);
+extern void *kmem_cache_alloc(kmem_cache_t *, int) malloc_function;
 extern void kmem_cache_free(kmem_cache_t *, void *);
 extern unsigned int kmem_cache_size(kmem_cache_t *);
 
-extern void *kmalloc(size_t, int);
+extern void *kmalloc(size_t, int) malloc_function;
 extern void kfree(const void *);
 
 extern int FASTCALL(kmem_cache_reap(int));
diff -X ../KDIFX -burp linux/include/linux/vmalloc.h linux-2.5.39-work/include/linux/vmalloc.h
--- linux/include/linux/vmalloc.h	2002-09-25 00:59:27.000000000 +0200
+++ linux-2.5.39-work/include/linux/vmalloc.h	2002-09-29 17:06:33.000000000 +0200
@@ -21,9 +21,9 @@ struct vm_struct {
 /*
  *	Highlevel APIs for driver use
  */
-extern void *vmalloc(unsigned long size);
-extern void *vmalloc_32(unsigned long size);
-extern void *__vmalloc(unsigned long size, int gfp_mask, pgprot_t prot);
+extern void *vmalloc(unsigned long size) malloc_function;
+extern void *vmalloc_32(unsigned long size) malloc_function;
+extern void *__vmalloc(unsigned long size, int gfp_mask, pgprot_t prot) malloc_function;
 extern void vfree(void *addr);
 
 extern void *vmap(struct page **pages, unsigned int count);

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Adrian Bunk]

On Sun, 29 Sep 2002, Andi Kleen wrote:

>...
> --- linux/include/linux/kernel.h	2002-09-29 16:54:34.000000000 +0200
> +++ linux-2.5.39-work/include/linux/kernel.h	2002-09-29 17:20:52.000000000 +0200
> @@ -47,6 +47,16 @@ void __might_sleep(char *file, int line)
>  #define might_sleep() do {} while(0)
>  #endif
>
> +#if __GNUC__ >= 3 && __GNUC_MINOR__ >= 1
> +/* Function allocates new memory and return cannot alias with anything */
> +#define malloc_function __attribute__((malloc))
> +/* Never inline */
> +#define noinline __attribute__((noinline))
> +#else
> +#define malloc_function
> +#define noinline
> +#endif
> +
>...


Why did you put it in kernel.h and not in compiler.h?


cu
Adrian

-- 

You only think this is a free country. Like the US the UK spends a lot of
time explaining its a free country because its a police state.
								Alan Cox

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Christoph Hellwig]

On Sun, Sep 29, 2002 at 05:27:31PM +0200, Andi Kleen wrote:
> -extern pgd_t *pgd_alloc(struct mm_struct *);
> +extern pgd_t *pgd_alloc(struct mm_struct *) malloc_function;

Anz chance you could make that __malloc?  That how the other
atrributes in the kernel (e.g. __init/__exit) work.

> +/* Function allocates new memory and return cannot alias with anything */
> +#define malloc_function __attribute__((malloc))
> +/* Never inline */
> +#define noinline __attribute__((noinline))
> +#else
> +#define malloc_function
> +#define noinline
> +#endif

Dito for __noinline?  And IMHO compiler.h is the better place for this.

BTW, do you have any stats on the better optimization?

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Jakub Jelinek]

On Sun, Sep 29, 2002 at 05:27:31PM +0200, Andi Kleen wrote:
> 
> gcc 3.2 has an __attribute__((malloc)) function attribute. It tells gcc
> that a function returns newly allocated memory and that the return pointer
> cannot alias with any other pointer in the parent function. This often
> allows gcc to generate better code because the optimizer doesn't need take
> pointer aliasing in account.

Does this matter when the kernel is compiled with -fno-strict-aliasing?

	Jakub

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Anton Blanchard]

Hi Andi,

> Also added an noinline macro to wrap __attribute__((noinline)). That's 
> not used yet. It tells the compiler that it should never inline, which
> may be useful to prevent some awful code generation for those misguided
> folks who use -O3 (gcc often screws up the register allocation of a 
> function completely when bigger functions are inlined). 

Could you also add an always inline? It would be useful for functions
like context_switch, where we require it to be inlined (otherwise it
falls outside scheduling_functions_{start,end}_here and wchan handling
fails).

Anton

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Andi Kleen]

Jakub Jelinek <jakub@redhat.com> writes:

> On Sun, Sep 29, 2002 at 05:27:31PM +0200, Andi Kleen wrote:
> > 
> > gcc 3.2 has an __attribute__((malloc)) function attribute. It tells gcc
> > that a function returns newly allocated memory and that the return pointer
> > cannot alias with any other pointer in the parent function. This often
> > allows gcc to generate better code because the optimizer doesn't need take
> > pointer aliasing in account.
> 
> Does this matter when the kernel is compiled with -fno-strict-aliasing?

I don't know. But it's certainly worth trying, isn't it ?  Giving the 
compiler more information can't be a bad thing.

-Andi

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[David S. Miller]

   From: Christoph Hellwig <hch@infradead.org>
   Date: Sun, 29 Sep 2002 18:26:43 +0100
   
   BTW, do you have any stats on the better optimization?

Unlikely since we disable strict aliasing on the gcc command
line which is why I think this suggested __malloc thing is
utterly pointless.

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Andi Kleen]

On Mon, Sep 30, 2002 at 02:11:10AM +0200, David S. Miller wrote:
>    From: Christoph Hellwig <hch@infradead.org>
>    Date: Sun, 29 Sep 2002 18:26:43 +0100
>    
>    BTW, do you have any stats on the better optimization?
> 
> Unlikely since we disable strict aliasing on the gcc command
> line which is why I think this suggested __malloc thing is
> utterly pointless.

My understanding of it is: Please correct me if I'm wrong [i haven't 
verified this with tree dumps]

-fno-strict-aliasing tells each pointer that it can alias with everything
else (puts it in alias set 0)

__attribute__((malloc)) overwrites this so that the compiler knows that
this particular pointer that has been returned  (puts it into an own
alias set again and overwrites the -fno-strict-aliasing default) 

Another way to overwrite it would be restrict, but nobody uses that
currently. May make sense to add it to compile compiler version checked
macros too, so that it can be safely used.

Then it would be actually possible to write functions that get optimized
this way. I'm aware that the aliasing stuff mostly helps with array walks
and loops, which are not that common in the kernel, but it could be still
useful.

-Andi

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Andi Kleen]

On Mon, Sep 30, 2002 at 01:51:41AM +0200, Anton Blanchard wrote:
> 
> Hi Andi,
> 
> > Also added an noinline macro to wrap __attribute__((noinline)). That's 
> > not used yet. It tells the compiler that it should never inline, which
> > may be useful to prevent some awful code generation for those misguided
> > folks who use -O3 (gcc often screws up the register allocation of a 
> > function completely when bigger functions are inlined). 
> 
> Could you also add an always inline? It would be useful for functions
> like context_switch, where we require it to be inlined (otherwise it
> falls outside scheduling_functions_{start,end}_here and wchan handling
> fails).

Ok. gcc supports it with __attribute__((always_inline))

Suggestions for a name? alwaysinline would be a bit lengthy.

-Andi

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Daniel Jacobowitz]

On Mon, Sep 30, 2002 at 02:31:21AM +0200, Andi Kleen wrote:
> On Mon, Sep 30, 2002 at 01:51:41AM +0200, Anton Blanchard wrote:
> > 
> > Hi Andi,
> > 
> > > Also added an noinline macro to wrap __attribute__((noinline)). That's 
> > > not used yet. It tells the compiler that it should never inline, which
> > > may be useful to prevent some awful code generation for those misguided
> > > folks who use -O3 (gcc often screws up the register allocation of a 
> > > function completely when bigger functions are inlined). 
> > 
> > Could you also add an always inline? It would be useful for functions
> > like context_switch, where we require it to be inlined (otherwise it
> > falls outside scheduling_functions_{start,end}_here and wchan handling
> > fails).
> 
> Ok. gcc supports it with __attribute__((always_inline))
> 
> Suggestions for a name? alwaysinline would be a bit lengthy.

Stick with always_inline?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Pavel Machek]

Hi!


> > > may be useful to prevent some awful code generation for those misguided
> > > folks who use -O3 (gcc often screws up the register allocation of a 
> > > function completely when bigger functions are inlined). 
> > 
> > Could you also add an always inline? It would be useful for functions
> > like context_switch, where we require it to be inlined (otherwise it
> > falls outside scheduling_functions_{start,end}_here and wchan handling
> > fails).
> 
> Ok. gcc supports it with __attribute__((always_inline))
> 
> Suggestions for a name? alwaysinline would be a bit lengthy.

do_inline?
								Pavel
-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Richard Henderson]

On Sun, Sep 29, 2002 at 04:01:13PM -0400, Jakub Jelinek wrote:
> Does this matter when the kernel is compiled with -fno-strict-aliasing?

Yes.  The malloc attribute uses REG_NOALIAS, not alias sets.


r~

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[David S. Miller]

   From: Richard Henderson <rth@twiddle.net>
   Date: Mon, 7 Oct 2002 03:05:41 -0700

   On Sun, Sep 29, 2002 at 04:01:13PM -0400, Jakub Jelinek wrote:
   > Does this matter when the kernel is compiled with -fno-strict-aliasing?
   
   Yes.  The malloc attribute uses REG_NOALIAS, not alias sets.
   
Great, I'm all for Andi's patch in that case.

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Andi Kleen]

On Mon, Oct 07, 2002 at 12:29:00PM +0200, David S. Miller wrote:
>    From: Richard Henderson <rth@twiddle.net>
>    Date: Mon, 7 Oct 2002 03:05:41 -0700
> 
>    On Sun, Sep 29, 2002 at 04:01:13PM -0400, Jakub Jelinek wrote:
>    > Does this matter when the kernel is compiled with -fno-strict-aliasing?
>    
>    Yes.  The malloc attribute uses REG_NOALIAS, not alias sets.
>    
> Great, I'm all for Andi's patch in that case.

I retested it on gcc 3.2 and it unfortunately doesn't make any difference
(resulting kernel is byte-for-byte identical). So it looks like with
current gcc it isn't worth the effort.

-Andi

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

On Mon, 2002-10-07 at 12:56, Andi Kleen wrote:
> On Mon, Oct 07, 2002 at 12:29:00PM +0200, David S. Miller wrote:
> >    From: Richard Henderson <rth@twiddle.net>
> >    Date: Mon, 7 Oct 2002 03:05:41 -0700
> > 
> >    On Sun, Sep 29, 2002 at 04:01:13PM -0400, Jakub Jelinek wrote:
> >    > Does this matter when the kernel is compiled with -fno-strict-aliasing?
> >    
> >    Yes.  The malloc attribute uses REG_NOALIAS, not alias sets.
> >    
> > Great, I'm all for Andi's patch in that case.
> 
> I retested it on gcc 3.2 and it unfortunately doesn't make any difference
> (resulting kernel is byte-for-byte identical). So it looks like with
> current gcc it isn't worth the effort.

I'm still in favor of doing it, even if it's just to break the
chick-and-egg deadlock that such issues can have.

Greetings,
  Arjan van de Ven

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Richard Henderson]

On Mon, Oct 07, 2002 at 12:56:22PM +0200, Andi Kleen wrote:
> I retested it on gcc 3.2 and it unfortunately doesn't make any difference
> (resulting kernel is byte-for-byte identical). So it looks like with
> current gcc it isn't worth the effort.

*shrug* It's still good documentation of intent.  And one of
these days we might get around to writing some aliasing code
that doesn't suck quite so much.  ;-)


r~

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Andi Kleen]

On Mon, Oct 07, 2002 at 01:45:06PM +0200, Richard Henderson wrote:
> On Mon, Oct 07, 2002 at 12:56:22PM +0200, Andi Kleen wrote:
> > I retested it on gcc 3.2 and it unfortunately doesn't make any difference
> > (resulting kernel is byte-for-byte identical). So it looks like with
> > current gcc it isn't worth the effort.
> 
> *shrug* It's still good documentation of intent.  And one of
> these days we might get around to writing some aliasing code
> that doesn't suck quite so much.  ;-)

I'll resubmit the patch then.

Thanks for the feedback.

-Andi

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[David S. Miller]

   From: Andi Kleen <ak@suse.de>
   Date: 29 Sep 2002 21:09:26 +0200
   
   No, with gcc 3.2 it doesn't seem to make much difference.

Because -fno-strict-aliasing is still in CFLAGS.  Your
change is pointless until that situation changes.

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Adrian Bunk]

On 29 Sep 2002, Andi Kleen wrote:

>...
> I will move it into linux/compiler.h to add some more clutter to include hell,
> because it requires even more #include <linux/compiler.h>
>...

kernel.h includes compiler.h so it shouldn't make a difference.

> -Andi

cu
Adrian

-- 

You only think this is a free country. Like the US the UK spends a lot of
time explaining its a free country because its a police state.
								Alan Cox

Re: [PATCH] Use __attribute__((malloc)) for gcc 3.2

[Andi Kleen]

Christoph Hellwig <hch@infradead.org> writes:

> On Sun, Sep 29, 2002 at 05:27:31PM +0200, Andi Kleen wrote:
> > -extern pgd_t *pgd_alloc(struct mm_struct *);
> > +extern pgd_t *pgd_alloc(struct mm_struct *) malloc_function;
> 
> Anz chance you could make that __malloc?  That how the other
> atrributes in the kernel (e.g. __init/__exit) work.

No.

> 
> > +/* Function allocates new memory and return cannot alias with anything */
> > +#define malloc_function __attribute__((malloc))
> > +/* Never inline */
> > +#define noinline __attribute__((noinline))
> > +#else
> > +#define malloc_function
> > +#define noinline
> > +#endif
> 
> Dito for __noinline?  And IMHO compiler.h is the better place for this.

Because I dislike all the __. It's just useless visual clutter and doesn't 
follow the usual convention of prefixing only low level functions with this.
Also now that the kernel has given up on ANSI/POSIX name space cleanliness
it is double plus useless.

Same thing for __get_cpu_var for example. The __ is completely useless.

I will move it into linux/compiler.h to add some more clutter to include hell,
because it requires even more #include <linux/compiler.h>

> BTW, do you have any stats on the better optimization?

With this it sings twice as loud and dances twice and a half times as fast.

No, with gcc 3.2 it doesn't seem to make much difference.

More important is that will allow better optimization by gcc in the future.
In kernel land the gcc optimizer is already a bit crippled because of 
the -fno-strict-aliasing use. This will hopefully help a bit.

-Andi