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

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

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.