On 30 March 2011 14:41, Richard Guenther <richard.guent...@gmail.com> wrote: > On Wed, Mar 30, 2011 at 2:22 PM, Ira Rosen <ira.ro...@linaro.org> wrote: >> On 30 March 2011 12:59, Richard Guenther <richard.guent...@gmail.com> wrote: >>> On Wed, Mar 30, 2011 at 11:13 AM, Ira Rosen <ira.ro...@linaro.org> wrote: >>>> Hi, >>>> >>>> With this patch a data-ref is marked as unconditionally read or >>>> written also if its adjacent field is read or written unconditionally >>>> in the loop. >>>> My concern is that this is not safe enough, even though the fields >>>> have to be non-pointers and non-aggregates, and this optimization is >>>> applied only with -ftree-loop-if-convert-stores flag. >>>> >>>> Bootstrapped on powerpc64-suse-linux and tested on x86_64-suse-linux. >>>> >>>> OK for trunk? >>> >>> The restrictions do not make too much sense to me. For the C++ >>> memory model we can't do speculative stores at all, but for the >>> rest I'd say just checking if the data-refs access the same object >>> is enough, thus, instead of same_data_refs (a, b) simply check >>> operand_equal_p (DR_BASE_ADDRESS (a), DR_BASE_ADDRESS (b), 0) >>> or operand_equal_p (get_base_address (DR_REF (a)), get_base_address >>> (DR_REF (b)), 0), whatever makes more sense (I always confuse what >>> the contents of the various DR fields are). >> >> But what about >> >> int a[10], b[100], c[100]; >> for (i = 0; i < 100; i++) >> { >> if (i < 10) >> t = a[i]; >> else >> t = b[i]; >> >> c[i] = t + a[1]; >> } >> >> We can't transform this to >> >> int a[10], b[100], c[100]; >> for (i = 0; i < 100; i++) >> { >> t1 = a[i]; >> t2 = b[i]; >> t = (i < 10) ? t1 : t2; >> c[i] = t + a[1]; >> } >> >> since we create accesses to a[10:100], right? > > That's correct - we may only ever create "valid" accesses, but any > valid access to a is ok after we see an unconditional access to a. > > So we probably have to restrict ourselves to DECL_P (get_base_address ()) > objects and make sure we don't access past it. > > Thus, I see what you were trying to do - may I suggest sth like > > ref_base_a = DR_REF (a); > while (TREE_CODE (ref_base_a) == COMPONENT_REF > || TREE_CODE (ref_base_a) == IMAGPART_EXPR > || TREE_CODE (ref_base_a) == REALPART_EXPR) > ref_base_a = TREE_OPERAND (ref_base_a, 0); > > ... same for DR_REF (b) ... > > if (operand_equal_p (ref_base_a, ref_base_b, 0)) > ok > > that is, strip all non-variable offset handled_component_refs and compare > the remaining base of the two accesses. If they are equal we are ok. > > Any hole in that?
I don't see any :) I'll test your version. Thanks, Ira > > Thanks, > Richard. > >> Thanks, >> Ira >> >>> >>> Thanks, >>> Richard. >>> >>>> Thanks, >>>> Ira >>>> >>>> >>>> ChangeLog: >>>> >>>> * tree-if-conv.c (memrefs_read_or_written_unconditionally): Return >>>> true >>>> if an adjacent field of the data-ref is accessed unconditionally. >>>> >>>> testsuite/ChangeLog: >>>> >>>> * gcc.dg/vect/if-cvt-stores-vect-ifcvt-18.c: New test. >>>> * gcc.dg/vect/vect.exp: Run if-cvt-stores-vect* tests with >>>> -ftree-loop-if-convert-stores. >>>> >>> >> >