[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