-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[SandboxIR] Add more functions to sandboxir:Instruction class. #110050
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
The getter functions simply turn around and call the LLVM counterparts. This is fine until we don't add new sandbox IR opcodes. At that point, we may have to explicitly check if the behavior is different.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Sterling-Augustine
left a comment
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.
Pretty straight-forward pass through functions.
| //===--------------------------------------------------------------------===// | ||
|
|
||
| /// Return true if the instruction has any metadata attached to it. | ||
| bool hasMetadata() const { return cast<llvm::Instruction>(Val)->hasMetadata(); } |
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.
Not sure about whether we should give access to metadata at this point, because these are LLVM IR structures and we can't really track their state.
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.
Removed getMetadata and getAllMetadata functions.
Add them back after MDNode is available in sandboxir. For now, exposing them could give access to functions that modify the metadata.
vporpo
left a comment
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.
Looks good, thanks!
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/5788 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/5786 Here is the relevant piece of the build log for the reference |
|
This should fix it: 13809b3 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/8673 Here is the relevant piece of the build log for the reference |
…110050) The getter functions simply turn around and call the LLVM counterparts. This is fine until we don't add new sandbox IR opcodes. At that point, we may have to explicitly check if the behavior is different.
The getter functions simply turn around and call the LLVM
counterparts. This is fine until we don't add new sandbox IR
opcodes. At that point, we may have to explicitly check if
the behavior is different.