google
[enhancement] DSLX Constexpr-If Semantics
What's hard to do? (limit 100 words)
This is a proposal to adapt constexpr-if semantics for parametrics in DSLX in response to the bug https://github.com/google/xls/issues/2423.
We frequently encounter this potential issue that in a parametric function, a certain statement inside a branch fails type-checking, even if that branch is not taken under this particular parametrics instantiation which causes type-checking failure.
For example, consider this code.
fn f<A: u32>(a : u32) -> u32 {
if A == u32:0 {
a
} else {
let b : u32[A] = u32[A]:[u32:1,];
for (i, acc) in u32:0..A {
b[i] + acc
} (u32:0)
}
}
For f<0>, the true branch is taken, and returns a, and the false branch, despites being untaken, fails type-checking because u32[0]:[u32:1,] is not valid. For f<A> where A!=0, the false branch compiles normally. Since it is not possible to have the code flow to actually go into the false branch where it will be type-check invalid, there is no point to fail the compilation for f<0>. This type of if-expression is prevalent in floating point libraries due to all the special cases in representing floating points.
Current best alternative workaround (limit 100 words)
There's no feasible workaround under TIv2. Note that most of the reported errors are not checked in TIv1 (such as checking constexpr array size being less than 0, or bitshift with constexpr amount greater than bit width), so in theory these checks could be disabled selectively in TIv2, but doing so will weaken type-checking, and likely introducing more bugs (hard/impossible to distinguish (proof) whether some type errors are due to certain parametrics instantiations or always happening)
Your view of the "best case XLS enhancement" (limit 100 words)
The proposed solution is inspired from the constexpr-if feature in C++17 (https://en.cppreference.com/w/cpp/language/if.html#Constexpr_if), where in a template function if the conditional expression can be evaluated as a constant at compile time for a particular instantiation, only the taken branch according to the conditional's value is compiled, and the untaken branch is discarded as long as it is not ill-formed for every possible template specialization. Similarly, we can implement this in DSLX. In a parametric function, if a conditional of an if-expression can be evaluated as a constant at compile time in a particular instantiation, then only the taken branch is type-checked and compiled to IR, while the untaken branch is discarded, even if it contains type errors under this instantiation. This process is applied separately for every instance (For each instance we should check whether the conditional is a constexpr and what it evaluates to). Both branches are still required to parse correctly by the front-end, even if one is never taken.
In the future we may expand this concept to match-expression as well. If in a parametric the match pattern can be evaluated as a constant, only the corresponding match arm is type-checked and compiled to IR, and other match arms are discarded. However, there are much fewer production code written like this, so this may not be a priority.
Would the changes be limited to the tiv2 code? If so, could you document where?
I think the C++-23 feature captures the behavior we want, but I think we should try to stick to syntax which is more Rust-like. There is an open Rust rfc for a const if construct (https://github.com/rust-lang/rfcs/issues/3582). In this case you would have something like
fn f<A: u32>(a : u32) -> u32 {
const if A == u32:0 {
a
} else {
let b : u32[A] = u32[A]:[u32:1,];
for (i, acc) in u32:0..A {
b[i] + acc
} (u32:0)
}
}
The rfc is not approved and discussion is ongoing, however, I think it is still reasonable to choose this form for DSLX today. If the final form of the Rust construct is something else we can do a migration to the new form.
Agree with @meheff that if there's a front runner in the Rust syntax, even if it's not finalized yet, we should try to do that one, because it'll likely minimize stuff we feel like we need to fix up later if it does get finalized.
General notion of having a const if sounds good to me, I think recursion will make it most useful but even before then it is useful to ensure things are truly constant-evaluated.
(And avoid warnings/errors that are effectively impossible, like our recent shift and empty range examples.)
You mentioned the change is internal, any reason it's difficult to expose as a PR? If it can be then we could offer more detailed feedback. 🙏
I support this enhancement as a general-purpose solution that solves this issue and more.
Regarding syntax, sticking with Rust syntax makes sense.
The proposed patch is at https://github.com/google/xls/pull/2413, which is still work in progress.
Big +1 to this, I've had to write some really ugly workarounds to get type checking to pass when I know only one match arm will be taken in any particular concrete instantiation. https://github.com/xlsynth/bedrock-dslx/blob/main/lib/ecc/secded_decoder.x
fn f<A: u32>(a : u32) -> u32 { if A == u32:0 { a } else { let b : u32[A] = u32[A]:[u32:1,]; for (i, acc) in u32:0..A { b[i] + acc } (u32:0) } }
One possible (ugly) workaround for getting the for expression to typecheck is to increment the range expression by 1 and do a nop (just return the accumulator) in the extra iteration:
for (i, acc) in u32:0..A+1 {
if i < A { b[i] + acc } else { acc }
} (u32:0)
