[SandboxIR] Implement InvokeInst #100796
Merged
[SandboxIR] Implement InvokeInst #100796
Conversation
This file contains hidden or 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
vporpo
requested review from
slackito,
echristo,
alinas,
tschuett,
tmsri and
Sterling-Augustine
tmsri
approved these changes
Jul 26, 2024
| BasicBlock *IfNormal, BasicBlock *IfException, | ||
| ArrayRef<Value *> Args, Instruction *InsertBefore, | ||
| Context &Ctx, const Twine &NameStr = ""); | ||
|
|
There was a problem hiding this comment.
Why is InvokeInstr missing a constructor with "InsertAtEnd" parameter? CallInstr has it.
There was a problem hiding this comment.
So the issue is that in other instructions we had just two create() functions, one with InsertBefore and one with InsertAtEnd. But both of these can be replaced by one create() function that takes as arguments a BasicBlock::Iterator WhereIt and a BasicBlock *WhereBB. Anyway, it's probably OK to either keep all three variants, or keep only one. I would need to revisit them with a refactoring patch. Any preference?
But anyway, I will add the missing create() function with the InsertAtEnd parameter.
There was a problem hiding this comment.
I don't have a preference but the LLVM IR has InsertPosition. Should we just create one for SandboxIR too?
There was a problem hiding this comment.
Yes, we should introduce a similar class in SandboxIR.
This patch implements sandboxir::InvokeInst which mirrors llvm::InvokeInst.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/2191 Here is the relevant piece of the build log for the reference: |
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 patch implements sandboxir::InvokeInst which mirrors llvm::InvokeInst.