Skip to content

add tu_unaligned_read32/write32#772

Merged
hathach merged 5 commits into
masterfrom
unaligned-access
Apr 6, 2021
Merged

add tu_unaligned_read32/write32#772
hathach merged 5 commits into
masterfrom
unaligned-access

Conversation

@hathach
Copy link
Copy Markdown
Owner

@hathach hathach commented Apr 6, 2021

Describe the PR
Alternative to #758 but prefer to using static inline instead of macros. Also take into account that ARMv7, v8 (M3-M7 , M23-M33) can do unaligned access natively.

@HiFiPhile @PanRe this is what I had in mind in previous discussion regarding unaligned access. Let me know what you think.

@PanRe
Copy link
Copy Markdown
Collaborator

PanRe commented Apr 6, 2021

I guess @HiFiPhile knows more of this topic but in any case, could you provide an unaligned access to int16 too? This is required for fast half word copies in the audio encoding/deconding! Thanks :)

@HiFiPhile
Copy link
Copy Markdown
Collaborator

With a quick looking it should work :)

With IAR there is a little inconvenient is with inline function I can't disable Warning[Pa039]: use of address of unaligned structure member.
So we need to continue to use (uint8_t*) desc_bos + offsetof(tusb_desc_bos_t, wTotalLength) instead of &desc_bos->wTotalLength.

@hathach
Copy link
Copy Markdown
Owner Author

hathach commented Apr 6, 2021

With a quick looking it should work :)

With IAR there is a little inconvenient is with inline function I can't disable Warning[Pa039]: use of address of unaligned structure member.
So we need to continue to use (uint8_t*) desc_bos + offsetof(tusb_desc_bos_t, wTotalLength) instead of &desc_bos->wTotalLength.

can the pragma suppress be added into the inline ? we can have temporarily add that for now and find an better/general way later on.

@hathach
Copy link
Copy Markdown
Owner Author

hathach commented Apr 6, 2021

I guess @HiFiPhile knows more of this topic but in any case, could you provide an unaligned access to int16 too? This is required for fast half word copies in the audio encoding/deconding! Thanks :)

Yeah, I was initially want to add 16-bit version, but put it off since I am not sure if we would use it at all. Though since there are all inline, there is no generated code if not used. So here it is

@HiFiPhile
Copy link
Copy Markdown
Collaborator

HiFiPhile commented Apr 6, 2021

With __attribute__((always_inline)) the warning can be disabled using pragma:

__attribute__((always_inline)) static inline uint32_t tu_unaligned_read32(const void* mem)
{
  _Pragma ("diag_suppress=Pa039");
  tu_unaligned_uint32_t const* ua32 = (tu_unaligned_uint32_t const*) mem;
  return ua32->val;
  _Pragma ("diag_default=Pa039");
}

Without always_inline sometimes IAR doesn't like to inline the function especially in low optimization, the pragma doesn't work since the warning is issued on function caller.

@hathach
Copy link
Copy Markdown
Owner Author

hathach commented Apr 6, 2021

@HiFiPhile I added the TU_ATTR_ALWAYS_INLINE, and applied it to basic inline function. Though I don't like to suppress and unsuppress it here. It makes code look less readable, also not all unaligned access need to suppress this e.g uint8_t/void. I am fine with the offsetof() at the caller where it is needed.

#ifdef __ICCARM__
  #define IAR_PA309_SUPPRESS    _Pragma("diag_suppress=Pa039")
  #define IAR_PA309_UNSUPPRESS  _Pragma ("diag_default=Pa039")
#else
  #define IAR_PA309_SUPPRESS
  #define IAR_PA309_UNSUPPRESS
#endif

#if TUP_ARCH_STRICT_ALIGN

typedef struct { uint16_t val; } TU_ATTR_PACKED tu_unaligned_uint16_t;
typedef struct { uint32_t val; } TU_ATTR_PACKED tu_unaligned_uint32_t;

TU_ATTR_ALWAYS_INLINE static inline uint32_t tu_unaligned_read32(const void* mem)
{
  IAR_PA309_SUPPRESS
  tu_unaligned_uint32_t const* ua32 = (tu_unaligned_uint32_t const*) mem;
  return ua32->val;
  IAR_PA309_UNSUPPRESS
}

TU_ATTR_ALWAYS_INLINE static inline void tu_unaligned_write32(void* mem, uint32_t value)
{
  IAR_PA309_SUPPRESS

  tu_unaligned_uint32_t* ua32 = (tu_unaligned_uint32_t*) mem;
  ua32->val = value;

  IAR_PA309_UNSUPPRESS
}

TU_ATTR_ALWAYS_INLINE static inline uint16_t tu_unaligned_read16(const void* mem)
{
  IAR_PA309_SUPPRESS

  tu_unaligned_uint16_t const* ua16 = (tu_unaligned_uint16_t const*) mem;
  return ua16->val;

  IAR_PA309_UNSUPPRESS
}

TU_ATTR_ALWAYS_INLINE static inline void tu_unaligned_write16(void* mem, uint16_t value)
{
  IAR_PA309_SUPPRESS

  tu_unaligned_uint16_t* ua16 = (tu_unaligned_uint16_t*) mem;
  ua16->val = value;

  IAR_PA309_UNSUPPRESS
}

#else

TU_ATTR_ALWAYS_INLINE static inline uint32_t tu_unaligned_read32  (const void* mem)
{
  IAR_PA309_SUPPRESS
  return *((uint32_t*) mem);
  IAR_PA309_UNSUPPRESS
}

@hathach hathach merged commit 1d20c84 into master Apr 6, 2021
@hathach hathach deleted the unaligned-access branch April 6, 2021 12:32
7FM pushed a commit to 7FM/tinyusb that referenced this pull request Aug 23, 2025
When cloning the pico-sdk repo manually, one normally would do `git
submodule update --init`, which is non-recursive. However, when cloning
automatically, CMake will recursively update submodules by default.
Updating all of tiny-usb's submodules takes an extremely long time.
Luckily, CMake 3.17 added an option we can specify for FetchContent to
tell it not to recursively update submodules. On older CMake versions,
the flag is not used. For those with a new enough version of CMake, this
will significantly speed up SDK cloning.

Fixes hathach#771.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants