-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[LLVM][Triple] Add an argument to specify canonical form to Triple::normalize
#122935
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm so confused by this addition
What if I ask for a 4-tuple, and the code produces a 5-tuple? Right now, i think I get a 5-tuple? It's not clear what the code should do here. This seems like an oversight in working out what you intend.
Are you sure that the items in the 4-/5-tuples are in their correct locations? I'm not sure you can just add unknown
at the end, but I could be wrong.
I'd like to see a lot more testing with real triples, not totally fake ones. That would probably answer whether the right thing is happening in the latter case with a lot more certainty.
(I'm not sure who the maintainer of this code really is, tbh, but I realise you picked me as I was the last to touch this code - as part of the TargetParser migration. This is reasonable but it means I'm coming at this reasonably "fresh").
Yes.
I'm not sure what we should do here, but my second thought would be to cut it to the asked form.
The code to expand
I can definitely do that. |
…normalize` Currently, the output of `Triple::normalize` can vary depending on how the `Triple` object is constructed, producing a 3-field, 4-field, or even 5-field string. However, there is no way to control the format of the output, as all forms are considered canonical according to the LangRef. This lack of control can be inconvenient when a specific format is required. To address this, this PR introduces an argument to specify the desired format (3, 4, or 5 identifiers), with the default set to none to maintain the current behavior. If the requested format requires more components than are available in the actual `Data`, `"unknown"` is appended as needed.
c7c5b74
to
bd285ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'm happy given the additional testing.
…normalize` (llvm#122935) Currently, the output of `Triple::normalize` can vary depending on how the `Triple` object is constructed, producing a 3-field, 4-field, or even 5-field string. However, there is no way to control the format of the output, as all forms are considered canonical according to the LangRef. This lack of control can be inconvenient when a specific format is required. To address this, this PR introduces an argument to specify the desired format (3, 4, or 5 identifiers), with the default set to none to maintain the current behavior. If the requested format requires more components than are available in the actual `Data`, `"unknown"` is appended as needed. (cherry picked from commit ebef440)
…normalize` (llvm#122935) Currently, the output of `Triple::normalize` can vary depending on how the `Triple` object is constructed, producing a 3-field, 4-field, or even 5-field string. However, there is no way to control the format of the output, as all forms are considered canonical according to the LangRef. This lack of control can be inconvenient when a specific format is required. To address this, this PR introduces an argument to specify the desired format (3, 4, or 5 identifiers), with the default set to none to maintain the current behavior. If the requested format requires more components than are available in the actual `Data`, `"unknown"` is appended as needed. (cherry picked from commit ebef440)
Currently, the output of
Triple::normalize
can vary depending on how theTriple
object is constructed, producing a 3-field, 4-field, or even 5-fieldstring. However, there is no way to control the format of the output, as all
forms are considered canonical according to the LangRef.
This lack of control can be inconvenient when a specific format is required. To
address this, this PR introduces an argument to specify the desired format (3,
4, or 5 identifiers), with the default set to none to maintain the current
behavior. If the requested format requires more components than are available in
the actual
Data
,"unknown"
is appended as needed.