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

TypeInferenceError with parametrized structs #1523

Closed
mtdudek opened this issue Jul 30, 2024 · 1 comment
Closed

TypeInferenceError with parametrized structs #1523

mtdudek opened this issue Jul 30, 2024 · 1 comment
Assignees
Labels
bug Something isn't working or is incorrect dslx DSLX (domain specific language) implementation / front-end
Milestone

Comments

@mtdudek
Copy link
Contributor

mtdudek commented Jul 30, 2024

Describe the bug
When parametrized function returns parametrized struct as an function value,
parser/type checker fails with TypeInferenceError.

0037: #[quickcheck]
0038: fn prop_double_reverse(x: bits[WeightPreScanInternalDataSize(u32:8)]) -> bool {
0039:   x == InternalStructToBits<u32:8>(BitsToInternalStruct<u32:8>(x))
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^--------^ TypeInferenceError: WeightPreScanInternalData { occurance_count: uN[COUNTER_WIDTH][8], valid_weigths: uN[1][12], weights_count: uN[COUNTER_WIDTH][12] } Instantiated return type did not have all parametrics resolved.
0040: }

Struct has inferred parameters that aren't resolved during InstantiateParametricFunction.

Code that triggers this bug:

import std;

const MAX_WEIGTH = u32:11;
const WEIGTH_LOG = std::clog2(MAX_WEIGTH);
const MAX_SYMBOL_COUNT = u32:256;

struct WeightPreScanInternalData <
    PARALLEL_ACCESS_WIDTH: u32,
    COUNTER_WIDTH: u32 = {std::clog2(PARALLEL_ACCESS_WIDTH + u32:1)}
> {
    occurance_count: uN[COUNTER_WIDTH][PARALLEL_ACCESS_WIDTH],
    valid_weigths:   u1[MAX_WEIGTH + u32:1],
    weights_count:   uN[COUNTER_WIDTH][MAX_WEIGTH + u32:1],
}

fn WeightPreScanInternalDataSize(PARALLEL_ACCESS_WIDTH: u32) -> u32 {
    let COUNTER_WIDTH = {std::clog2(PARALLEL_ACCESS_WIDTH + u32:1)};
    (COUNTER_WIDTH as u32) * (PARALLEL_ACCESS_WIDTH as u32) +
    (MAX_WEIGTH as u32) + u32:1 +
    (COUNTER_WIDTH as u32) * (MAX_WEIGTH as u32 + u32:1)
}

fn InternalStructToBits<
    PARALLEL_ACCESS_WIDTH: u32,
    BITS: u32 = {WeightPreScanInternalDataSize(PARALLEL_ACCESS_WIDTH)}
> (internalStruct: WeightPreScanInternalData<PARALLEL_ACCESS_WIDTH>) -> bits[BITS] {
    internalStruct as bits[BITS]
}

fn BitsToInternalStruct<
    PARALLEL_ACCESS_WIDTH: u32,
    BITS: u32 = {WeightPreScanInternalDataSize(PARALLEL_ACCESS_WIDTH)}
> (rawBits: bits[BITS]) -> WeightPreScanInternalData<PARALLEL_ACCESS_WIDTH> {
    rawBits as WeightPreScanInternalData<PARALLEL_ACCESS_WIDTH>
}

#[quickcheck]
fn prop_double_reverse(x: bits[WeightPreScanInternalDataSize(u32:8)]) -> bool {
  x == InternalStructToBits<u32:8>(BitsToInternalStruct<u32:8>(x))
}

//#[quickcheck]
//fn prop_double_reverse(x: WeightPreScanInternalData<u32:8>) -> bool {
//  x == BitsToInternalStruct<u32:8>(InternalStructToBits<u32:8>(x))
//}

Switching order of function calls results

#[quickcheck]
fn prop_double_reverse(x: WeightPreScanInternalData<u32:8>) -> bool {
  x == BitsToInternalStruct<u32:8>(InternalStructToBits<u32:8>(x))
}

in the following error:

