Skip to content

Commit

Permalink
Try to reduce code size when using Lpspi futures
Browse files Browse the repository at this point in the history 
A handful of inlines, code movements, and structural changes to reduce
code size due to Lpspi fututures. I'm only measuring using the
rtic_spi_blocking example and today's stable toolchain.

Here's the before (using futures, without this commit):

    cargo bloat --example=rtic_spi_blocking --target=thumbv7em-none-eabihf --features=board/imxrt1010evk,board/spi --release --full-fn --filter="imxrt_hal?"

    File .text   Size      Crate Name
    1.0%  8.0%   912B imxrt_hal? <imxrt_hal::common::lpspi::Lpspi<P,_> as embedded_hal::blocking::spi::Transfer<u8>>::transfer::h14f349cc5cffb732
    0.4%  3.6%   416B imxrt_hal? <imxrt_hal::common::lpspi::Lpspi<P,_> as embedded_hal::blocking::spi::Write<u16>>::write::h7197ec21e2742661
    0.4%  3.3%   372B imxrt_hal? <imxrt_hal::common::lpspi::Lpspi<P,_> as embedded_hal::blocking::spi::Write<u32>>::write::h7a95cfc351eae3bd
    0.4%  3.1%   352B imxrt_hal? <imxrt_hal::common::lpspi::Lpspi<P,_> as embedded_hal::blocking::spi::Write<u8>>::write::h97a4d00b9d61362e
    0.3%  2.3%   260B imxrt_hal? <imxrt_hal::common::lpspi::LpspiError as core::fmt::Debug>::fmt::h159b83c70fb11bb8
    0.2%  1.3%   152B  imxrt_hal imxrt_hal::common::lpspi::Lpspi<P,_>::spin_start_transaction::{{closure}}::h851006df67b00efe
    0.1%  1.1%   120B  imxrt_hal imxrt_hal::common::lpspi::Lpspi<P,_>::recover_from_error::hcb7d1a99cf628a19
    0.1%  0.7%    84B  imxrt_hal imxrt_hal::chip::imxrt10xx::ccm::clock_gate::Locator::set::he922d425fd153971
    0.1%  0.6%    74B  imxrt_hal <imxrt_hal::common::lpspi::TransmitBuffer<W> as imxrt_hal::common::lpspi::TransmitData>::next_word::h89742fedea75aa39
    0.1%  0.5%    56B  imxrt_hal imxrt_hal::common::lpspi::Lpspi<P,_>::spin_for_fifo_space::{{closure}}::hb88c57be8c5c038a
    0.0%  0.3%    34B  imxrt_hal <imxrt_hal::common::lpspi::TransmitBuffer<W> as imxrt_hal::common::lpspi::TransmitData>::next_word::{{closure}}::h2ade022f73d53893
    0.0%  0.3%    32B  imxrt_hal imxrt_hal::common::lpspi::word_count::h369e015c99262d71
    0.0%  0.2%    22B  imxrt_hal <imxrt_hal::common::lpspi::TransmitBuffer<W> as imxrt_hal::common::lpspi::TransmitData>::next_word::{{closure}}::h5fee28c4feefa0c4
    0.0%  0.1%     8B  imxrt_hal imxrt_hal::chip::imxrt10xx::ccm::wait_handshake::h65a4921464b13727
    0.0%  0.0%     2B        std core::ptr::drop_in_place<imxrt_hal::common::lpspi::LpspiError>::ha1d57218f9d3130e
    0.0%  0.0%     0B            And 0 smaller methods. Use -n N to show more.
    3.0% 25.4% 2.8KiB            filtered data size, the file size is 93.3KiB

