google / xls

XLS: Accelerated HW Synthesis
http://google.github.io/xls/
Apache License 2.0
1.2k stars 173 forks source link

add_with_carry function not found #1474

Closed proppy closed 2 months ago

proppy commented 3 months ago

Describe the bug

When trying to use the documented built-in add_with_carry like in the snippet below:

import float32;
import std;

pub fn double_fraction_carry(f: float32::F32) -> (uN[float32::F32_FRACTION_SZ], u1) {
    let f = f.fraction as uN[u32:23 + u32:1];
    let (c, f_x2) = add_with_carry(f, f);
    (f_x2[0+:u23], c)
}

ir_converter_main fails with the following error:

xls_work_dir/user_module.x:8:35-8:41
0006: pub fn double_fraction_carry(f: float32::F32) -> (uN[float32::F32_FRACTION_SZ], u1) {
0007:     let f = f.fraction as uN[u32:23 + u32:1];
0008:     let (c, f_x2) = add_with_carry(f, f);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^----^ IrConversionError: Could not find name for invocation: `add_with_carry`; available: [double_fraction_carry]
0009:     (f_x2[0+:u23], c)
0010: }
cdleary commented 3 months ago

If we're not going to add an IR node for it (and I don't think we have any active plans to) I think we should probably remove this in favor of stdlib userspace helper function.

cdleary commented 3 months ago

(And note this is technically a frontend issue since it's an error in IR conversion.)

dplassgit commented 2 months ago

If we're not going to add an IR node for it (and I don't think we have any active plans to) I think we should probably remove this in favor of stdlib userspace helper function.

This seems to be the consensus; we'll remove it from the FE. There is no IR for it, as Leary points out, and it's only implemented in the bytecode interpreter. We'll probably add it to stdlib at a later date. @proppy do you have a preferred format/template/already existing FR for the stdlib?

proppy commented 1 month ago

we'll remove it from the FE.

Did we also update the doc?

@proppy do you have a preferred format/template/already existing FR for the stdlib?

@dplassgit https://github.com/google/xls/issues/new?assignees=&labels=enhancement&projects=&template=enhancement-proposal.yml&title=%5Benhancement%5D+ would do also filed # to discuss how to consolidate builtins and stdlib namespace #1579