elliottt opened PR #5132 from trevor/ssa-assertions
to main
:
- Rework the
compile
interface to require fewer args- Plumb the machine_env through the lowering context
- Plumb the MachineEnv through to the OperandCollector
- Assert that fixed allocations are to allocatable registers
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
Doesn't this depend on the calling convention in some cases? Like for the spidermonkey integration which uses to reserve some registers.
bjorn3 edited PR review comment.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
cfallin created PR review comment:
That was actually handled as an ISA flag; in principle which registers we allocate within a function body and which ABI we implement at caller and callee boundaries are orthogonal, and changing which registers we use is an optimization (so controlling this via compiler settings is adequate).
cfallin submitted PR review.
cfallin submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
Hmm, this is pretty unfortunate (forcing the need to have a
MachineEnv
in theEmitInfo
). One might not always have aMachineEnv
when emitting: specifically, for Winch, we need to be able to operate more or less without regalloc2.Can we find a way to do this without the
MachineEnv
? Maybe theOperandCollector
is parameterized on a second function, a "validator", that checks whether each operand is actually valid? Then the use of theOperandCollector
in the main lowering path can insert it, and other uses (e.g. by s390x) can insert a vacuous|_| true
validator callback.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt submitted PR review.
elliottt created PR review comment:
Fixed in #5164 by removing
count_operands
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt edited PR #5132 from trevor/ssa-assertions
to main
:
Add assertions to the
OperandCollector
that show we're not using pinned vregs.
<!--Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt updated PR #5132 from trevor/ssa-assertions
to main
.
elliottt edited PR #5132 from trevor/ssa-assertions
to main
:
Add assertions to the
OperandCollector
that show we're not using pinned vregs, and usereg_fixed_nonallocatable
constraints when a real register is used with other constraint generation functions likereg_use
etc.I experimented with adding debug assertions that the reused operand referenced in
reg_reuse_def
constraints was compatible with the new constraint that would be generated. This didn't end up being possible as some backends will recursively process instructions when collecting operands, making it impossible without more invasive changes to tell where that operand is in the operand vector. Given that we would need to plumb through somewhat substantial changes to support adding debug assertions, I decided that rephrasing the existing warning comment about real registers would be good enough.
<!--Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Last updated: Dec 23 2024 at 12:05 UTC