[SandboxIR] Implement AllocaInst #102027
[SandboxIR] Implement AllocaInst #102027
Conversation
vporpo
requested review from
slackito,
tschuett,
aeubanks and
Sterling-Augustine
llvm/lib/SandboxIR/SandboxIR.cpp
Outdated
| auto &Tracker = Ctx.getTracker(); | ||
| if (Tracker.isTracking()) | ||
| Tracker.track(std::make_unique<AllocaSetAllocatedType>( | ||
| cast<AllocaInst>(this), Tracker)); |
There was a problem hiding this comment.
I may be blind, but why is this not a AllocaInst?
There was a problem hiding this comment.
Good catch, yes, this is redundant.
| Use getOperandUseInternal(unsigned OpIdx, bool Verify) const final { | ||
| return getOperandUseDefault(OpIdx, Verify); | ||
| } | ||
| SmallVector<llvm::Instruction *, 1> getLLVMInstrs() const final { |
There was a problem hiding this comment.
perhaps we should have a helper SingleLLVMInstructionImpl helper class for these that you can inherit from
There was a problem hiding this comment.
Yeah that's a good point, let me give it a try.
There was a problem hiding this comment.
I think that would also help with a couple more functions that repeat in a similar way, like getOpreandUseInteral(), getUseOperandNo() and getNumOfIRInstrs().
I will upload a separate patch for this.
| } | ||
|
|
||
| public: | ||
| AllocaInst(llvm::AllocaInst *AI, Context &Ctx) |
There was a problem hiding this comment.
I thought this was supposed to be private
There was a problem hiding this comment.
Good catch, yes this should be private.
| } | ||
| /// Return true if this alloca is used as an inalloca argument to a call. Such | ||
| /// allocas are never considered static even if they are in the entry block. | ||
| bool isUsedWithInAlloca() const { |
There was a problem hiding this comment.
personally I wouldn't add inalloca/swifterror methods unless somebody specifically requests those, but I guess up to you.
swifterror is extremely swift-specific, and inalloca is extremely windows-specific
There was a problem hiding this comment.
I recall having to add some special casing for inalloca in the scheduler's DAG, because of some instruction ordering issue related to it. So I would rather keep it.
Yeah I can drop the swifterror one.
This patch implements sandboxir::AllocaInst which mirrors llvm::AllocaInst.
This patch implements sandboxir::AllocaInst which mirrors llvm::AllocaInst.