Update mmu_get... and mmu_set... by mhightower83 · Pull Request #8290 · esp8266/Arduino

added 6 commits

August 23, 2021 19:25
Added 32-bit dependency injections as needed to guard against compiler
optimizing 32-bit loads from IRAM to 8-bit or 16-bit loads.
Added 32-bit dependency injections as needed to guard against compiler
optimizing 32-bit loads from IRAM to 8-bit or 16-bit loads.
…83/Arduino into pr-mmu-inline-strict-aliasing

@mhightower83

Corrected start of DRAM constant in mmu_is_dram().
Replaced #define(s) with const to properly limit scope. Compiler appears to
optomize it down to the same size.
In some places used ld variables and core-isa.h defines to set range checking values.

@mhightower83

mcspr

@mcspr

hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request

Nov 18, 2024
These changes are needed to address bugs that can emerge with the improved optimization from the GCC 10.3 compiler.

Updated performance inline functions `mmu_get_uint8()`, ... and `mmu_set_uint8()`, ...  to comply with strict-aliasing rules. 
Without this change, stale data may be referenced. This issue was revealed in discussions on esp8266#8261 (comment) 

Changes to avoid over-optimization of 32-bit wide transfers from IRAM, turning into 8-bit or 16-bit transfers by the new GCC 10.3 compiler. This has been a reoccurring/tricky problem for me with the new compiler. 

So far referencing the 32-bit value loaded by way of an Extended ASM R/W output register has stopped the compiler from optimizing down to an 8-bit or 16-bit transfer.

Example:
```cpp
  uint32_t val;
  __builtin_memcpy(&val, v32, sizeof(uint32_t));
  asm volatile ("" :"+r"(val)); // inject 32-bit dependency
  ...
```

Updated example `irammem.ino`
* do a simple test of compliance to strict-aliasing rules
* For `mmu_get_uint8()`, added tests to evaluate if 32-bit wide transfers were converted to an 8-bit transfer.