Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect generated CoreIR code issue #139

Open
wyanzhao opened this issue Aug 12, 2022 · 3 comments
Open

Incorrect generated CoreIR code issue #139

wyanzhao opened this issue Aug 12, 2022 · 3 comments

Comments

@wyanzhao
Copy link

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants