[Beignet] [PATCH] Backend: Fix one bug of printf because of ir reorder.
Zhigang Gong
zhigang.gong at linux.intel.com
Thu Feb 5 20:40:43 PST 2015
LGTM, will push latter. Thanks.
On Fri, Jan 30, 2015 at 06:31:29PM +0800, junyan.he at inbox.com wrote:
> From: Junyan He <junyan.he at linux.intel.com>
>
> The llvm will generate ir which has if.else block before
> if.then block. We parse the printf statement before llvm_to_gen.
> The later if-else analysis will reorder the if-else blocks.
> This cause when we print out the result, we get the wrong message
> from another printf statement.
> Add printf index to the index buffer to record which one the result
> belongs to, and so this bug is fixed.
>
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
> backend/src/ir/printf.cpp | 25 ++++++++++++++++++++-----
> backend/src/ir/printf.hpp | 2 +-
> backend/src/llvm/llvm_printf_parser.cpp | 19 ++++++++++++-------
> src/cl_command_queue_gen7.c | 2 +-
> 4 files changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/backend/src/ir/printf.cpp b/backend/src/ir/printf.cpp
> index fa108dc..3d9b2fd 100644
> --- a/backend/src/ir/printf.cpp
> +++ b/backend/src/ir/printf.cpp
> @@ -34,8 +34,9 @@ namespace gbe
> uint32_t PrintfSet::append(PrintfFmt* fmt, Unit& unit)
> {
> fmts.push_back(*fmt);
> + vector<PrintfSlot>& vp = fmts.back().first;
>
> - for (PrintfFmt::iterator f = fmts.back().begin(); f != fmts.back().end(); ++f) {
> + for (vector<PrintfSlot>::iterator f = vp.begin(); f != vp.end(); ++f) {
> if (f->type == PRINTF_SLOT_TYPE_STRING)
> continue;
>
> @@ -126,14 +127,27 @@ namespace gbe
> int stmt = 0;
>
> for (size_t count = 0; count < fmts.size(); ++count) {
> - PrintfFmt& pf = fmts[count];
> for (i = 0; i < global_wk_sz0; i++) {
> for (j = 0; j < global_wk_sz1; j++) {
> for (k = 0; k < global_wk_sz2; k++) {
> - int loop_num = ((int *)index_addr)[stmt*global_wk_sz0*global_wk_sz1*global_wk_sz2
> - + k*global_wk_sz0*global_wk_sz1 + j*global_wk_sz0 + i];
> + int loop_num = ((int *)index_addr)[(stmt*global_wk_sz0*global_wk_sz1*global_wk_sz2
> + + k*global_wk_sz0*global_wk_sz1 + j*global_wk_sz0 + i)*2];
> + int printf_num = ((int *)index_addr)[(stmt*global_wk_sz0*global_wk_sz1*global_wk_sz2
> + + k*global_wk_sz0*global_wk_sz1 + j*global_wk_sz0 + i)*2 + 1];
> + if (!loop_num) continue;
> +
> + PrintfFmt* ppf = NULL;
> + for (auto& f : fmts) {
> + if (f.second == printf_num) {
> + ppf = &f;
> + break;
> + }
> + }
> +
> + PrintfFmt& pf = *ppf;
> +
> for (int n = 0; n < loop_num; n++) {
> - for (PrintfFmt::iterator pfit = pf.begin(); pfit != pf.end(); ++pfit) {
> + for (vector<PrintfSlot>::iterator pfit = pf.first.begin(); pfit != pf.first.end(); ++pfit) {
> PrintfSlot& slot = *pfit;
> pf_str = "";
> int vec_num;
> @@ -146,6 +160,7 @@ namespace gbe
>
> generatePrintfFmtString(*slot.state, pf_str);
>
> +
> vec_num = slot.state->vector_n > 0 ? slot.state->vector_n : 1;
>
> for (int vec_i = 0; vec_i < vec_num; vec_i++) {
> diff --git a/backend/src/ir/printf.hpp b/backend/src/ir/printf.hpp
> index f6c6bcf..cbab759 100644
> --- a/backend/src/ir/printf.hpp
> +++ b/backend/src/ir/printf.hpp
> @@ -198,7 +198,7 @@ namespace gbe
> }
> };
>
> - typedef vector<PrintfSlot> PrintfFmt;
> + typedef std::pair<vector<PrintfSlot>, int> PrintfFmt;
> uint32_t append(PrintfFmt* fmt, Unit &unit);
>
> uint32_t getPrintfNum(void) const {
> diff --git a/backend/src/llvm/llvm_printf_parser.cpp b/backend/src/llvm/llvm_printf_parser.cpp
> index 8fec683..52da2e5 100644
> --- a/backend/src/llvm/llvm_printf_parser.cpp
> +++ b/backend/src/llvm/llvm_printf_parser.cpp
> @@ -268,7 +268,7 @@ again:
>
> if (p != begin) {
> std::string s = std::string(begin, size_t(p - begin));
> - printf_fmt->push_back(PrintfSlot(s.c_str()));
> + printf_fmt->first.push_back(PrintfSlot(s.c_str()));
> }
>
> if (p == end) // finish
> @@ -279,7 +279,7 @@ again:
> if (ret_char < 0)
> goto error;
>
> - printf_fmt->push_back(&state);
> + printf_fmt->first.push_back(&state);
> num++;
>
> if (rend == end)
> @@ -385,14 +385,14 @@ error:
>
> /////////////////////////////////////////////////////
> /* calculate index address.
> - index_addr = (index_offset + wg_offset )* sizeof(int) + index_buf_ptr
> + index_addr = (index_offset + wg_offset )* sizeof(int) * 2 + index_buf_ptr
> index_offset = global_size2 * global_size1 * global_size0 * printf_num */
>
> Value* index_offset = builder->CreateMul(g1Xg2Xg3, ConstantInt::get(intTy, printf_num));
> // index_offset + offset
> op0 = builder->CreateAdd(index_offset, wg_offset);
> - // (index_offset + offset)* sizeof(int)
> - op0 = builder->CreateMul(op0, ConstantInt::get(intTy, sizeof(int)));
> + // (index_offset + offset)* sizeof(int) * 2
> + op0 = builder->CreateMul(op0, ConstantInt::get(intTy, sizeof(int)*2));
> // Final index address = index_buf_ptr + (index_offset + offset)* sizeof(int)
> op0 = builder->CreateAdd(index_buf_ptr, op0);
> Value* index_addr = builder->CreateIntToPtr(op0, Type::getInt32PtrTy(module->getContext(), 1));
> @@ -401,9 +401,13 @@ error:
> val = builder->CreateAdd(loop_num, ConstantInt::get(intTy, 1));
> builder->CreateStore(val, index_addr);// The loop number.
>
> + op0 = builder->CreateAdd(op0, ConstantInt::get(intTy, sizeof(int)));
> + index_addr = builder->CreateIntToPtr(op0, Type::getInt32PtrTy(module->getContext(), 1));
> + builder->CreateStore(ConstantInt::get(intTy, printf_num), index_addr);// The printf number.
> +
> int i = 1;
> Value* data_addr = NULL;
> - for (auto &s : *pInfo.printf_fmt) {
> + for (auto &s : (*pInfo.printf_fmt).first) {
> if (s.type == PRINTF_SLOT_TYPE_STRING)
> continue;
>
> @@ -458,6 +462,7 @@ error:
> NULL)));
> assert(printfs[printf_inst] == NULL);
> printfs[printf_inst] = pInfo.printf_fmt;
> + printfs[printf_inst]->second = printf_num;
> printf_num++;
> return true;
> }
> @@ -499,7 +504,7 @@ error:
>
> sizeof_size = 0;
> int i = 1;
> - for (auto &s : *printf_fmt) {
> + for (auto &s : (*printf_fmt).first) {
> int sz = 0;
> if (s.type == PRINTF_SLOT_TYPE_STRING)
> continue;
> diff --git a/src/cl_command_queue_gen7.c b/src/cl_command_queue_gen7.c
> index eec39b4..253c4f2 100644
> --- a/src/cl_command_queue_gen7.c
> +++ b/src/cl_command_queue_gen7.c
> @@ -268,7 +268,7 @@ cl_bind_printf(cl_gpgpu gpgpu, cl_kernel ker, void* printf_info, int printf_num,
> int32_t offset = interp_kernel_get_curbe_offset(ker->opaque, value, 0);
> size_t buf_size = global_sz * sizeof(int) * printf_num;
> if (offset > 0) {
> - if (cl_gpgpu_set_printf_buffer(gpgpu, 0, buf_size, offset, interp_get_printf_indexbuf_bti(printf_info)) != 0)
> + if (cl_gpgpu_set_printf_buffer(gpgpu, 0, buf_size*2, offset, interp_get_printf_indexbuf_bti(printf_info)) != 0)
> return -1;
> }
>
> --
> 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