Per-contract output selection in CompilerStack
#15433
Conversation
cameel
force-pushed
the
compiler-stack-per-contract-output-selection
branch
2 times, most recently
from
35cb6af to
89b6684
Compare
cameel
force-pushed
the
compiler-stack-per-contract-output-selection
branch
from
89b6684 to
19dc81f
Compare
cameel
force-pushed
the
output-selection-tests
branch
from
c6cb6e8 to
3452ba0
Compare
cameel
force-pushed
the
compiler-stack-per-contract-output-selection
branch
from
19dc81f to
a1e929b
Compare
cameel
force-pushed
the
compiler-stack-per-contract-output-selection
branch
from
a1e929b to
9a06e1e
Compare
cameel
force-pushed
the
compiler-stack-per-contract-output-selection
branch
from
9a06e1e to
8e002fb
Compare
| enum class IROutputSelection { | ||
| None, | ||
| UnoptimizedOnly, | ||
| UnoptimizedAndOptimized, | ||
| /// Indicates which stages of the compilation pipeline were explicitly requested and provides | ||
| /// logic to determine which ones are effectively needed to accomplish that. | ||
| /// Note that parsing and analysis are not selectable, since they cannot be skipped. | ||
| struct PipelineSelection | ||
| { | ||
| bool irCodegen = false; ///< Want IR output straight from code generator. | ||
| bool irOptimization = false; ///< Want reparsed IR that went through YulStack. May be optimized or not, depending on settings. | ||
| bool bytecode = false; ///< Want EVM-level outputs, especially EVM assembly and bytecode. May be optimized or not, depending on settings. |
There was a problem hiding this comment.
I had a really hard time coming up with good names for this, and I'm still not fully satisfied.
Initially I went with OutputSelection, but this does not really apply to all the outputs, just those that require going into new branches of he pipeline. StandardCompiler (and Standard JSON) already have a thing called "output selection" and it's not exactly this so that naming would be confusing.
Same for the fields - e.g. bytecode could mean just generating EVM assembly so it's not perfect. I considered naming this evm or evmasm, but bytecode seems like a better overall name, even if it's not precise.
There was a problem hiding this comment.
Perhaps PipelineComponentSelection? Or PipelineStageSelection? That would at least make it clearer in terms of not selecting a whole different pipeline, just branches of it.
There was a problem hiding this comment.
It does sound a bit ambiguous, because I didn't want to explicitly call it a "stage" and because to some extent it does select a pipeline (when not compiling via IR, selecting --ir really enables the second pipeline).
"Stage" does not sound right because it implies sequentially going through stages like parsing/analysis/codegen/optimization/assembling/linking/etc. and this is not exactly what we are selecting here. CompilerStack::State/m_stopAfter covers exactly this progression and I didn't want confusion with those. What the new struct is describing is more how to put together a potentially branching franken-pipeline from parts lying around :) I would not really call these "components" though.
I decided to rename it to PipelineConfig now to remove the ambiguity, but it's still not that great IMO, because the meaning a bit too broad. Names are hard :)
There was a problem hiding this comment.
Definitely are - thanks for the elaboration! I'm fine with it in any case
| bool operator!=(PipelineSelection const& _other) const { return !(*this == _other); } | ||
| bool operator==(PipelineSelection const& _other) const | ||
| { | ||
| return | ||
| irCodegen == _other.irCodegen && | ||
| irOptimization == _other.irOptimization && | ||
| bytecode == _other.bytecode; | ||
| } |
There was a problem hiding this comment.
default comparison operations from c++20 would be really handy here :)
There was a problem hiding this comment.
Yeah, having to enumerate all the fields explicitly is an unending source of errors.
cameel
force-pushed
the
compiler-stack-per-contract-output-selection
branch
2 times, most recently
from
bcb0d05 to
6a73afe
Compare
cameel
force-pushed
the
compiler-stack-per-contract-output-selection
branch
from
6a73afe to
a9d9143
Compare
Fixes #15373.
Depends on #15447.
Turned out to be less tedious than expected, though TBH contract and output selection feels like something that shouldn't really be a part of
CompilerStackand should sit inStandardCompilerinstead.