adding -fnoalias ... would a patch be accepted ?
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
Re: adding -fnoalias ... would a patch be accepted ?
On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote: > On Tue, Jan 5, 2010 at 2:40 PM, torbenh 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
Re: adding -fnoalias ... would a patch be accepted ?
On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> On Tue, Jan 5, 2010 at 2:40 PM, torbenh 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
#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
Re: adding -fnoalias ... would a patch be accepted ?
On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
> On Tue, Jan 5, 2010 at 4:03 PM, torbenh wrote:
> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh 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 buf[i] = ramp.process();
> }
would that also work with this stuff:
template
class Mixer;
template
class Mixer : public Block
{
private:
T1 t1 __attribute__((restrict));
Mixer t2;
public:
inline float process() {
return t1.process() + t2.process();
}
};
template
class Mixer : public Block
{
private:
T1 t1 __attribute__((restrict));
T2 t2 __attribute__((restrict));
public:
inline float process() {
return t1.process() + t2.process();
}
};
Mixer 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
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;
Re: adding -fnoalias ... would a patch be accepted ?
On Wed, Jan 06, 2010 at 02:27:15PM +0100, Richard Guenther wrote:
> On Tue, Jan 5, 2010 at 5:39 PM, torbenh wrote:
> > On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
> >> On Tue, Jan 5, 2010 at 4:03 PM, torbenh wrote:
> >> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> >> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh 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 >> buf[i] = ramp.process();
> >> }
> >
> > would that also work with this stuff:
> >
> >
> > template
> > class Mixer;
> >
> > template
> > class Mixer : public Block
> > {
> > private:
> > T1 t1 __attribute__((restrict));
> > Mixer t2;
> > public:
> > inline float process() {
> > return t1.process() + t2.process();
> > }
> > };
> >
> > template
> > class Mixer : public Block
> > {
> > private:
> > T1 t1 __attribute__((restrict));
> > T2 t2 __attribute__((restrict));
> > public:
> > inline float process() {
> > return t1.process() + t2.process();
> > }
> > };
> >
> > Mixer mix __attribute__((restrict))
void fill_buffer( float * __restrict buf, size_t nframes )
{
for( size_t i=0; i >
> > ?
>
> 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
Re: adding -fnoalias ... would a patch be accepted ?
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 wrote:
> > > On Tue, Jan 05, 2010 at 04:27:33PM +0100, Richard Guenther wrote:
> > >> On Tue, Jan 5, 2010 at 4:03 PM, torbenh wrote:
> > >> > On Tue, Jan 05, 2010 at 02:46:30PM +0100, Richard Guenther wrote:
> > >> >> On Tue, Jan 5, 2010 at 2:40 PM, torbenh 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 > >> buf[i] = ramp.process();
> > >> }
> > >
> > > would that also work with this stuff:
> > >
> > >
> > > template
> > > class Mixer;
> > >
> > > template
> > > class Mixer : public Block
> > > {
> > > private:
> > > T1 t1 __attribute__((restrict));
> > > Mixer t2;
> > > public:
> > > inline float process() {
> > > return t1.process() + t2.process();
> > > }
> > > };
> > >
> > > template
> > > class Mixer : public Block
> > > {
> > > private:
> > > T1 t1 __attribute__((restrict));
> > > T2 t2 __attribute__((restrict));
> > > public:
> > > inline float process() {
> > > return t1.process() + t2.process();
> > > }
> > > };
> > >
> > > Mixer mix __attribute__((restrict))
>
> void fill_buffer( float * __restrict buf, size_t nframes )
> {
> for( size_t i=0; 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
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)
Re: adding -fnoalias ... would a patch be accepted ?
On Wed, Jan 06, 2010 at 04:49:29PM +0100, Richard Guenther wrote:
> On Wed, Jan 6, 2010 at 4:45 PM, Richard Guenther
> wrote:
> > On Wed, Jan 6, 2010 at 4:25 PM, torbenh wrote:
> >>> >
> >>> > Mixer mix __attribute__((restrict))
> >>
> >> void fill_buffer( float * __restrict buf, size_t nframes )
> >> {
> >> for( size_t i=0; 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 * restrict mixp = &mix;
> for( size_t i=0; 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
Re: adding -fnoalias ... would a patch be accepted ?
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
some infinite recursion in gen_lsm_tmp_name()
hi...
i was seeing an infinite recursion in gen_lsm_tmp_name() in trunk.
it only happened with my code, when my __attribute__((restrict)) patch
is applied.
i dont really know how to reproduce it without it.
but the relevant place doesnt look like its supposed to be a
fallthrough.
see attached patch.
--
torben Hohn
diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index 1cc5659..3d2cf9b 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -1778,6 +1778,7 @@ gen_lsm_tmp_name (tree ref)
name = "F";
lsm_tmp_name_add ("_");
lsm_tmp_name_add (name);
+ break;
case ARRAY_REF:
gen_lsm_tmp_name (TREE_OPERAND (ref, 0));
