[PATCH] Add show and merge tools for sample PGO profiles.

Diego Novillo dnovillo at google.com
Fri Oct 31 17:53:01 PDT 2014


On 10/31/14 17:43, Justin Bogner wrote:
> Diego Novillo <dnovillo at google.com> writes:
>
> +    for (CallTargetMap::const_iterator I = Other.getCallTargets().begin(),
> +                                       E = Other.getCallTargets().end();
> +         I != E; ++I)
> I guess you can use range-for here. If not, I'd probably just use "auto"
> for the iterator type.

One day, I'll get used to this idiom. Done.

> +  /// \brief Profile writer factory. Create a new writer based on the value of
> +  /// \p Format.
> +  static std::error_code create(StringRef Filename,
> +                                std::unique_ptr<SampleProfileWriter> &Result,
> +                                SampleProfileFormat Format);
> This create idiom that passes in a "unique_ptr<> &" is really weird, but
> I see you got it from code that I wrote, so I won't make you clean it up
> right now. I'll likely follow up soon with something that makes these
> return an ErrorOr<unique_ptr> or something like that instead.

I used it precisely because I saw it used in the instrumentation 
profiler code. I'll change it in a subsequent patch.

>   
> +int showSampleProfile(std::string Filename, bool ShowCounts,
> +                      bool ShowAllFunctions, std::string ShowFunction,
> +                      raw_fd_ostream &OS) {
> +  using namespace sampleprof;
> +  std::unique_ptr<SampleProfileReader> Reader;
> +  if (std::error_code EC =
> +          SampleProfileReader::create(Filename, Reader, getGlobalContext()))
> +    exitWithError(EC.message(), Filename);
> +
> +  Reader->read();
> +  if (!ShowFunction.empty())
> +    Reader->dumpFunctionProfile(ShowFunction, OS);
> +  else
> +    Reader->dump(OS);
> I guess this should be:
>
>    if (ShowAllFunctions || ShowFunction.empty())
>      Reader->dump(OS);
>    else
>      Reader->dumpFunctionProfile(ShowFunction, OS);
>
> Otherwise the "-function argument ignored" below is lying.

Oops. Done.


Thanks. Diego.



More information about the llvm-commits mailing list