This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 12/02/13 08:47, Yufeng Zhang wrote:
Ping~ http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03360.html
First a note, I need to check on voting for Bill as the slsr maintainer from the steering committee. Voting was in progress just before the close of stage1 development so I haven't tallied the results :-)Thanks, Yufeng On 11/26/13 15:02, Yufeng Zhang wrote:On 11/26/13 12:45, Richard Biener wrote:On Thu, Nov 14, 2013 at 12:25 AM, Yufeng Zhang<Yufeng.Zhang@arm.com> wrote:On 11/13/13 20:54, Bill Schmidt wrote:The second version of your original patch is ok with me with the following changes. Sorry for the little side adventure into the next-interp logic; in the end that's going to hurt more than it helps in this case. Thanks for having a look at it, anyway. Thanks also for cleaning up this version to be less intrusive to common interfaces; I appreciate it.Thanks a lot for the review. I've attached an updated patch with the suggested changes incorporated. For the next-interp adventure, I was quite happy to do the experiment; it's a good chance of gaining insight into the pass. Many thanks for your prompt replies and patience in guiding!Everything else looks OK to me. Please ask Richard for final approval, as I'm not a maintainer.
Without some sense of how expensive the lookups are vs how often the cache hits it's awful hard to know if the cache is worth it.Yes, you are right about the non-trivial 'base' tree are rarely shared. The cached is introduced mainly because get_alternative_base () may be called twice on the same 'base' tree, once in the find_basis_for_candidate () for look-up and the other time in alloc_cand_and_find_basis () for record_potential_basis (). I'm happy to leave out the cache if you think the benefit is trivial.
I'd say take it out unless you have some sense it's really saving time. It's a pretty minor implementation detail either way.
I think adding some details to the dump and scanning for them would be better. That's the only change that is required for this to move forward.+/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-slsr" } */ + +typedef int arr_2[50][50]; + +void foo (arr_2 a2, int v1) +{ + int i, j; + + i = v1 + 5; + j = i; + a2 [i-10] [j] = 2; + a2 [i] [j++] = i; + a2 [i+20] [j++] = i; + a2 [i-3] [i-1] += 1; + return; +} + +/* { dg-final { scan-tree-dump-times "MEM" 5 "slsr" } } */ +/* { dg-final { cleanup-tree-dump "slsr" } } */ scanning for 5 MEMs looks non-sensical. What transform do you expect? I see other slsr testcases do similar non-sensical checking which is bad, too.As the slsr optimizes CAND_REF candidates by simply lowering them to MEM_REF from e.g. ARRAY_REF, I think scanning for the number of MEM_REFs is an effective check. Alternatively, I can add a follow-up patch to add some dumping facility in replace_ref () to print out the replacing actions when -fdump-tree-slsr-details is on.
I suggest doing it quickly. We're well past stage1 close at this point. jeff
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |