[libc-commits] [PATCH] D137871: [libc][math] Improve the performance and error printing of UInt.
Tue Ly via Phabricator via libc-commits
libc-commits at lists.llvm.org
Mon Nov 14 16:13:15 PST 2022
lntue added inline comments.
================
Comment at: libc/utils/UnitTest/LibcTest.cpp:281
+template bool test<__llvm_libc::cpp::UInt<192>>(
+ RunContext *Ctx, TestCondition Cond, __llvm_libc::cpp::UInt<192> LHS,
----------------
sivachandra wrote:
> lntue wrote:
> > sivachandra wrote:
> > > lntue wrote:
> > > > sivachandra wrote:
> > > > > Ideally, these specializations should be in a different file, say `integer_test_utils.cpp`, right next to where `UInt` is defined. May be it leads to a CMake cyclic directory problem?
> > > > Since we `UInt<128>` is required here as replacement for `__uint128_t` if not available, instantiate other `UInt` here is the most suitable, without complicating the dependency, I think.
> > > We can keep specializations of only standard types here. Everything else, including those for `__uint128_t`, should go to another file.
> > Does it also mean that by default we will need to link to the other file, in addition to `LibcTest` ?
> Yes, like other custom matchers we have.
The template `test` that is instantiated here is actually only defined internally inside `LibcTest.cpp`. Untangle and refactoring it out to a common header so that we can have a separate instantiation target for `UInt<192>` and `UInt<256>` feel like out-of-scope for this patch. It should be a separate task / patch to refactor this file instead.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137871/new/
https://reviews.llvm.org/D137871
More information about the libc-commits
mailing list