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

We should have helper functions that return bool Exprs that query properties of the target #8192

Open
abadams opened this issue Apr 12, 2024 · 4 comments
Labels
enhancement New user-visible features or improvements to existing features.

Comments

@abadams
Copy link
Member

abadams commented Apr 12, 2024

Portable code is best, but sometimes when you have a high-level mathematical operation you want to perform, there are multiple ways to express it, and different ways are more or less suited to the instructions available on a given target. E.g. for integer division within some certain range you may want to use bit-tricks, or you may want to do it as float, depending on the floating point throughput of your target.

To support this, currently you have to plumb the damn target though every mathematical helper function, so that you can switch on it. This leads to ugly code. It would be cool if you could instead say something like:

Expr e = select(target_arch_is(Target::ARM), something, something_else)

I propose one new helper functions in IROperator.h for each enum in Target. They would each return a boolean Expr:

Expr target_arch_is(Target::Arch);
Expr target_os_is(Target::OS);
Expr target_processor_is(Target::Processor)
Expr target_has_feature(Target::Feature);

These Exprs would be calls to similarly-named intrinsics in the IR, with the enum arg converted to a constant integer. These intrinsics would be lowered to either true or false at the earliest possible opportunity (probably alongside strictify_float at the top of lowering).

@abadams abadams added the enhancement New user-visible features or improvements to existing features. label Apr 12, 2024
@soufianekhiat
Copy link

soufianekhiat commented Apr 15, 2024

Nice to have could be, as a syntaxic sugar:

Expr device_api_is(DeviceAPI api);

@zvookin
Copy link
Member

zvookin commented Apr 17, 2024

The main use case is library implementation, though I'm tempted to say using a context argument for such things may be preferable as the configuration can be more powerful than just the target. (E.g. we need to revisit the way we specify implementations of architectures in Target as it is clearly not being elaborated to cover any of the space it needs to cover.)

It is important to clarify that this is compile time only. Passing the current target through C++, as generators do, allows controlling the construction of IR. This proposed feature allows constructing the IR with multiple paths, one of which is chosen during lowering. A further option would allow selecting based on the actual hardware at runtime, which is something that would need other changes and currently doesn't make a lot of sense, but this mechanism could be easily confused for that if one isn't already well versed in Halide's compilation model.

@steven-johnson
Copy link
Contributor

Maybe also add Expr target_natural_vector_width(Type)?

@abadams
Copy link
Member Author

abadams commented Apr 17, 2024

Yeah I was wondering about vector width too. Normally anywhere you'd write a schedule you have a Target, but it could be handy if somewhere deep in a math library with no access to a Target you need to make a compute_root memoized Func as a lut.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New user-visible features or improvements to existing features.
Projects
None yet
Development

No branches or pull requests

4 participants