Re: adding -fnoalias ... would a patch be accepted ?

[Richard Guenther <richard.guenther@gmail.com>]

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.