[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:43:17 PST 2016
On Fri, Jan 15, 2016 at 1:41 PM, Rong Xu <xur at google.com> wrote:
> 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.
I can get rid of this variable and use FirstProfileInInst to guard the
getOrCreateRegsionCounters() call.
>
>>
>> ================
>> 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