[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