Data type error when using `hcl.select`

[chhzh123]

The hcl.select function generates <cond> ? <true_val> : <false_val> in C HLS. However, it does not guarantee the same data types of <true_val> and <false_val>, leading to possible compile-time error.

As an example, I slightly modify the code in Tutorial: Back-end Support.

import heterocl as hcl
import numpy as np

A = hcl.placeholder((10, 10), "A")
def kernel(A):
    return hcl.compute((8, 8), lambda y, x: 
        hcl.select(x < 4, A[y][x] + A[y+2][x+2], 0), "B")
s = hcl.create_scheme(A, kernel)
s = hcl.create_schedule_from_scheme(s)
f = hcl.build(s, target="vhls")
print(f)

The generated C HLS code is listed below.

#include <ap_int.h>
#include <ap_fixed.h>
#include <hls_stream.h>
#include <math.h>
#include <stdint.h>

void default_function(ap_int<32>* A, ap_int<32>* B) {
                  ap_int<32> _top;
  for (ap_int<32> y = 0; y < 8; ++y) {
    for (ap_int<32> x = 0; x < 8; ++x) {
      B[(x + (y * 8))] = ((ap_int<32>)((x < 4) ? (((ap_int<33>)A[(x + (y * 10))]) + ((ap_int<33>)A[((x + (y * 10)) + 22)])) : (ap_int<33>)0));
    }
  }
}

And I get the following error when compiling with Vivado HLS 2018.1 .

ERROR: [HLS 200-70] Compilation errors found:
Pragma processor failed: In file included from ./test.cpp:1:
./test.cpp:11:48: error: conditional expression is ambiguous; 'typename ap_int_base<33, true>::RType<33, true>::plus' (aka 'ap_int_base<plus_w, plus_s>') can be converted to 'ap_int<33>' and vice versa
      B[(x + (y * 8))] = ((ap_int<32>)((x < 4) ? (((ap_int<33>)A[(x + (y * 10))]) + ((ap_int<33>)A[((x + (y * 10)) + 22)])) : (ap_int<33>)0));

The codegen needs to explicitly cast the data types for the two branches.

[hecmay]

The codegen part for select: https://github.com/cornell-zhang/heterocl/blob/master/tvm/src/codegen/codegen_c.cc#L628-L670

[zhangzhiru]

We need to generate proper error/warning messages instead of letting the output code fails HLS.

[chhzh123]

The codegen generates two data types now. As shown below, there are two ap_int<33> before the constant zero. I'm not sure whether this will cause problems in the future.

B[(x + (y * 8))] = ((ap_int<32>)((x < 4) ? ((ap_int<33>)(((ap_int<33>)A[(x + (y * 10))]) + ((ap_int<33>)A[((x + (y * 10)) + 22)]))) : ((ap_int<33>)(ap_int<33>)0)));

Actually I think the current solution in #195 is dangerous and will easily cause potential bugs if two branches should haven't been put together. As an example,

A = hcl.placeholder((8, 8), "A", dtype=hcl.Int(20))
B = hcl.placeholder((8, 8), "B", dtype=hcl.Fixed(16,12))
def kernel(A, B):
    return hcl.compute((8, 8), lambda y, x: 
        hcl.select(x < 4, A[y][x], B[y][x]), "C", dtype=hcl.Int(8))

I received no warnings or errors and got the following code.

void default_function(ap_int<20> A[8*8], ap_fixed<16, 4> B[8*8], ap_int<8> C[8*8]) {
  ap_int<32> _top;
  for (ap_int<32> y = 0; y < 8; ++y) {
    for (ap_int<32> x = 0; x < 8; ++x) {
      C[(x + (y * 8))] = ((ap_int<8>)((x < 4) ? ((ap_fixed<32, 0>)A[(x + (y * 8))]) : ((ap_fixed<32, 0>)((ap_int<20>)B[(x + (y * 8))]))));
    }
  }
}

The data type of the B array is changed four times, ap_fixed<16,4> -> ap_int<20> -> ap_fixed<32,0> -> ap_int<8>, which involves truncation, and ap_fixed<32,0> appears strangely. We need a strong-typed DSL to avoid this kind of conversion.

[zhangzhiru]

When we fix a bug, we should concisely describe the problem and the solution. @Hecmay can you can a comment in #195?

[hecmay]

HeteroCL is not strongly typed, and automatic data type casting logic is generated when two different variables with different types are intermixed. We can generate a warning message if the two branches have different types here. The ap_fixed<32,0> was not covered in the test cases and I will fix it.

[seanlatias]

I think the problem here is that the data types are not cast correctly. When we have two branches with different types, we should safely cast them into the common data type without losing the information. For example, in your case, we should cast both ap_int<20> and ap_fixed<16, 4> to ap_fixed<32, 20>. And since our output C is ap_int<8>, we should directly cast the final results to be ap_int<8>. The truncation is explicitly specified by the users so I don't think that is a problem.

[zhangzhiru]

HeteroCL is not strongly typed, and automatic data type casting logic is generated when two different variables with different types are intermixed

It's important to note that we are dealing with two ap_int<> here. So the discussions on having a strong type system is not too relevant. C++ is strongly typed but we can easily mix the use of ap_int<> with different bit sizes as long as the corresponding operators can be overloaded to do implicit casting. In this specific case, it's really the ternary operator ?: which is not easy to overload.

[zhangzhiru]

I think the problem here is that the data types are not cast correctly. When we have two branches with different types, we should safely cast them into the common data type without losing the information.

We need a document (at least for developers) that describes how we do the implicit casts on integers in HeteroCL.

[chhzh123]

I think the problem here is that the data types are not cast correctly. When we have two branches with different types, we should safely cast them into the common data type without losing the information. For example, in your case, we should cast both ap_int<20> and ap_fixed<16, 4> to ap_fixed<32, 20>. And since our output C is ap_int<8>, we should directly cast the final results to be ap_int<8>. The truncation is explicitly specified by the users so I don't think that is a problem.

Yes, truncation is not the problem. I'm not sure if all the data types in HeteroCL can be cast arbitrarily. Maybe we can generate a warning message as what @Hecmay said.

[chhzh123]

This issue has not been thoroughly tackled in #201 . The following test

A = hcl.placeholder((8, 8), "A", dtype=hcl.UInt(1))
def kernel(A):
    return hcl.compute((8, 8), lambda y, x: 
        hcl.select(x < 4, A[y][x], 0), "B")
s = hcl.create_scheme([A], kernel)
s = hcl.create_schedule_from_scheme(s)
f = hcl.build(s, target="vhls")

fails with the error below.

error: conditional expression is ambiguous; 'ap_uint<32>' can be converted to 'unsigned int' and vice versa
      B[(x + (y * 8))] = ((ap_int<32>)((x < 4) ? (((ap_uint<32>)A[(x + (y * 8))])) : (0U)));

[hecmay]

Issue caused by CodeGenC here: https://github.com/cornell-zhang/heterocl/blob/master/tvm/src/codegen/codegen_c.cc#L448-L472 The immediate const number should always come with a data type casting.

[chhzh123]

Fixed in #209 .