And here's the after (using futures, with this commit):

    cargo bloat --example=rtic_spi_blocking --target=thumbv7em-none-eabihf --features=board/imxrt1010evk,board/spi --release --full-fn --filter="imxrt_hal?"

    File .text   Size      Crate Name
    0.7%  6.3%   664B imxrt_hal? <imxrt_hal::common::lpspi::Lpspi<P,_> as embedded_hal::blocking::spi::Transfer<u8>>::transfer::h14f349cc5cffb732
    0.3%  2.5%   260B imxrt_hal? <imxrt_hal::common::lpspi::Lpspi<P,_> as embedded_hal::blocking::spi::Write<u16>>::write::h7197ec21e2742661
    0.3%  2.5%   260B imxrt_hal? <imxrt_hal::common::lpspi::LpspiError as core::fmt::Debug>::fmt::h159b83c70fb11bb8
    0.2%  1.7%   176B imxrt_hal? <imxrt_hal::common::lpspi::Lpspi<P,_> as embedded_hal::blocking::spi::Write<u32>>::write::h7a95cfc351eae3bd
    0.2%  1.7%   176B imxrt_hal? <imxrt_hal::common::lpspi::Lpspi<P,_> as embedded_hal::blocking::spi::Write<u8>>::write::h97a4d00b9d61362e
    0.1%  1.1%   120B  imxrt_hal imxrt_hal::common::lpspi::Lpspi<P,_>::recover_from_error::hcb7d1a99cf628a19
    0.1%  0.8%    84B  imxrt_hal imxrt_hal::chip::imxrt10xx::ccm::clock_gate::Locator::set::he922d425fd153971
    0.1%  0.8%    80B  imxrt_hal imxrt_hal::common::lpspi::Lpspi<P,_>::enqueue_transaction::hbbccb7c33a84537a
    0.1%  0.7%    74B  imxrt_hal <imxrt_hal::common::lpspi::TransmitBuffer<W> as imxrt_hal::common::lpspi::TransmitData>::next_word::h89742fedea75aa39
    0.1%  0.5%    56B  imxrt_hal imxrt_hal::common::lpspi::Lpspi<P,_>::spin_for_fifo_space::{{closure}}::hb88c57be8c5c038a
    0.0%  0.3%    36B  imxrt_hal imxrt_hal::common::lpspi::Lpspi<P,_>::wait_for_transmit_fifo_space::h5850db6571c1671b
    0.0%  0.3%    34B  imxrt_hal <imxrt_hal::common::lpspi::TransmitBuffer<W> as imxrt_hal::common::lpspi::TransmitData>::next_word::{{closure}}::h2ade022f73d53893
    0.0%  0.2%    22B  imxrt_hal <imxrt_hal::common::lpspi::TransmitBuffer<W> as imxrt_hal::common::lpspi::TransmitData>::next_word::{{closure}}::h5fee28c4feefa0c4
    0.0%  0.1%     8B  imxrt_hal imxrt_hal::chip::imxrt10xx::ccm::wait_handshake::h65a4921464b13727
    0.0%  0.0%     2B        std core::ptr::drop_in_place<imxrt_hal::common::lpspi::LpspiError>::ha1d57218f9d3130e
    0.0%  0.0%     0B            And 0 smaller methods. Use -n N to show more.
    2.2% 19.4% 2.0KiB            filtered data size, the file size is 92.4KiB

If I switch from full LTO to thin local LTO and reveal the whole build
with `cargo bloat`, I see that TryJoin::poll() is 460 bytes large. If
TryJoin::poll is inlined within the transfer function, that helps
explain why it's so big.

Note that I've been using the checked-in configuration for the release
profile.
  • Loading branch information
@mciantyre
mciantyre committed May 15, 2024
1 parent c3f170b commit 746d763
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 52 deletions.
4 changes: 0 additions & 4 deletions src/chip/dma.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,6 @@ unsafe impl<P, const N: u8> peripheral::Destination<u32> for lpspi::Lpspi<P, N>
unsafe impl<P, const N: u8> peripheral::Bidirectional<u32> for lpspi::Lpspi<P, N> {}

