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

[Xinliang David Li via llvm-commits]

yes -- it is better to  pre-create counters before lowering regardless
of value profiling is there.

David

On Fri, Jan 15, 2016 at 1:43 PM, Rong Xu <xur at google.com> wrote:
> 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
>>>
>>>
>>>