ISLE: Allow emitting safepoint insns #3723
Conversation
Change the implementation of emitted_insts in IsleContext from a plain vector of instructions into a vector of tuples, where the second element is a boolean that indicates whether this instruction should be emitted as a safepoint. This allows targets to emit safepoint insns via ISLE.
| { | ||
| pub lower_ctx: &'a mut C, | ||
| pub flags: &'a F, | ||
| pub isa_flags: &'a I, | ||
| pub emitted_insts: SmallVec<[C::I; N]>, | ||
| pub emitted_insts: SmallVec<[(C::I, bool); N]>, |
There was a problem hiding this comment.
This probably increases memory usage by a considerable amount due to padding. Maybe we could instead have an is_safepoint method on instructions?
There was a problem hiding this comment.
This vector tends to be small, it only holds the machine instructions emitted for a single CLIF instruction. So I don't think memory usage is really any concern here ...
github-actions
bot
added
cranelift
Subscribe to Label Action
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64", "isle"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
| self.emitted_insts | ||
| .push((MInst::MovN { rd, imm, size }, false)); | ||
| } else { | ||
| let imm = MoveWideConst::maybe_with_shift(imm16 as u16, i * 16).unwrap(); | ||
| self.emitted_insts.push(MInst::MovZ { rd, imm, size }); | ||
| self.emitted_insts | ||
| .push((MInst::MovZ { rd, imm, size }, false)); | ||
| } | ||
| } else { | ||
| let imm = MoveWideConst::maybe_with_shift(imm16 as u16, i * 16).unwrap(); | ||
| self.emitted_insts.push(MInst::MovK { rd, imm, size }); | ||
| self.emitted_insts | ||
| .push((MInst::MovK { rd, imm, size }, false)); |
There was a problem hiding this comment.
Could we use self.emit(..) here to hide the bool tuple entry, which is noise/distracting/raises irrelevant questions for new code readers who stumble upon this?
There was a problem hiding this comment.
Ah I see this already merged, I'll send a follow up PR.
Change the implementation of emitted_insts in IsleContext from
a plain vector of instructions into a vector of tuples, where
the second element is a boolean that indicates whether this
instruction should be emitted as a safepoint.
This allows targets to emit safepoint insns via ISLE.
@cfallin @fitzgen