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

consistent TFloat prototypes #2

Merged
merged 1 commit into from
Jul 13, 2021
Merged

Conversation

GerHobbelt
Copy link

Tfloat patch 1 - TFloat used consistently in prototypes tesseract-ocr#3492

Just saw these this morning; anyway: question is: do we want the various specific implementations to have TFloat in their function definition (correctly matching the typedef) or do we want the compiler to match up float <-> TFloat and double <-> TFloat instead?

(if you decide on the latter direction, that would be using the C++ compiler and removing the FAST_TFLOAT #ifdef/#else conditions in the AVX/SSE codes, but it would also mean slightly more work for the compiler, so slightly longer compile times. 🤔 Just thinking out loud and wondering what your preferred coding style for these beasts would be, in the end.)

@stweil
Copy link
Owner

stweil commented Jul 13, 2021

I prefer using float and double explicitly in the implementations (instead of TFloat) because that's required if we want to have Tesseract binaries which support both variants at the same time. As long as the decision is made at build time, the #ifdefs should stay in the source files to minimize the generated code.

@GerHobbelt
Copy link
Author

Okay, that would nil pullreq #5.

Question: what do you feel about having that stuff (same code for float and double) done as C++ template instantiations? (one for float, one for double). While I'm not a big fan of templates, this would be a good use IMO as it would take a single written codebase and have the compiler do the work of duplication, thus killing the risk of subtle bugs which occur when someone fixes one version, but is in a bit of a hurry and misses to fix the other equivalent code as well? --> that would mean #5 would be rewritten with a bit of templating on top to accomplish this: write once, have two.

Copy link
Owner

@stweil stweil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@stweil stweil merged commit 3b8f329 into stweil:tfloat Jul 13, 2021
@stweil
Copy link
Owner

stweil commented Jul 13, 2021

I reverted this commit in my branch because I prefer to use double explicitly.

@GerHobbelt
Copy link
Author

👍 got it.

GerHobbelt added a commit to GerHobbelt/tesseract that referenced this pull request Jul 13, 2021
…ntations: `unittest/tfloat_benchmark.cc`

- working towards float+double co-existence as desired in stweil#2 (comment) using function templates for DRY as per query in stweil#2 (comment)
- fix typo mistake in OpenMP code. (Probably me earlier this morning, too hurried.)
GerHobbelt added a commit to GerHobbelt/tesseract that referenced this pull request Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants