[Beignet] [PATCH 5/6] Debug Support: Pass debug infomation to GenInsn

Song, Ruiling ruiling.song at intel.com
Mon Nov 2 18:31:37 PST 2015


> 
> +  extern bool OCL_PROFILING; // first defined by calling BVAR in program.cpp
> +#define SET_GENINSN_DBGINFO(I) \
> +  if(OCL_PROFILING) { \
> +    p->DBGInfo.line = I.DBGInfo.line; \
> +    p->DBGInfo.col = I.DBGInfo.col;}
Please don't assign the fields one by one in DBGInfo, please use p->DBGInfo = l.DBGInfo.
I would like to see a unified definition of the DebugInfo class across the whole backend. Don't define it here and there.
In the current implementation, only the line/column are gathered, the "file name" is skipped, which is also very important.
We may add it later. So at that time, we need to modify the so many definition of the DebugInfo scattered across the whole backend.

> diff --git a/backend/src/backend/gen_encoder.cpp
> b/backend/src/backend/gen_encoder.cpp
> index 2cc51cc..5eaddf1 100644
> --- a/backend/src/backend/gen_encoder.cpp
> +++ b/backend/src/backend/gen_encoder.cpp
> @@ -591,11 +591,26 @@ namespace gbe
>        this->setSrc1(insn, bti);
>      }
>    }
> +
> +  extern bool OCL_PROFILING; // first defined by calling BVAR in program.cpp
> +  void GenEncoder::setDBGInfo(uint32_t line, uint32_t col, bool hasHigh)

So you'd better define the function prototype as GenEncoder::setDBGInfo (DebugInfo info, bool hasHigh)

> --- a/backend/src/backend/gen_encoder.hpp
> +++ b/backend/src/backend/gen_encoder.hpp
> @@ -88,6 +88,12 @@ namespace gbe
>      uint32_t deviceID;
>      /*! simd width for this codegen */
>      uint32_t simdWidth;
> +    struct {
> +      uint32_t line;
> +      uint32_t col;
> +    } DBGInfo;

> +    vector<GenInsnDBGInfo> storedbg;
Please don't define variables in the .hpp file. Please move it to .cpp file. If the header file is included in many source file, it would have multiple definitions.

Thanks!
Ruiling
> +    void setDBGInfo(uint32_t line, uint32_t col, bool hasHigh);
>      ////////////////////////////////////////////////////////////////////////
>      // Encoding functions
>      ////////////////////////////////////////////////////////////////////////
> --
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list