public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* adding -fnoalias ... would a patch be accepted ?
@ 2010-01-05 13:40 torbenh
  2010-01-05 13:46 ` Richard Guenther
  0 siblings, 1 reply; 15+ messages in thread
From: torbenh @ 2010-01-05 13:40 UTC (permalink / raw)
  To: gcc


hi...

i am new to this list. 

i am trying to something like:

struct Ramp
{
    float phase;
    inline float process() { return phase++; }
} ramp;

void fill_buffer( float *buf, size_t nframes )
{
	for( size_t i=0; i<nframes; i++ )
		buf[i] = ramp.process()
}

the goal is to chain several of such Ramp structs together via
templates. so that i can model dsp graphs, but have it all inlined into
the loop.

however the stores to phase cant be moved out of the loop due to
aliasing. but that is one of the points to do this whole exercise.

this simple case can be optimized when i use -fargument-noalias-anything
and:

void fill_buffer( float *buf, size_t nframes )
{
	for( size_t i=0; i<nframes; i++ )
		*(buf++) = ramp.process()
}

but things start to break again, when i add:

struct Sample
{
	unsigned int pos;
	float *sample;
	inline float process() { return sample[pos++]; }
}


__restrict__ is of no help here. which leads me to the question whats
the point of a restricted this pointer ? members of structs arent
unaliased by a __restrict__ pointer to the struct. 

my favourite solution would be __noalias__ ... msvc has that. 
but -fnoalias would make me happy too.

i havent read much of the gcc code yet, so i am not sure what i need to
patch. 

refs_may_alias_p_1() is my current bet though.

-- 
torben Hohn

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-05 13:40 adding -fnoalias ... would a patch be accepted ? torbenh
@ 2010-01-05 13:46 ` Richard Guenther
  2010-01-05 15:02   ` torbenh
  2010-01-05 15:03   ` torbenh
  0 siblings, 2 replies; 15+ messages in thread
From: Richard Guenther @ 2010-01-05 13:46 UTC (permalink / raw)
  To: gcc

On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
>
> hi...
>
> i am new to this list.
>
> i am trying to something like:
>
> struct Ramp
> {
>    float phase;
>    inline float process() { return phase++; }
> } ramp;
>
> void fill_buffer( float *buf, size_t nframes )
> {
>        for( size_t i=0; i<nframes; i++ )
>                buf[i] = ramp.process()
> }
>
> the goal is to chain several of such Ramp structs together via
> templates. so that i can model dsp graphs, but have it all inlined into
> the loop.
>
> however the stores to phase cant be moved out of the loop due to
> aliasing. but that is one of the points to do this whole exercise.
>
> this simple case can be optimized when i use -fargument-noalias-anything
> and:
>
> void fill_buffer( float *buf, size_t nframes )
> {
>        for( size_t i=0; i<nframes; i++ )
>                *(buf++) = ramp.process()
> }
>
> but things start to break again, when i add:
>
> struct Sample
> {
>        unsigned int pos;
>        float *sample;
>        inline float process() { return sample[pos++]; }
> }
>
>
> __restrict__ is of no help here. which leads me to the question whats
> the point of a restricted this pointer ? members of structs arent
> unaliased by a __restrict__ pointer to the struct.
>
> my favourite solution would be __noalias__ ... msvc has that.
> but -fnoalias would make me happy too.
>
> i havent read much of the gcc code yet, so i am not sure what i need to
> patch.
>
> refs_may_alias_p_1() is my current bet though.

The -fno-alias-X things do not make much sense for user code (they
have been historically used from Frontends).  If restrict doesn't work
for you (do you have a testcase that can reproduce your issue?)
then you probably need to wait for IPA pointer analysis to be
fixed in GCC 4.6.

Richard.

> --
> torben Hohn
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-05 13:46 ` Richard Guenther
@ 2010-01-05 15:02   ` torbenh
  2010-01-05 16:42     ` Tim Prince
  2010-01-05 15:03   ` torbenh
  1 sibling, 1 reply; 15+ messages in thread
From: torbenh @ 2010-01-05 15:02 UTC (permalink / raw)
  To: gcc

On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
> > __restrict__ is of no help here. which leads me to the question whats
> > the point of a restricted this pointer ? members of structs arent
> > unaliased by a __restrict__ pointer to the struct.
> >
> > my favourite solution would be __noalias__ ... msvc has that.
> > but -fnoalias would make me happy too.
> >
> > i havent read much of the gcc code yet, so i am not sure what i need to
> > patch.
> >
> > refs_may_alias_p_1() is my current bet though.
> 
> The -fno-alias-X things do not make much sense for user code (they
> have been historically used from Frontends).  If restrict doesn't work
> for you (do you have a testcase that can reproduce your issue?)
> then you probably need to wait for IPA pointer analysis to be
> fixed in GCC 4.6.

can you please explain, why you reject the idea of -fnoalias ? 
msvc has declspec(noalias) icc has -fnoalias 


ramp.cc attached.
compiling it with gcc -S -O3 ramp.cc on x86_64 yields:

------------------------------------------------------------
_Z11fill_bufferPfm:
.LFB1:
	testq	%rsi, %rsi
	je	.L1
	xorl	%eax, %eax
	movss	.LC0(%rip), %xmm2
	.align 16
.L3:
	movss	ramp(%rip), %xmm0
	movaps	%xmm0, %xmm1
	addss	%xmm2, %xmm1
	movss	%xmm1, ramp(%rip)
	movss	%xmm0, (%rdi,%rax,4)
	addq	$1, %rax
	cmpq	%rax, %rsi
	ja	.L3
.L1:
	rep
	ret
-----------------------------------------------------------


-- 
torben Hohn

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-05 13:46 ` Richard Guenther
  2010-01-05 15:02   ` torbenh
@ 2010-01-05 15:03   ` torbenh
  2010-01-05 15:27     ` Richard Guenther
  1 sibling, 1 reply; 15+ messages in thread
From: torbenh @ 2010-01-05 15:03 UTC (permalink / raw)
  To: gcc

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

On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
> 
> The -fno-alias-X things do not make much sense for user code (they
> have been historically used from Frontends).  If restrict doesn't work
> for you (do you have a testcase that can reproduce your issue?)
> then you probably need to wait for IPA pointer analysis to be
> fixed in GCC 4.6.

sorry... forget the attachment :S


-- 
torben Hohn

[-- Attachment #2: ramp.cc --]
[-- Type: text/x-c++src, Size: 224 bytes --]


#include "stddef.h"

struct Ramp {
    float phase;
    inline float process() { return phase++; }
} ramp;

void fill_buffer( float *buf, size_t nframes )
{
    for( size_t i=0; i<nframes; i++ )
	buf[i] = ramp.process();
}

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-05 15:03   ` torbenh
@ 2010-01-05 15:27     ` Richard Guenther
  2010-01-05 16:39       ` torbenh
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Guenther @ 2010-01-05 15:27 UTC (permalink / raw)
  To: gcc

On Tue, Jan 5, 2010 at 4:03 PM, torbenh <torbenh@gmx.de> wrote:
> On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
>> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
>>
>> The -fno-alias-X things do not make much sense for user code (they
>> have been historically used from Frontends).  If restrict doesn't work
>> for you (do you have a testcase that can reproduce your issue?)
>> then you probably need to wait for IPA pointer analysis to be
>> fixed in GCC 4.6.
>
> sorry... forget the attachment :S

Yes, in this case you can fix it by making ramp static.  Otherwise its
address may be takein in another translation unit.  For Fortran we
have the DECL_RESTRICTED_P which we could expose to other
languages via an attribute.  It tells that a decl is not aliased by
restrict qualified pointers, so

struct Ramp {
    float phase;
    inline float process() { return phase++; }
} ramp __attribute__((restrict));

void fill_buffer( float * __restrict buf, size_t nframes )
{
    for( size_t i=0; i<nframes; i++ )
        buf[i] = ramp.process();
}

would then be optimized as well.  Can you file an enhancement
bugreport according to this?

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-05 15:27     ` Richard Guenther
@ 2010-01-05 16:39       ` torbenh
  2010-01-06 13:27         ` Richard Guenther
  0 siblings, 1 reply; 15+ messages in thread
From: torbenh @ 2010-01-05 16:39 UTC (permalink / raw)
  To: gcc

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

On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
> On Tue, Jan 5, 2010 at 4:03 PM, torbenh <torbenh@gmx.de> wrote:
> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
> >>
> >> The -fno-alias-X things do not make much sense for user code (they
> >> have been historically used from Frontends).  If restrict doesn't work
> >> for you (do you have a testcase that can reproduce your issue?)
> >> then you probably need to wait for IPA pointer analysis to be
> >> fixed in GCC 4.6.
> >
> > sorry... forget the attachment :S
> 
> Yes, in this case you can fix it by making ramp static.  Otherwise its
> address may be takein in another translation unit.  For Fortran we
> have the DECL_RESTRICTED_P which we could expose to other
> languages via an attribute.  It tells that a decl is not aliased by
> restrict qualified pointers, so
> 
> struct Ramp {
>     float phase;
>     inline float process() { return phase++; }
> } ramp __attribute__((restrict));
> 
> void fill_buffer( float * __restrict buf, size_t nframes )
> {
>     for( size_t i=0; i<nframes; i++ )
>         buf[i] = ramp.process();
> }

would that also work with this stuff:


template<typename ... Args>
class Mixer;

template<typename T1, typename ... Args>
class Mixer<T1, Args...> : public Block
{
    private:
	T1 t1 __attribute__((restrict));
	Mixer<Args...> t2;
    public:
	inline float process() {
	    return t1.process() + t2.process();
	}
};

template<typename T1, typename T2>
class Mixer<T1,T2> : public Block
{
    private:
	T1 t1 __attribute__((restrict));
	T2 t2 __attribute__((restrict));
    public:
	inline float process() {
	    return t1.process() + t2.process();
	}
};

Mixer<Ramp,Ramp,Ramp,Ramp> mix __attribute__((restrict))

?

i still dont understand whats the problem with -fnolias,
as in attached patch.

> 
> would then be optimized as well.  Can you file an enhancement
> bugreport according to this?
> 
> Thanks,
> Richard.

-- 
torben Hohn

[-- Attachment #2: noalias.patch --]
[-- Type: text/x-diff, Size: 1385 bytes --]

diff --git a/gcc/common.opt b/gcc/common.opt
index 77967f8..21eebb2 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -834,6 +834,10 @@ freschedule-modulo-scheduled-loops
 Common Report Var(flag_resched_modulo_sched) Optimization
 Enable/Disable the traditional scheduling in loops that already passed modulo scheduling
 
+fnoalias
+Common Report Var(flag_noalias) Optimization
+Assume no aliasing is happening
+
 fnon-call-exceptions
 Common Report Var(flag_non_call_exceptions) Optimization
 Support synchronous non-call exceptions
diff --git a/gcc/opts.c b/gcc/opts.c
index 5407527..9b8639e 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2053,6 +2053,10 @@ common_handle_option (size_t scode, const char *arg, int value,
       flag_ipa_cp_clone_set = true;
       break;
 
+    case OPT_fnoalias:
+      flag_noalias = true;
+      break;
+
     case OPT_fpredictive_commoning:
       flag_predictive_commoning_set = true;
       break;
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index cbb43b5..0b66577 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -662,6 +662,9 @@ indirect_ref_may_alias_decl_p (tree ref1, tree ptr1,
   if (!ptr_deref_may_alias_decl_p (ptr1, base2))
     return false;
 
+  if (flag_noalias)
+    return false;
+
   /* Disambiguations that rely on strict aliasing rules follow.  */
   if (!flag_strict_aliasing)
     return true;

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-05 15:02   ` torbenh
@ 2010-01-05 16:42     ` Tim Prince
  0 siblings, 0 replies; 15+ messages in thread
From: Tim Prince @ 2010-01-05 16:42 UTC (permalink / raw)
  To: gcc

torbenh wrote:

> can you please explain, why you reject the idea of -fnoalias ? 
> msvc has declspec(noalias) icc has -fnoalias 
> 
msvc needs it because it doesn't implement restrict and supports 
violation of typed aliasing rules as a default.  ICL needs it for msvc 
compatibility, but has better alternatives. gcc can't copy the worst 
features of msvc.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-05 16:39       ` torbenh
@ 2010-01-06 13:27         ` Richard Guenther
  2010-01-06 15:26           ` torbenh
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Guenther @ 2010-01-06 13:27 UTC (permalink / raw)
  To: gcc

On Tue, Jan 5, 2010 at 5:39 PM, torbenh <torbenh@gmx.de> wrote:
> On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
>> On Tue, Jan 5, 2010 at 4:03 PM, torbenh <torbenh@gmx.de> wrote:
>> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
>> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
>> >>
>> >> The -fno-alias-X things do not make much sense for user code (they
>> >> have been historically used from Frontends).  If restrict doesn't work
>> >> for you (do you have a testcase that can reproduce your issue?)
>> >> then you probably need to wait for IPA pointer analysis to be
>> >> fixed in GCC 4.6.
>> >
>> > sorry... forget the attachment :S
>>
>> Yes, in this case you can fix it by making ramp static.  Otherwise its
>> address may be takein in another translation unit.  For Fortran we
>> have the DECL_RESTRICTED_P which we could expose to other
>> languages via an attribute.  It tells that a decl is not aliased by
>> restrict qualified pointers, so
>>
>> struct Ramp {
>>     float phase;
>>     inline float process() { return phase++; }
>> } ramp __attribute__((restrict));
>>
>> void fill_buffer( float * __restrict buf, size_t nframes )
>> {
>>     for( size_t i=0; i<nframes; i++ )
>>         buf[i] = ramp.process();
>> }
>
> would that also work with this stuff:
>
>
> template<typename ... Args>
> class Mixer;
>
> template<typename T1, typename ... Args>
> class Mixer<T1, Args...> : public Block
> {
>    private:
>        T1 t1 __attribute__((restrict));
>        Mixer<Args...> t2;
>    public:
>        inline float process() {
>            return t1.process() + t2.process();
>        }
> };
>
> template<typename T1, typename T2>
> class Mixer<T1,T2> : public Block
> {
>    private:
>        T1 t1 __attribute__((restrict));
>        T2 t2 __attribute__((restrict));
>    public:
>        inline float process() {
>            return t1.process() + t2.process();
>        }
> };
>
> Mixer<Ramp,Ramp,Ramp,Ramp> mix __attribute__((restrict))
>
> ?

I don't see a restrict qualified pointer here.  Note that the
restrict attribute would only disambiguate against those.
Also I think you need the restrict attribute on the Mixer
objects, not its members.

> i still dont understand whats the problem with -fnolias,
> as in attached patch.

The patch will miscompile everything.

Richard.

>>
>> would then be optimized as well.  Can you file an enhancement
>> bugreport according to this?
>>
>> Thanks,
>> Richard.
>
> --
> torben Hohn
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-06 13:27         ` Richard Guenther
@ 2010-01-06 15:26           ` torbenh
  2010-01-06 15:45             ` Richard Guenther
  2010-01-06 15:45             ` torbenh
  0 siblings, 2 replies; 15+ messages in thread
From: torbenh @ 2010-01-06 15:26 UTC (permalink / raw)
  To: gcc

On Wed, Jan 06, 2010 at 02:27:15PM +0100, Richard Guenther wrote:
> On Tue, Jan 5, 2010 at 5:39 PM, torbenh <torbenh@gmx.de> wrote:
> > On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
> >> On Tue, Jan 5, 2010 at 4:03 PM, torbenh <torbenh@gmx.de> wrote:
> >> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> >> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
> >> >>
> >> >> The -fno-alias-X things do not make much sense for user code (they
> >> >> have been historically used from Frontends).  If restrict doesn't work
> >> >> for you (do you have a testcase that can reproduce your issue?)
> >> >> then you probably need to wait for IPA pointer analysis to be
> >> >> fixed in GCC 4.6.
> >> >
> >> > sorry... forget the attachment :S
> >>
> >> Yes, in this case you can fix it by making ramp static.  Otherwise its
> >> address may be takein in another translation unit.  For Fortran we
> >> have the DECL_RESTRICTED_P which we could expose to other
> >> languages via an attribute.  It tells that a decl is not aliased by
> >> restrict qualified pointers, so
> >>
> >> struct Ramp {
> >>     float phase;
> >>     inline float process() { return phase++; }
> >> } ramp __attribute__((restrict));
> >>
> >> void fill_buffer( float * __restrict buf, size_t nframes )
> >> {
> >>     for( size_t i=0; i<nframes; i++ )
> >>         buf[i] = ramp.process();
> >> }
> >
> > would that also work with this stuff:
> >
> >
> > template<typename ... Args>
> > class Mixer;
> >
> > template<typename T1, typename ... Args>
> > class Mixer<T1, Args...> : public Block
> > {
> >    private:
> >        T1 t1 __attribute__((restrict));
> >        Mixer<Args...> t2;
> >    public:
> >        inline float process() {
> >            return t1.process() + t2.process();
> >        }
> > };
> >
> > template<typename T1, typename T2>
> > class Mixer<T1,T2> : public Block
> > {
> >    private:
> >        T1 t1 __attribute__((restrict));
> >        T2 t2 __attribute__((restrict));
> >    public:
> >        inline float process() {
> >            return t1.process() + t2.process();
> >        }
> > };
> >
> > Mixer<Ramp,Ramp,Ramp,Ramp> mix __attribute__((restrict))

void fill_buffer( float * __restrict buf, size_t nframes )
{
    for( size_t i=0; i<nframes; i++ )
        buf[i] = mix.process();
}

there is your pointer :)

> >
> > ?
> 
> I don't see a restrict qualified pointer here.  Note that the
> restrict attribute would only disambiguate against those.
> Also I think you need the restrict attribute on the Mixer
> objects, not its members.

so the attribute would promote down to all member vars of 
member objects ?


> > i still dont understand whats the problem with -fnolias,
> > as in attached patch.
> 
> The patch will miscompile everything.

point taken. 
obviously reading code for a few hours without knowing enough about the
code isnt enough :)

__attribute__((restrict)) is the better solution.
although not portable to other compilers. 

but i need this kind of functionality now, to test my concepts. 
thats why i am spending a bit time on this. 

when do you plan to add this feature ?
since you know the code, there would be no point for me to tackle
it if you do it soonish. 

(and you dont need to deal with dumb patches from me :)


speaking of dumb patches:

would you care to comment this patch for gcc-4.4 ?
this one seems to work for simple examples.

-- 
torben Hohn

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-06 15:26           ` torbenh
@ 2010-01-06 15:45             ` Richard Guenther
  2010-01-06 15:49               ` Richard Guenther
  2010-01-06 17:37               ` torbenh
  2010-01-06 15:45             ` torbenh
  1 sibling, 2 replies; 15+ messages in thread
From: Richard Guenther @ 2010-01-06 15:45 UTC (permalink / raw)
  To: gcc

On Wed, Jan 6, 2010 at 4:25 PM, torbenh <torbenh@gmx.de> wrote:
> On Wed, Jan 06, 2010 at 02:27:15PM +0100, Richard Guenther wrote:
>> On Tue, Jan 5, 2010 at 5:39 PM, torbenh <torbenh@gmx.de> wrote:
>> > On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
>> >> On Tue, Jan 5, 2010 at 4:03 PM, torbenh <torbenh@gmx.de> wrote:
>> >> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
>> >> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
>> >> >>
>> >> >> The -fno-alias-X things do not make much sense for user code (they
>> >> >> have been historically used from Frontends).  If restrict doesn't work
>> >> >> for you (do you have a testcase that can reproduce your issue?)
>> >> >> then you probably need to wait for IPA pointer analysis to be
>> >> >> fixed in GCC 4.6.
>> >> >
>> >> > sorry... forget the attachment :S
>> >>
>> >> Yes, in this case you can fix it by making ramp static.  Otherwise its
>> >> address may be takein in another translation unit.  For Fortran we
>> >> have the DECL_RESTRICTED_P which we could expose to other
>> >> languages via an attribute.  It tells that a decl is not aliased by
>> >> restrict qualified pointers, so
>> >>
>> >> struct Ramp {
>> >>     float phase;
>> >>     inline float process() { return phase++; }
>> >> } ramp __attribute__((restrict));
>> >>
>> >> void fill_buffer( float * __restrict buf, size_t nframes )
>> >> {
>> >>     for( size_t i=0; i<nframes; i++ )
>> >>         buf[i] = ramp.process();
>> >> }
>> >
>> > would that also work with this stuff:
>> >
>> >
>> > template<typename ... Args>
>> > class Mixer;
>> >
>> > template<typename T1, typename ... Args>
>> > class Mixer<T1, Args...> : public Block
>> > {
>> >    private:
>> >        T1 t1 __attribute__((restrict));
>> >        Mixer<Args...> t2;
>> >    public:
>> >        inline float process() {
>> >            return t1.process() + t2.process();
>> >        }
>> > };
>> >
>> > template<typename T1, typename T2>
>> > class Mixer<T1,T2> : public Block
>> > {
>> >    private:
>> >        T1 t1 __attribute__((restrict));
>> >        T2 t2 __attribute__((restrict));
>> >    public:
>> >        inline float process() {
>> >            return t1.process() + t2.process();
>> >        }
>> > };
>> >
>> > Mixer<Ramp,Ramp,Ramp,Ramp> mix __attribute__((restrict))
>
> void fill_buffer( float * __restrict buf, size_t nframes )
> {
>     for( size_t i=0; i<nframes; i++ )
>         buf[i] = mix.process();
> }
>
> there is your pointer :)

ok ;)

>> >
>> > ?
>>
>> I don't see a restrict qualified pointer here.  Note that the
>> restrict attribute would only disambiguate against those.
>> Also I think you need the restrict attribute on the Mixer
>> objects, not its members.
>
> so the attribute would promote down to all member vars of
> member objects ?

Yes.  In fact the example above would work I guess.

>
>> > i still dont understand whats the problem with -fnolias,
>> > as in attached patch.
>>
>> The patch will miscompile everything.
>
> point taken.
> obviously reading code for a few hours without knowing enough about the
> code isnt enough :)
>
> __attribute__((restrict)) is the better solution.
> although not portable to other compilers.
>
> but i need this kind of functionality now, to test my concepts.
> thats why i am spending a bit time on this.
>
> when do you plan to add this feature ?

For GCC 4.6 earliest.

> since you know the code, there would be no point for me to tackle
> it if you do it soonish.

It should be simple - just look into c-common.c, add this attribute
and in the handler, for VAR_DECLs simply set
DECL_RESTRICTED_P like:

static tree
handle_restrict_attribute (tree *node, ..., bool *no_add_attrs)
{
  if (TREE_CODE (*node) == VAR_DECL)
    DECL_RESTRICTED_P (*node) = true;
  *no_add_attrs = true;
  return NULL_TREE;
}

> (and you dont need to deal with dumb patches from me :)
>
>
> speaking of dumb patches:
>
> would you care to comment this patch for gcc-4.4 ?
> this one seems to work for simple examples.

Which patch?

Richard.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-06 15:26           ` torbenh
  2010-01-06 15:45             ` Richard Guenther
@ 2010-01-06 15:45             ` torbenh
  2010-01-06 15:51               ` Richard Guenther
  1 sibling, 1 reply; 15+ messages in thread
From: torbenh @ 2010-01-06 15:45 UTC (permalink / raw)
  To: gcc

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

On Wed, Jan 06, 2010 at 04:25:59PM +0100, torbenh wrote:
> On Wed, Jan 06, 2010 at 02:27:15PM +0100, Richard Guenther wrote:
> > On Tue, Jan 5, 2010 at 5:39 PM, torbenh <torbenh@gmx.de> wrote:
> > > On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
> > >> On Tue, Jan 5, 2010 at 4:03 PM, torbenh <torbenh@gmx.de> wrote:
> > >> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> > >> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
> > >> >>
> > >> >> The -fno-alias-X things do not make much sense for user code (they
> > >> >> have been historically used from Frontends).  If restrict doesn't work
> > >> >> for you (do you have a testcase that can reproduce your issue?)
> > >> >> then you probably need to wait for IPA pointer analysis to be
> > >> >> fixed in GCC 4.6.
> > >> >
> > >> > sorry... forget the attachment :S
> > >>
> > >> Yes, in this case you can fix it by making ramp static.  Otherwise its
> > >> address may be takein in another translation unit.  For Fortran we
> > >> have the DECL_RESTRICTED_P which we could expose to other
> > >> languages via an attribute.  It tells that a decl is not aliased by
> > >> restrict qualified pointers, so
> > >>
> > >> struct Ramp {
> > >>     float phase;
> > >>     inline float process() { return phase++; }
> > >> } ramp __attribute__((restrict));
> > >>
> > >> void fill_buffer( float * __restrict buf, size_t nframes )
> > >> {
> > >>     for( size_t i=0; i<nframes; i++ )
> > >>         buf[i] = ramp.process();
> > >> }
> > >
> > > would that also work with this stuff:
> > >
> > >
> > > template<typename ... Args>
> > > class Mixer;
> > >
> > > template<typename T1, typename ... Args>
> > > class Mixer<T1, Args...> : public Block
> > > {
> > >    private:
> > >        T1 t1 __attribute__((restrict));
> > >        Mixer<Args...> t2;
> > >    public:
> > >        inline float process() {
> > >            return t1.process() + t2.process();
> > >        }
> > > };
> > >
> > > template<typename T1, typename T2>
> > > class Mixer<T1,T2> : public Block
> > > {
> > >    private:
> > >        T1 t1 __attribute__((restrict));
> > >        T2 t2 __attribute__((restrict));
> > >    public:
> > >        inline float process() {
> > >            return t1.process() + t2.process();
> > >        }
> > > };
> > >
> > > Mixer<Ramp,Ramp,Ramp,Ramp> mix __attribute__((restrict))
> 
> void fill_buffer( float * __restrict buf, size_t nframes )
> {
>     for( size_t i=0; i<nframes; i++ )
>         buf[i] = mix.process();
> }
> 
> there is your pointer :)
> 
> > >
> > > ?
> > 
> > I don't see a restrict qualified pointer here.  Note that the
> > restrict attribute would only disambiguate against those.
> > Also I think you need the restrict attribute on the Mixer
> > objects, not its members.
> 
> so the attribute would promote down to all member vars of 
> member objects ?
> 
> 
> > > i still dont understand whats the problem with -fnolias,
> > > as in attached patch.
> > 
> > The patch will miscompile everything.
> 
> point taken. 
> obviously reading code for a few hours without knowing enough about the
> code isnt enough :)
> 
> __attribute__((restrict)) is the better solution.
> although not portable to other compilers. 
> 
> but i need this kind of functionality now, to test my concepts. 
> thats why i am spending a bit time on this. 
> 
> when do you plan to add this feature ?
> since you know the code, there would be no point for me to tackle
> it if you do it soonish. 
> 
> (and you dont need to deal with dumb patches from me :)
> 
> 
> speaking of dumb patches:
> 
> would you care to comment this patch for gcc-4.4 ?
> this one seems to work for simple examples.

