[Openmp-commits] [PATCH] D146642: [OpenMP] Implement task record and replay mechanism

[Chenle YU via Phabricator via Openmp-commits]

yuchenle marked 3 inline comments as done.
yuchenle added a comment.

In D146642#4229088 <https://reviews.llvm.org/D146642#4229088>, @josemonsalve2 wrote:

> In D146642#4229078 <https://reviews.llvm.org/D146642#4229078>, @tianshilei1992 wrote:
>
>> In D146642#4229069 <https://reviews.llvm.org/D146642#4229069>, @josemonsalve2 wrote:
>>
>>> In D146642#4229067 <https://reviews.llvm.org/D146642#4229067>, @tianshilei1992 wrote:
>>>
>>>> In D146642#4229048 <https://reviews.llvm.org/D146642#4229048>, @josemonsalve2 wrote:
>>>>
>>>>> In D146642#4228996 <https://reviews.llvm.org/D146642#4228996>, @tianshilei1992 wrote:
>>>>>
>>>>>> I think it's better to guard the entire related code with macro.
>>>>>
>>>>> What do you mean by this Shilei? To disable this feature?
>>>>
>>>> Like a opt-in feature.
>>>
>>> Why do you think so? What's the harm of leaving it enable as this is just an API function? Are you saying this to save space in the structs?
>>
>> It's not just an API function. It contains many runtime checks which can potentially compromise the performance for users that don't need the feature. I'd prefer to take it similar to OMPT.
>
> Got it. It can produce a performance degradation in regular tasks, even when not used. I think that's a fair idea.
>
> @yuchenle have you measure the overhead of this in regular tasks when no recording is used? This is specially important for you guys that use fine grain tasking

Sorry for the delay. I was trying to generate some data. Hopefully, I will write some scripts to automate this process in the future, so that anyone (including me) can test this patch's performance impact with ease.
I ran Heat propagation simulation (https://github.com/yuchenle/tdg-benchs/tree/master/heat) on an exclusive node of Marenostrum 4 with different granularities and numbers of threads. So far, according to the results (https://www.dropbox.com/s/jur1qrftmw2epvk/LLVM%20RR%20Perf.xlsx?dl=0) the performance impact is small to unnoticeable.
Though @Munesanz (Adrian Munera) and I agreed on including this patch within a macro to exclude performance concerns.
I will update the patch : )


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146642/new/

https://reviews.llvm.org/D146642