E0730 22:10:37.766863 2828959 parametric_bind.cc:112] INTERNAL: XLS_RET_CHECK failure (xls/dslx/type_system/parametric_bind.cc:112) !arg_dim.IsParametric() COUNTER_WIDTH
0x5617ebeb0a6a: xabsl::StatusBuilder::CreateStatusAndConditionallyLog()
0x5617e9ee2719: xabsl::StatusBuilder::operator absl::lts_20240116::Status()
0x5617e9fe4228: xls::dslx::ParametricBindTypeDim()
0x5617e9fe4a8b: xls::dslx::ParametricBind()
0x5617e9fe4d85: xls::dslx::ParametricBind()
0x5617e9fe4eab: xls::dslx::ParametricBind()
0x5617e9fdc9a6: xls::dslx::internal::ParametricInstantiator::InstantiateOneArg()
0x5617e9fdd486: xls::dslx::internal::FunctionInstantiator::Instantiate()
0x5617e9fda883: xls::dslx::InstantiateFunction()
0x5617e9f79f15: xls::dslx::InstantiateParametricFunction()
0x5617e9f70474: xls::dslx::TypecheckInvocation()
0x5617e9f66b78: std::__1::__function::__func<>::operator()()
0x5617e9fc8d1e: xls::dslx::DeduceInstantiation()
0x5617e9fc9cde: xls::dslx::DeduceInvocation()
0x5617e9fa6a8b: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleInvocation()
0x5617ea3d686b: xls::dslx::Invocation::Accept()
0x5617e9f9ac7f: xls::dslx::Deduce()
0x5617e9f668b5: std::__1::__function::__func<>::operator()()
0x5617ea0601a8: xls::dslx::DeduceCtx::Deduce()
0x5617e9f9b1f2: xls::dslx::DeduceAndResolve()
0x5617e9fc9031: xls::dslx::AppendArgsForInstantiation()
0x5617e9fc9c54: xls::dslx::DeduceInvocation()
0x5617e9fa6a8b: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleInvocation()
0x5617ea3d686b: xls::dslx::Invocation::Accept()
0x5617e9f9ac7f: xls::dslx::Deduce()
0x5617e9f668b5: std::__1::__function::__func<>::operator()()
0x5617ea0601a8: xls::dslx::DeduceCtx::Deduce()
0x5617e9f9b1f2: xls::dslx::DeduceAndResolve()
0x5617e9fc505a: xls::dslx::DeduceBinop()
0x5617e9fa117b: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleBinop()
0x5617ea3d6f7b: xls::dslx::Binop::Accept()
0x5617e9f9ac7f: xls::dslx::Deduce()
0x5617e9f668b5: std::__1::__function::__func<>::operator()()
0x5617ea0601a8: xls::dslx::DeduceCtx::Deduce()
0x5617e9f9db94: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleStatement()
0x5617ea3d789b: xls::dslx::Statement::Accept()
0x5617e9f9ac7f: xls::dslx::Deduce()
0x5617e9f668b5: std::__1::__function::__func<>::operator()()
0x5617ea0601a8: xls::dslx::DeduceCtx::Deduce()
0x5617e9fa12c8: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleStatementBlock()
0x5617ea3d70ab: xls::dslx::StatementBlock::Accept()
0x5617e9f9ac7f: xls::dslx::Deduce()
0x5617e9f668b5: std::__1::__function::__func<>::operator()()
0x5617ea0601a8: xls::dslx::DeduceCtx::Deduce()
0x5617e9f9b1f2: xls::dslx::DeduceAndResolve()
0x5617e9f6b324: xls::dslx::TypecheckFunction()
0x5617e9f63b2f: xls::dslx::TypecheckModule()
0x5617e9ef2897: xls::dslx::TypecheckModule()
0x5617e9ef1d5c: xls::dslx::ParseAndTypecheck()
0x5617e9ee7652: xls::dslx::ParseAndTest()

E0730 22:10:37.766977 2828959 command_line_utils.cc:46] Could not extract a textual position from error message: INTERNAL: XLS_RET_CHECK failure (xls/dslx/type_system/parametric_bind.cc:112) !arg_dim.IsParametric() COUNTER_WIDTH: INVALID_ARGUMENT: Provided status is not in recognized error form: INTERNAL: XLS_RET_CHECK failure (xls/dslx/type_system/parametric_bind.cc:112) !arg_dim.IsParametric() COUNTER_WIDTH

To Reproduce
Steps to reproduce the behavior:

  1. Build interpreter_main
  2. Copy attached code into file.x
  3. Run interpreter_main file.x
  4. Observe error message(s)

Expected behavior
Interpreter should correctly deduce types for parametric structs.

Environment (this can be helpful for troubleshooting):

  • OS: Debian
  • Versions 12

Additional context
Tested on the newest available main(ea5c4dc)
I've already looked into code base for function instantiation, and it doesn't check if return or argument types contain parametrized structs.
That unfortunately leads to the EagerlyPopulateParametricEnvMap missing struct inferred parameters.
I' wonder if adding extra step in the InstantiateParametricFunction (

  • iterates over function args and return type
  • checks for parametric structs
  • add their parameters to typed_parametrics
    ) would fix this issue.
@mtdudek mtdudek moved this to Bug in Antmicro XLS Jul 31, 2024
@richmckeever richmckeever self-assigned this Jul 31, 2024
@mtdudek
Copy link
Contributor Author

mtdudek commented Jul 31, 2024

I've discovered more things regarding parameters in structs:

  • non-explicit parameters that use {} syntax aren't evaluated in ConcretizeStructAnnotation,
  • there is no class that inherits from ParametricExpression that is used to represent {} statements,
  • evaluation on the parameters in the ConcretizeStructAnnotation is a bit confusing, as parameters are scanned in order, but evaluation has access to all parameters at once.

@hongted hongted added this to the DSLX Next milestone Aug 5, 2024
@proppy proppy added bug Something isn't working or is incorrect dslx DSLX (domain specific language) implementation / front-end labels Aug 19, 2024
@proppy proppy moved this to Todo in XLS usability Aug 20, 2024
@richmckeever richmckeever moved this from Todo to In Progress in XLS usability Aug 20, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in XLS usability Sep 5, 2024
@github-project-automation github-project-automation bot moved this from Bug to Done in Antmicro XLS Sep 5, 2024
copybara-service bot pushed a commit that referenced this issue Sep 6, 2024
This fixes the usage in #1523.

There are also a couple of unrelated lint fixes here.

PiperOrigin-RevId: 671909309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working or is incorrect dslx DSLX (domain specific language) implementation / front-end
Projects
Status: Done
Development

No branches or pull requests

4 participants