This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH] Cancel bswap opt when intermediate stmts are reused
- From: "Thomas Preud'homme" <thomas dot preudhomme at arm dot com>
- To: "'Richard Biener'" <richard dot guenther at gmail dot com>
- Cc: "GCC Patches" <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 18 Nov 2014 16:33:48 -0000
- Subject: RE: [PATCH] Cancel bswap opt when intermediate stmts are reused
- Authentication-results: sourceware.org; auth=none
- References: <002401cfb22e$1d26de40$57749ac0$ at arm dot com> <CAFiYyc2Yn52Z0mB__gsXWYjwg4wBVqfDygHaKxnJaa6mTYeLVQ at mail dot gmail dot com>
> From: Richard Biener [mailto:richard.guenther@gmail.com]
> Sent: Monday, November 17, 2014 12:47 PM
>
> Hmm. I am a little bit concerned about the malloc traffic generated here.
> So why not use a vec<usedtree>, get rid of the ->next pointer and
> use a hash_map <gimple, unsigned> to associate the stmt with
> an index into the vector?
Sure, it even makes things easier. However I don't understand why a vector is
better than malloc. Is it a matter of fragmentation?
>
> At this point I'd rather leave DCE to DCE ...
I thought since all information is there why not do it. It makes it easier to
read the dump of the pass.
>
> Ick - do we really have to use gotos here? Can you refactor this
> a bit to avoid it?
Yes I can. I needed the same kind of thing for fixing PR63761 (r217409) and
found a way to avoid it.
>
> The whole scheme wouldn't exactly fly with the idea of eventually
> using a lattice to propagate the symbolic numbers, but well...
>
> I think the overall idea is sound and if you have time to adjust according
> to my comments that would be nice.
To be honest I think it should wait for next stage1. After several ping I took
another look at the testcase with the idea of measuring the size reduction
the patch could give and was surprised that in all cases I could construct the
size was actually bigger. Performance might be improved nonetheless but
I think this needs more careful consideration.
And as you said the approach would need to be changed if bswap was
rewritten to do a forward analysis. At last, nobody reported a code size or
performance regression so far due to the changes so that might be a non
issue. If such a report happens, then it will be a good time to revisit that
decision.
Do you agree?
>
> Sorry for the very late review.
That's alright, I know how it is. Thank you for keeping track of it. I actually
feel sorry I didn't warn about my findings. I thought the patch fell through
the cracks and didn't want to spam gcc-patches uselessly.
Best regards,
Thomas