OPEN: FP integration (v2) #12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- remove extra code / spurious comments - radically simplify FloatImmediate checkValue - remove offset and n_levels from FloatAddTemplate
This is better aligned with conventions and the rest of the code.
Scheremo
reviewed
Nov 10, 2024
There was a problem hiding this comment.
Hey guys,
I appreciate the additional changes to the FloatImmediateType class, I feel it's very comprehensive now. I left a few minor open points, but modulo a passing CI, this PR seems merge-worthy to me.
EDIT: Currently typing tests are failing, but it seems due to an important of bfloat16 which is simply not implemented.
| # | ||
| # Copyright (C) 2021, ETH Zurich and University of Bologna. | ||
| # | ||
| # Author: Moritz Scherer, ETH Zurich |
There was a problem hiding this comment.
NIT: I appreciate the credit, but you might want to claim it for yourselves :)
Using float64_t, float32_t, float16_t and bfloat16_t. Add also a further include from DeeployBasicMath.h, DeeployPULPMath.h for these types.
Victor-Jung
approved these changes
Nov 13, 2024
Closed
Victor-Jung
requested changes
Nov 13, 2024
Victor-Jung
approved these changes
Nov 14, 2024
|
[heart] Francesco Conti reacted to your message:
…________________________________
From: Victor Jung ***@***.***>
Sent: Thursday, November 14, 2024 9:55:12 AM
To: pulp-platform/Deeploy ***@***.***>
Cc: Francesco Conti ***@***.***>; Author ***@***.***>
Subject: Re: [pulp-platform/Deeploy] OPEN: FP integration (v2) (PR #12)
Merged #12<#12> into devel.
—
Reply to this email directly, view it on GitHub<#12 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AB4EPOSD2QKIK4EQ5YVA62L2ARXQBAVCNFSM6AAAAABRPRMKOSVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJVGI4TKNZWGQZTSMY>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is my own take on #3 and follows up directly all the comments discussed in that PR. I do not continue that one directly as it comes from a branch in a privately owned repository.
Compared to the last version discussed there:
FloatImmediatecheckValueFloatAddTemplateLimitations:
bfloat16_tandfloat16_tdata types due to lack of compiler support in LLVM15.