[PATCH] D16015: [PGO] Create the profile data variable before the lowering

Rong Xu via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 15 13:41:50 PST 2016


On Fri, Jan 15, 2016 at 1:33 PM, David Li <davidxl at google.com> wrote:
> davidxl added inline comments.
>
> ================
> Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:141
> @@ +140,3 @@
> +  //
> +  // Also create the profile data variable -- do it here because there is no
> +  // guarantee that a counter increment will be ahead of value profile
> ----------------
> Remove the comments here.

OK.

>
> ================
> Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:154
> @@ -144,1 +153,3 @@
>            computeNumValueSiteCounts(Ind);
> +          hasValueInstrumentation = true;
> +        }
> ----------------
> Why is this flag needed?

I want to create the profile data variable here only when there are
value instrumentations in this function.
Another reason is the assertion in line 160. It's possible that the
function is not instrumented (like a personality routine), in which
case we don't find the ProfileInc instruction.

>
> ================
> Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:161
> @@ +160,3 @@
> +      assert (FirstProfIncInst != nullptr);
> +      getOrCreateRegionCounters(FirstProfIncInst);
> +    }
> ----------------
> Add a brief comment here: Value profiling intrinsic lowering requires per-function profile data variable to be created first.

OK.

>
>
> http://reviews.llvm.org/D16015
>
>
>


More information about the llvm-commits mailing list