[PATCH] D18022: Do not specialize IRBuilder to strip names in SROA
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 11 09:11:39 PST 2016
> On Mar 11, 2016, at 12:52 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
>
> chandlerc accepted this revision.
> chandlerc added a comment.
> This revision is now accepted and ready to land.
>
> LGTM patch is fine, but also see comment below.
>
>
> ================
> Comment at: lib/Transforms/Scalar/SROA.cpp:93-104
> @@ -95,5 +92,14 @@
> +class IRBuilderPrefixedInserter : public IRBuilderDefaultInserter<true> {
> +#ifndef NDEBUG
> std::string Prefix;
> + const Twine getNameWithPrefix(const Twine &Name) const {
> + return Name.isTriviallyEmpty() ? Name : Prefix + Name;
> + }
>
> public:
> void SetNamePrefix(const Twine &P) { Prefix = P.str(); }
> +#else
> + const Twine getNameWithPrefix(const Twine &Name) const { return Name; }
> + void SetNamePrefix(const Twine &P) {}
> +#endif
>
> ----------------
> When you measured, did you measure getting rid of this complexity too?
No I didn't.
>
> If it doesn't slow SROA down when the context has the flag set, I would be all in favor of just completely removing the NDEBUG hacking here. My expectation is that the overhead of this while not zero (it uses an actual std::string) is *very* close to zero.
I have the same expectation of not seeing this on any profile. I wasn't sure it was fine adding this anyway.
I'll clean this is up and submit then.
--
Mehdi
>
> That said, if you'd rather submit as-is, I can take care of cleaning this up.
>
>
> http://reviews.llvm.org/D18022
>
>
>
More information about the llvm-commits
mailing list