Skip to content

ISLE: port more ops on x64 to lowering patterns.#3855

Merged
cfallin merged 1 commit intobytecodealliance:mainfrom
cfallin:isle-misc-ops
Feb 28, 2022
Merged

ISLE: port more ops on x64 to lowering patterns.#3855
cfallin merged 1 commit intobytecodealliance:mainfrom
cfallin:isle-misc-ops

Conversation

@cfallin
Copy link
Member

@cfallin cfallin commented Feb 26, 2022

Depends on / stacked on top of #3848, #3849.

@cfallin cfallin requested a review from abrown February 26, 2022 01:26
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:x64 Issues related to x64 codegen isle Related to the ISLE domain-specific language labels Feb 26, 2022
@github-actions
Copy link

Subscribe to Label Action

cc @cfallin, @fitzgen

Details 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:

  • cfallin: isle
  • fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

;; Rules for `debugtrap` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(rule (lower (debugtrap))
(side_effect (hlt)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing/curious: Do you know why debugtrap isn't a safe point? Our other traps are safe points.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually not sure! For what it's worth, hlt is a bit of an odd choice here as well (it can only execute in kernel mode, and I think generates SIGILL in userspace). I guess the idea is that it's not meant to be a "normal" transfer of control to the runtime paired with the generated code, but rather a place where a debugger could catch a signal or hook in with ptrace or something of the sort, so setting up state for a "normal" runtime invocation to e.g. scan the stack wouldn't be useful. I'm not sure though.

In the absence of more context here I'm inclined to leave the lowering as-is (translated from existing), but we should definitely ask whether this opcode is even necessary, or at least what its precise guarantees should be, when we get to cleaning up the CLIF opcode space...

@cfallin cfallin merged commit d9dfc44 into bytecodealliance:main Feb 28, 2022
@cfallin cfallin deleted the isle-misc-ops branch February 28, 2022 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants