[Beignet] [PATCH] GBE: fix printf class static variable bug

[Pan, Xiuli]

Yes, but the problem is that if two thread has kernel with printf functions,
the map<CallInst *, PrintfSet::PrintfFmt*> printfs will be cleared in construction
and destructor. This will cause the one who is still need info in printfs get null pointer.
thread_local now is to protect printfs from other thread but not pass data from one
pass to another. 

I tried the getAnalysis, but what we need the printfs as you said is in GenWriter and
It could not be used there as easier as now.
Every passmanger will clear the printfs and need a new map to avoid disturb each
other. It is just ok when only one pass exist, but in multithread there will be a lot of
passes in the same time. If using interface like: 
 *PrintfParser::getPrintfInfo() { return &printfs; }
It still a static one, and threads may have disturb.


-----Original Message-----
From: Song, Ruiling 
Sent: Friday, November 6, 2015 9:54 AM
To: Pan, Xiuli <xiuli.pan at intel.com>; Pan, Xiuli <xiuli.pan at intel.com>; beignet at lists.freedesktop.org
Subject: RE: [Beignet] [PATCH] GBE: fix printf class static variable bug

Thread_local is not needed to pass data from one llvm pass to another.
You can still access the info after pass that has already run.
In a later llvm pass, you can use getAnalysis<PrintfParser>() to get the the PrintfParser pass handle.

Then expose an interface in PrintfParser 
then you can query the printfInfo in GenWriter.

Thanks!
Ruiling
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Pan, Xiuli
> Sent: Friday, November 6, 2015 9:44 AM
> To: Pan, Xiuli; beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] GBE: fix printf class static variable bug
> 
> Ping for review.
> 
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Pan Xiuli
> Sent: Tuesday, November 3, 2015 11:30 AM
> To: beignet at lists.freedesktop.org
> Cc: Pan, Xiuli <xiuli.pan at intel.com>
> Subject: [Beignet] [PATCH] GBE: fix printf class static variable bug
> 
> The PrintfParse::printfs static is not thread
> safe and maybe reset or adding something wrong
> when runing in mutlithread. Fix the problem by
> change the printfs to a thread local variable.
> 
> Signed-off-by: Pan Xiuli <xiuli.pan at intel.com>
> ---
>  backend/src/llvm/llvm_printf_parser.cpp | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/backend/src/llvm/llvm_printf_parser.cpp
> b/backend/src/llvm/llvm_printf_parser.cpp
> index bdaed8a..93f87ea 100644
> --- a/backend/src/llvm/llvm_printf_parser.cpp
> +++ b/backend/src/llvm/llvm_printf_parser.cpp
> @@ -45,6 +45,8 @@ namespace gbe
>  {
>    using namespace ir;
> 
> +  thread_local map<CallInst*, PrintfSet::PrintfFmt*> printfs;
> +
>    /* Return the conversion_specifier if succeed, -1 if failed. */
>    static char __parse_printf_state(char *begin, char *end, char** rend,
> PrintfState * state)
>    {
> @@ -301,7 +303,6 @@ error:
>      Value* g1Xg2Xg3;
>      Value* wg_offset;
>      int out_buf_sizeof_offset;
> -    static map<CallInst*, PrintfSet::PrintfFmt*> printfs;
>      int printf_num;
>      int totalSizeofSize;
> 
> @@ -972,12 +973,10 @@ error:
>      return false;
>    }
> 
> -  map<CallInst*, PrintfSet::PrintfFmt*> PrintfParser::printfs;
> -
>    void* getPrintfInfo(CallInst* inst)
>    {
> -    if (PrintfParser::printfs[inst])
> -      return (void*)PrintfParser::printfs[inst];
> +    if (printfs[inst])
> +      return (void*)printfs[inst];
>      return NULL;
>    }
> 
> --
> 2.1.4
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet