StanfordAHA / Halide-to-Hardware

Other
74 stars 12 forks source link

Incorrect generated CoreIR code issue #139

Open wyanzhao opened 1 year ago

wyanzhao commented 1 year ago

Hi, I am trying to write a simple CoreIR program. But I found in some cases, the tool will produce incorrect CoreIR code. Here's the given Halide C++ code:

class ExamplePipeline : public Halide::Generator<ExamplePipeline> {
public:
    Input<Buffer<uint64_t>> input {"input",1};
    Output<Buffer<uint64_t>> output {"output", 1};

    void generate() {
        // specify algorithm
        create_algorithm();

        // specify schedule
        if(get_target().has_feature(Target::CoreIR)) {
            schedule_coreir();
        } else if(get_target().has_feature(Target::HLS)) {
            schedule_hls();
        } else {
            schedule_cpu();
        }
    }

    /*
     * Specify the algorithm
     */
    void create_algorithm() {
        Expr constant(((uint64_t)(274877906943)));

         hw_output(x) =  input(x) & constant;
         output(x) = hw_output(x);
        // output.bound(c, 0, 3);
    }

This program supposed to clear the 39th and 40th bit of Input. However, when I type make coreir, in the generated coreIR.cpp, the code will be:

   // produce output
  for (int _output_s0_x = _17; _output_s0_x < _17 + _18; _output_s0_x++)
  {
   int32_t _112 = _output_s0_x - _11;
   uint64_t _113 = ((const uint64_t *)_input)[_112];
   uint64_t _114 = (uint64_t)(63);
   uint64_t _115 = _113 & _114;
   int32_t _116 = _output_s0_x - _17;
   ((uint64_t *)_output)[_116] = _115;
  } // for _output_s0_x
 } // if _37

Where the constant expr will become 63, instead of the original 274877906943. And this will produce incorrect results.

jeffsetter commented 1 year ago

Can you try using a uint16_t constant with a smaller value as a test? Most of the test cases are based on 16 bit numbers.

wyanzhao commented 1 year ago

I tried uint16_t and uint32_t, both of them are fine. Only in uint64_t, some special values can trigger this bug. I can give some groups of test data, when the constant is 2199023255551, 8796093022207, 4398046511103. After codegen, the value will turn to 511, 2047, and 1023. Have no idea about this issue.

jeffsetter commented 1 year ago

Yea, so the coreir generation uses the top 32 bits right now instead of the full 64 bits.