meh... i always forget attachments :(
> 
> -- 
> torben Hohn

-- 
torben Hohn

[-- Attachment #2: noalias.patch --]
[-- Type: text/x-diff, Size: 1232 bytes --]

diff --git a/gcc/common.opt b/gcc/common.opt
index 023d773..e02db8a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -790,6 +790,10 @@ fnon-call-exceptions
 Common Report Var(flag_non_call_exceptions) Optimization
 Support synchronous non-call exceptions
 
+fnoalias
+Common Report Var(flag_noalias) Optimization
+Assume no aliasing is happening
+
 fomit-frame-pointer
 Common Report Var(flag_omit_frame_pointer) Optimization
 When possible do not generate stack frames
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 4dd4fb7..35b6a99 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -2430,8 +2430,9 @@ compute_flow_insensitive_aliasing (struct alias_info *ai)
 
       FOR_EACH_REFERENCED_VAR (var, rvi)
 	{
-	  if (var_ann (var)->is_heapvar)
-	    add_may_alias (tag, var);
+	  if (!flag_noalias)
+	    if (var_ann (var)->is_heapvar)
+	      add_may_alias (tag, var);
 	}
     }
 
@@ -2949,6 +2950,9 @@ may_alias_p (tree ptr, alias_set_type mem_alias_set,
       return false;
     }
 
+  if (flag_noalias)
+    return false;
+
   /* If -fargument-noalias-global is > 2, pointer arguments may
      not point to anything else.  */
   if (flag_argument_noalias > 2 && TREE_CODE (ptr) == PARM_DECL)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-06 15:45             ` Richard Guenther
@ 2010-01-06 15:49               ` Richard Guenther
  2010-01-06 16:48                 ` torbenh
  2010-01-06 17:37               ` torbenh
  1 sibling, 1 reply; 15+ messages in thread
From: Richard Guenther @ 2010-01-06 15:49 UTC (permalink / raw)
  To: gcc

On Wed, Jan 6, 2010 at 4:45 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Jan 6, 2010 at 4:25 PM, torbenh <torbenh@gmx.de> wrote:
>> On Wed, Jan 06, 2010 at 02:27:15PM +0100, Richard Guenther wrote:
>>> On Tue, Jan 5, 2010 at 5:39 PM, torbenh <torbenh@gmx.de> wrote:
>>> > On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
>>> >> On Tue, Jan 5, 2010 at 4:03 PM, torbenh <torbenh@gmx.de> wrote:
>>> >> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
>>> >> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
>>> >> >>
>>> >> >> The -fno-alias-X things do not make much sense for user code (they
>>> >> >> have been historically used from Frontends).  If restrict doesn't work
>>> >> >> for you (do you have a testcase that can reproduce your issue?)
>>> >> >> then you probably need to wait for IPA pointer analysis to be
>>> >> >> fixed in GCC 4.6.
>>> >> >
>>> >> > sorry... forget the attachment :S
>>> >>
>>> >> Yes, in this case you can fix it by making ramp static.  Otherwise its
>>> >> address may be takein in another translation unit.  For Fortran we
>>> >> have the DECL_RESTRICTED_P which we could expose to other
>>> >> languages via an attribute.  It tells that a decl is not aliased by
>>> >> restrict qualified pointers, so
>>> >>
>>> >> struct Ramp {
>>> >>     float phase;
>>> >>     inline float process() { return phase++; }
>>> >> } ramp __attribute__((restrict));
>>> >>
>>> >> void fill_buffer( float * __restrict buf, size_t nframes )
>>> >> {
>>> >>     for( size_t i=0; i<nframes; i++ )
>>> >>         buf[i] = ramp.process();
>>> >> }
>>> >
>>> > would that also work with this stuff:
>>> >
>>> >
>>> > template<typename ... Args>
>>> > class Mixer;
>>> >
>>> > template<typename T1, typename ... Args>
>>> > class Mixer<T1, Args...> : public Block
>>> > {
>>> >    private:
>>> >        T1 t1 __attribute__((restrict));
>>> >        Mixer<Args...> t2;
>>> >    public:
>>> >        inline float process() {
>>> >            return t1.process() + t2.process();
>>> >        }
>>> > };
>>> >
>>> > template<typename T1, typename T2>
>>> > class Mixer<T1,T2> : public Block
>>> > {
>>> >    private:
>>> >        T1 t1 __attribute__((restrict));
>>> >        T2 t2 __attribute__((restrict));
>>> >    public:
>>> >        inline float process() {
>>> >            return t1.process() + t2.process();
>>> >        }
>>> > };
>>> >
>>> > Mixer<Ramp,Ramp,Ramp,Ramp> mix __attribute__((restrict))
>>
>> void fill_buffer( float * __restrict buf, size_t nframes )
>> {
>>     for( size_t i=0; i<nframes; i++ )
>>         buf[i] = mix.process();
>> }

Btw, the same effect can be obtained by instead writing

void fill_buffer( float * __restrict buf, size_t nframes )
{
     Mixer<Ramp,Ramp,Ramp,Ramp> * restrict mixp = &mix;
     for( size_t i=0; i<nframes; i++ )
         buf[i] = mix->process();
}

at least in theory (if it doesn't work its worth to try to fix it).

Richard.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-06 15:45             ` torbenh
@ 2010-01-06 15:51               ` Richard Guenther
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Guenther @ 2010-01-06 15:51 UTC (permalink / raw)
  To: gcc

On Wed, Jan 6, 2010 at 4:45 PM, torbenh <torbenh@gmx.de> wrote:
> On Wed, Jan 06, 2010 at 04:25:59PM +0100, torbenh wrote:
>> On Wed, Jan 06, 2010 at 02:27:15PM +0100, Richard Guenther wrote:
>> > On Tue, Jan 5, 2010 at 5:39 PM, torbenh <torbenh@gmx.de> wrote:
>> > > On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
>> > >> On Tue, Jan 5, 2010 at 4:03 PM, torbenh <torbenh@gmx.de> wrote:
>> > >> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
>> > >> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh <torbenh@gmx.de> wrote:
>> > >> >>
>> > >> >> The -fno-alias-X things do not make much sense for user code (they
>> > >> >> have been historically used from Frontends).  If restrict doesn't work
>> > >> >> for you (do you have a testcase that can reproduce your issue?)
>> > >> >> then you probably need to wait for IPA pointer analysis to be
>> > >> >> fixed in GCC 4.6.
>> > >> >
>> > >> > sorry... forget the attachment :S
>> > >>
>> > >> Yes, in this case you can fix it by making ramp static.  Otherwise its
>> > >> address may be takein in another translation unit.  For Fortran we
>> > >> have the DECL_RESTRICTED_P which we could expose to other
>> > >> languages via an attribute.  It tells that a decl is not aliased by
>> > >> restrict qualified pointers, so
>> > >>
>> > >> struct Ramp {
>> > >>     float phase;
>> > >>     inline float process() { return phase++; }
>> > >> } ramp __attribute__((restrict));
>> > >>
>> > >> void fill_buffer( float * __restrict buf, size_t nframes )
>> > >> {
>> > >>     for( size_t i=0; i<nframes; i++ )
>> > >>         buf[i] = ramp.process();
>> > >> }
>> > >
>> > > would that also work with this stuff:
>> > >
>> > >
>> > > template<typename ... Args>
>> > > class Mixer;
>> > >
>> > > template<typename T1, typename ... Args>
>> > > class Mixer<T1, Args...> : public Block
>> > > {
>> > >    private:
>> > >        T1 t1 __attribute__((restrict));
>> > >        Mixer<Args...> t2;
>> > >    public:
>> > >        inline float process() {
>> > >            return t1.process() + t2.process();
>> > >        }
>> > > };
>> > >
>> > > template<typename T1, typename T2>
>> > > class Mixer<T1,T2> : public Block
>> > > {
>> > >    private:
>> > >        T1 t1 __attribute__((restrict));
>> > >        T2 t2 __attribute__((restrict));
>> > >    public:
>> > >        inline float process() {
>> > >            return t1.process() + t2.process();
>> > >        }
>> > > };
>> > >
>> > > Mixer<Ramp,Ramp,Ramp,Ramp> mix __attribute__((restrict))
>>
>> void fill_buffer( float * __restrict buf, size_t nframes )
>> {
>>     for( size_t i=0; i<nframes; i++ )
>>         buf[i] = mix.process();
>> }
>>
>> there is your pointer :)
>>
>> > >
>> > > ?
>> >
>> > I don't see a restrict qualified pointer here.  Note that the
>> > restrict attribute would only disambiguate against those.
>> > Also I think you need the restrict attribute on the Mixer
>> > objects, not its members.
>>
>> so the attribute would promote down to all member vars of
>> member objects ?
>>
>>
>> > > i still dont understand whats the problem with -fnolias,
>> > > as in attached patch.
>> >
>> > The patch will miscompile everything.
>>
>> point taken.
>> obviously reading code for a few hours without knowing enough about the
>> code isnt enough :)
>>
>> __attribute__((restrict)) is the better solution.
>> although not portable to other compilers.
>>
>> but i need this kind of functionality now, to test my concepts.
>> thats why i am spending a bit time on this.
>>
>> when do you plan to add this feature ?
>> since you know the code, there would be no point for me to tackle
>> it if you do it soonish.
>>
>> (and you dont need to deal with dumb patches from me :)
>>
>>
>> speaking of dumb patches:
>>
>> would you care to comment this patch for gcc-4.4 ?
>> this one seems to work for simple examples.
>
> meh... i always forget attachments :(

Well, it's equally broken, so you'll get very interesting effects from such
a patch.

Richard.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-06 15:49               ` Richard Guenther
@ 2010-01-06 16:48                 ` torbenh
  0 siblings, 0 replies; 15+ messages in thread
From: torbenh @ 2010-01-06 16:48 UTC (permalink / raw)
  To: gcc

On Wed, Jan 06, 2010 at 04:49:29PM +0100, Richard Guenther wrote:
> On Wed, Jan 6, 2010 at 4:45 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
> > On Wed, Jan 6, 2010 at 4:25 PM, torbenh <torbenh@gmx.de> wrote:
> >>> >
> >>> > Mixer<Ramp,Ramp,Ramp,Ramp> mix __attribute__((restrict))
> >>
> >> void fill_buffer( float * __restrict buf, size_t nframes )
> >> {
> >>     for( size_t i=0; i<nframes; i++ )
> >>         buf[i] = mix.process();
> >> }
> 
> Btw, the same effect can be obtained by instead writing
> 
> void fill_buffer( float * __restrict buf, size_t nframes )
> {
>      Mixer<Ramp,Ramp,Ramp,Ramp> * restrict mixp = &mix;
>      for( size_t i=0; i<nframes; i++ )
>          buf[i] = mix->process();
> }
> 
> at least in theory (if it doesn't work its worth to try to fix it).

in gcc-4.4 it doesnt work. didnt test with trunk yet.
but the text here suggests, that it should never work.
from gcc/alias.c get_alias_set (tree t):

  else if (AGGREGATE_TYPE_P (pointed_to_type))
    /* For an aggregate, we must treat the restricted
       pointer the same as an ordinary pointer.  If we
       were to make the type pointed to by the
       restricted pointer a subset of the pointed-to
       type, then we would believe that other subsets
       of the pointed-to type (such as fields of that
       type) do not conflict with the type pointed to
       by the restricted pointer.  */
    DECL_POINTER_ALIAS_SET (decl)
      = pointed_to_alias_set;

> 
> Richard.

-- 
torben Hohn

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: adding -fnoalias ... would a patch be accepted ?
  2010-01-06 15:45             ` Richard Guenther
  2010-01-06 15:49               ` Richard Guenther
@ 2010-01-06 17:37               ` torbenh
  1 sibling, 0 replies; 15+ messages in thread
From: torbenh @ 2010-01-06 17:37 UTC (permalink / raw)
  To: gcc

On Wed, Jan 06, 2010 at 04:45:06PM +0100, Richard Guenther wrote:
> >> I don't see a restrict qualified pointer here.  Note that the
> >> restrict attribute would only disambiguate against those.
> >> Also I think you need the restrict attribute on the Mixer
> >> objects, not its members.
> >
> > so the attribute would promote down to all member vars of
> > member objects ?
> 
> Yes.  In fact the example above would work I guess.

confirmed. works nicely.
although it would be nice, if this would also
apply to FIELD_DECL.

i am not sure if this is really possible.
is the structure hierarchy still available at that layer ?

thanks for your help so far.




> 
> >
> >> > i still dont understand whats the problem with -fnolias,
> >> > as in attached patch.
> >>
> >> The patch will miscompile everything.
> >
> > point taken.
> > obviously reading code for a few hours without knowing enough about the
> > code isnt enough :)
> >
> > __attribute__((restrict)) is the better solution.
> > although not portable to other compilers.
> >
> > but i need this kind of functionality now, to test my concepts.
> > thats why i am spending a bit time on this.
> >
> > when do you plan to add this feature ?
> 
> For GCC 4.6 earliest.

hmm... bah :)


-- 
torben Hohn

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2010-01-06 17:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-05 13:40 adding -fnoalias ... would a patch be accepted ? torbenh
2010-01-05 13:46 ` Richard Guenther
2010-01-05 15:02   ` torbenh
2010-01-05 16:42     ` Tim Prince
2010-01-05 15:03   ` torbenh
2010-01-05 15:27     ` Richard Guenther
2010-01-05 16:39       ` torbenh
2010-01-06 13:27         ` Richard Guenther
2010-01-06 15:26           ` torbenh
2010-01-06 15:45             ` Richard Guenther
2010-01-06 15:49               ` Richard Guenther
2010-01-06 16:48                 ` torbenh
2010-01-06 17:37               ` torbenh
2010-01-06 15:45             ` torbenh
2010-01-06 15:51               ` Richard Guenther

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).