Skip to content

Commit

Permalink
[clang][emscripten] Reduce alignof long double from 16 to 8 bytes
Browse files Browse the repository at this point in the history
This means `max_align_t` is 8 bytes which also sets the alignment
malloc.  Since this is technically and ABI breaking change we have
limited to just the emscripten OS target.  It is also relatively low
import breakage since it will only effect the alignement of struct that
contai `long double`s (extremerly rare I imagine).

Emscripten's malloc implementation already use 8 byte alignement
(dlmalloc uses and alignement of 2*sizeof(void*) == 8 rather than
checking max_align_t) so will not be effected by this change.  By
bringing the ABI in line with the current malloc code this will fix
several issue we have seen in the wild.

See: emscripten-core/emscripten#14456

Differential Revision: https://reviews.llvm.org/D104808
  • Loading branch information
sbc100 committed Jul 2, 2021
1 parent 3ec88ca commit d1a96e9
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions clang/lib/Basic/Targets/OSTargets.h
Original file line number Diff line number Diff line change
Expand Up @@ -948,8 +948,16 @@ class LLVM_LIBRARY_VISIBILITY EmscriptenTargetInfo
}

public:
explicit EmscriptenTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
: WebAssemblyOSTargetInfo<Target>(Triple, Opts) {}
explicit EmscriptenTargetInfo(const llvm::Triple &Triple,
const TargetOptions &Opts)
: WebAssemblyOSTargetInfo<Target>(Triple, Opts) {
// Keeping the alignment of long double to 8 bytes even though its size is
// 16 bytes allows emscripten to have an 8-byte-aligned max_align_t which
// in turn gives is a 8-byte aligned malloc.
// Emscripten's ABI is unstable and we may change this back to 128 to match
// the WebAssembly default in the future.
this->LongDoubleAlign = 64;
}
};

} // namespace targets
Expand Down

0 comments on commit d1a96e9

Please sign in to comment.