search

google / xls

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

DSLX IR converter unexpected naming convention #393

Open ted-xie opened 3 years ago

ted-xie commented 3 years ago

I have a function called fn donut_inner in a file called donut_rolled.x. I call the IR converter like so:

~/sw/xls/bazel-bin/xls/dslx/ir_converter_main donut_rolled.x --entry=donut_inner > donut_inner.ir

I would expect that the top-level function would be named fn donut_inner like how I specified on the command line, but instead it is fn __donut_rolled__donut_inner.

Is there a way to get the correct naming behavior? My githash is 14d5f6e7ba1fd553cdfc9a0d5cb772c1c3ea75cf.

cdleary commented 3 years ago

Hey Ted, thanks for filing. We currently mangle DSL names so they're unique and in a flat symbolic namespace when we convert them to IR.

Since IR conversion is a fairly low level tool and part of the overall toolchain, you might think of this as part of our ABI at the moment.

The dslx_codegen build macro allows folks to provide an output module name -- that's implemented by passing the desired module name along to the codegen tool: https://google.github.io/xls/build_system/#dslx_codegen

I think something we should have, but we don't right now, is a convenience macro (in starlark) that turns an unmangled name into a mangled one so build system users never need to know the details.

Hope that makes sense!

cdleary commented 1 year ago

Just an update here that we've had a helper in Starlark that gets the mangled IR symbol for a while:

https://github.com/google/xls/blob/main/xls/build_rules/xls_ir_rules.bzl#L446

I understand this can be surprising so it'd be nice if we could automatically try to mangle the entry function passed so we do something expected when users pass an unmangled name. It's really an artifact that a) the DSL level rides on top of the XLS IR level of functionality b) the IR level doesn't want to know anything specific about the DSL level, ideally, and c) the DSL level may want to put symbols from different modules into the same XLS IR package namespace, so some kind of mangling is compelled to happen.

I think we can loosen (b) here a bit in the interest of the overall toolchain experience being a bit more cohesive.