impl<P, const N: u8> lpspi::Lpspi<P, N> {
fn wait_for_transmit_fifo_space(&self) -> Result<(), lpspi::LpspiError> {
crate::spin_on(self.spin_for_fifo_space())
}

/// Use a DMA channel to write data to the LPSPI peripheral.
///
/// The future completes when all data in `buffer` has been written to the
Expand Down
72 changes: 24 additions & 48 deletions src/common/lpspi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,10 @@ impl<P, const N: u8> Lpspi<P, N> {
.await
}

pub(crate) fn wait_for_transmit_fifo_space(&self) -> Result<(), LpspiError> {
crate::spin_on(self.spin_for_fifo_space())
}

/// Wait for receive data in a (concurrent) spin loop.
///
/// This future does not care about the RX FIFO watermark. Instead, it
Expand All @@ -653,19 +657,6 @@ impl<P, const N: u8> Lpspi<P, N> {
.await
}

/// Start a spinning transaction.
///
/// This shouldn't run concurrently with any of the other spinning tasks.
/// Instead, it should be executed before the I/O futures.
async fn spin_start_transaction(
&mut self,
transaction: &Transaction,
) -> Result<(), LpspiError> {
self.spin_for_fifo_space().await?;
self.enqueue_transaction(transaction);
Ok(())
}

/// Send `len` LPSPI words (u32s) out of the peripheral.
///
/// Expected to run in a (concurrent) spin loop, possibly with
Expand Down Expand Up @@ -766,15 +757,22 @@ impl<P, const N: u8> Lpspi<P, N> {
let mut transaction = Transaction::new_words(data)?;
transaction.bit_order = self.bit_order();

crate::spin_on(async {
self.spin_start_transaction(&transaction).await?;
self.spin_exchange_no_start(data).await?;
Ok(())
})
self.wait_for_transmit_fifo_space()?;
self.enqueue_transaction(&transaction);

let word_count = word_count(data);
let (tx, rx) = transfer_in_place(data);

crate::spin_on(futures::future::try_join(
self.spin_transmit(tx, word_count),
self.spin_receive(rx, word_count),
))
.map_err(|err| {
self.recover_from_error();
err
})
})?;

Ok(())
}

fn write_no_read<W: Word>(&mut self, data: &[W]) -> Result<(), LpspiError> {
Expand All @@ -786,12 +784,13 @@ impl<P, const N: u8> Lpspi<P, N> {
transaction.receive_data_mask = true;
transaction.bit_order = self.bit_order();

crate::spin_on(async {
self.spin_start_transaction(&transaction).await?;
self.spin_write_no_start(data).await?;
Ok(())
})
.map_err(|err| {
self.wait_for_transmit_fifo_space()?;
self.enqueue_transaction(&transaction);

let word_count = word_count(data);
let tx = TransmitBuffer::new(data);

crate::spin_on(self.spin_transmit(tx, word_count)).map_err(|err| {
self.recover_from_error();
err
})
Expand Down Expand Up @@ -912,29 +911,6 @@ impl<P, const N: u8> Lpspi<P, N> {
set_watermark(&self.lpspi, direction, watermark)
}

/// Perform a spinning write, assuming that the transaction
/// has already started.
///
/// This implementation requires that the transaction's receive mask
/// is enabled.
async fn spin_write_no_start<W: Word>(&mut self, words: &[W]) -> Result<(), LpspiError> {
let word_count = word_count(words);
self.spin_transmit(TransmitBuffer::new(words), word_count)
.await?;
Ok(())
}

/// Exhange data within a buffer, assuming the transaction has started.
async fn spin_exchange_no_start<W: Word>(&mut self, words: &mut [W]) -> Result<(), LpspiError> {
let word_count = word_count(words);
let (tx, rx) = transfer_in_place(words);
futures::try_join!(
self.spin_transmit(tx, word_count),
self.spin_receive(rx, word_count),
)?;
Ok(())
}

/// Recover from a transaction error.
fn recover_from_error(&mut self) {
// Resets the peripheral and flushes whatever is in the FIFOs.
Expand Down

0 comments on commit 746d763

Please sign in to comment.