Stream: cranelift

Topic: SSABuilder::declare_block does nothing

Veverak (Oct 26 2021 at 09:42):

The method cranelift_frontend::ssa::SSABuilder::declare_block is defined as:

    pub fn declare_block(&mut self, block: Block) {
        self.ssa_blocks[block] = SSABlockData {
            predecessors: PredBlockSmallVec::new(),
            sealed: false,
            undef_variables: Vec::new(),
        };
    }

The value constructed is the same as the default. Prettier than to construct this value manually would be to simply call default().

This function is only meant to be called for new blocks that haven't been recorded in self.ssa_blocks before. As ssa_blocks is a SecondaryMap, anything that hasn't been recorded before will implicitly be set to the default value, so assigning it the default value does nothing, and this entire method does nothing.

Chris Fallin (Oct 26 2021 at 16:10):

@Veverak yes, that's true in the implementation we have today; it looks like it used to be a PrimaryMap and the refactor in #1340 changed it to be a SecondaryMap. From an API-design point of view, it's useful to have the producer invoke declare_block()in case we later need to do some initialization at that step, but if you want to submit a PR that removes the body and replaces it with a comment similar to "No need to initialize ssa_blocks because SSABlockData::default() is sufficient and is returned automatically by SecondaryMap for all blocks by default", I'd be happy to review. Thanks!

Veverak (Oct 26 2021 at 16:30):

@Chris Fallin

if you want to submit a PR

Tempting, but I already deleted my Github account, and I'm reluctant to join this vendor lock-in again.

Last updated: Dec 23 2024 at 12:05 UTC