This is an archive of the discontinued LLVM Phabricator instance.

[libcxxabi] Define _LIBCXXABI_GUARD_ABI_ARM on WebAssembly

Authored by sbc100 on Jul 18 2019, 4:30 PM.

Event Timeline

sbc100 created this revision.Jul 18 2019, 4:30 PM
Herald added a project: Restricted Project. · View Herald Transcript

ARM is the correct monacer for this configuration. The spec we're implementing was written by ARM.

Can you elaborate a bit here about why we're choosing to deviate from the Itanium spec for WebAssembly?
Unless there's good reason why diverging from the Itanium spec is worth it I would rather we not do it.

But my opinion isn't too strong on this. libc++abi has to support this configuration ad infinitum.

My opinion isn't too strong here either. wasm32 doesn't need 8 bytes for this, and there's an existing ABI knob to use less, so it seems to make sense to use it.

I think the main argument is space saving. Wasm cares more about minimizing the binary size and memory usage than most other platforms. I believe that was the original motivation for the arm abi too (they tend to be embedded and care more about doubling the overhead of each guard variable).

The wasm32 ABI as implemented in clang and emscripten currently uses 4-byte guards so we can push back there and try to change the ABI there, or we can change libc++abi to comply the current de-facto ABI.

I originally went with the option of changing clang (and emscripten):

However I think as long as its documented the is no reason not so save a few bytes here in the WebAssembly case.

Can we go ahead with this? Or alternatively land

sbc100 added a comment.EditedAug 13 2019, 10:36 AM

What do you think @sunfish, @EricWF? I have a slight preference for landing this change but I want land one or the other.

sbc100 updated this revision to Diff 214878.Aug 13 2019, 10:40 AM
  • add comment
sunfish accepted this revision.Aug 13 2019, 12:23 PM

I have a slight preference for landing this change too.

This revision is now accepted and ready to land.Aug 13 2019, 12:23 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2019, 6:33 PM