@whitequark can you take another look at this patch? (I can rebase it if needed).

For what it's worth, RISC-V implements semantics close to but not exactly like ARM, with NaN being treated as if it was +Infinity.
Source: https://content.riscv.org/wp-content/uploads/2017/05/riscv-spec-v2.2.pdf#page=63

Ping?
This would be super useful to have: it avoids copying most of cc1as.

So far nobody has mentioned that the reduced struct (struct Foo { unsigned myBit : 1; char myByte; };) is 8 bytes with MSVC. It's highly possible that GCC is aligning their ABI with MSVC for better compatibility.

I ran the same code as above on MinGW 7.3.0, and with that version the struct size is 4 bytes. So it looks like GCC/MinGW changed how they lay out a struct in memory (breaking ABI compatibility?).

Another test. I compiled the following code: https://godbolt.org/z/hVCAUB

I ran the following commands in CI:

I wrote a patch, see: https://reviews.llvm.org/D70266

I did some monkey-patching of Symbol.h:

Just a note after the fact: this change broke linking in TinyGo. It used to manually build an archive (without index) with object files from compiler-rt and that worked just fine in LLVM 8 - it did read symbols from those archives even without a symbol table. In LLVM 9, ld.lld refuses to link those libraries. We'll work around it by manually building a symbol table for lld.

Sorry, that was probably https://reviews.llvm.org/D64077 instead.

FYI: this change caused a build error when including the AVR backend:

I do not think there is a reason why the Xtensa architecture shouldn't be included in LLVM. As a comparison, there is also the proprietary (Google) Lanai architecture which was added at a time when there was little public information on it and to this time nobody can actually buy Lanai hardware (there is only a simulator). As a comparison, Xtensa has easy to get hardware (in the form of the ESP8266/ESP32) and is already supported in GCC/binutils.

@hans can you help me out? I accidentally pushed to the release_80 branch instead of to trunk, because of a mistake in my git-llvm configuration. I think it should be reverted.
Also see IRC logs starting 17:52 UTC on this day (Sunday).

Quickly scanning this patch shows that it still uses the previous license. I think these need to be updated to the new Apache 2.0 license, see any file for the up-to-date file header.

LGTM, but I know very little about the LLVM backend.
Also, I see that this patch lacks one change that are included by the very similar RISC-V patch: https://github.com/lowRISC/riscv-llvm/blob/master/0003-RISCV-Add-RISC-V-ELF-defines.patch, namely the equivalent of ElfHeaderRISCVFlags in ELFDumper.cpp. I don't know whether that has a reason but mentioning it just in case.

*friendly ping*

LGTM from my perspective but I'm not a C++ person so can't comment on style etc.

Is there a reason why this hasn't been committed yet?

ping?

• removed useless anonymous namespace

@CodaFi thanks for the review! I assume "Accepted" means it can be merged now? (Just checking to be sure).

Yes, losing a bit of history is unfortunate. However, it will make it easier in the future to keep these bindings machine-formatted.

Ok, based on the above comments I believe that this is a bug in LLVM and not a missing feature in compiler-rt. LLVM should generate __atomic_* calls instead of __sync_ calls and it shouldn't generate those for Cortex-M3 and up when doing 32-bit atomics because it should be able to emit such instructions inline.

Phabricator says:

Merged in r362893.

Merged in r362890.

Merged.

• update to stripPointerCastsSameRepresentation
• move a single line of code (NFC)

I think I have addressed all review comments (except for stripPointerCasts which is waiting on D61607 and should hopefully land soon).

• use Value::getPointerDereferenceableBytes instead of checking for the specific attribute dereferenceable_or_null.
• Only apply this optimization when null pointers are not dereferenceable ("null-pointer-is-valid") and add an associated test.

Yes I know it can change the bit pattern. And if it is only about the bit pattern, the name is fine. However, it seems to me that the only difference between stripPointerCastsSameBitPattern and stripPointerCasts is the AS, which may or may not be relevant to the bit pattern. This is confusing: it isn't clear to me from the name stripPointerCastsSameBitPattern whether it may strip any addrspacecast that has the same bit pattern.

I've updated the code, but there are a few things I'm not so sure about:

• replace some types with auto
• update comment for getelementptr inbounds

To me (someone from outside LLVM), the "keep bit pattern" argument looks confusing. Pointer casts do not necessarily change the bit pattern? In fact, it seems to me that most pointer casts preserve the bit pattern but change the meaning of the pointer (example: address space 1 on AVR which points into flash instead of RAM - no bits are changed with an addrspacecast). Names like stripTrivialPointerCasts and stripAddressSpaceAndPointerCasts look much more obvious to me. Or did I miss the real intent of the change here?
In other words, I would suggest renaming stripPointerCastsSameBitPattern to stripTrivialPointerCasts.

Thank you, so I guess this is now ready to land?

• updated to allow bitcasts
• include bitcasts in tests
• other small improvements to the test

• now _really_ update the URL

• updated the URL to the GCC docs

• fixed the bug in SYNC_OP_8

• use the ldrex/strex implementation on Cortex-M3 and up for SYNC_OP_4
• fixed a bug in SYNC_OP_4 for the sub instruction (a bug remains in SYNC_OP_8, will fix soon)

No there isn't any reason to avoid ldrex/strex on M-class. It is the recommended way of implementing these functions on all architectures that support them.

If someone could suggest a good way to test this, that would be great. At the moment it has been tested by compiling for armv6m, armv7m, and armv7em and confirming that the add operations appear to work.