- User Since
- Apr 25 2018, 9:12 AM (68 w, 4 d)
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.
LGTM from my perspective but I'm not a C++ person so can't comment on style etc.
Thu, Aug 15
Is there a reason why this hasn't been committed yet?
Jul 17 2019
Jul 3 2019
Jun 26 2019
- removed useless anonymous namespace
Jun 25 2019
Jun 24 2019
Jun 18 2019
@CodaFi thanks for the review! I assume "Accepted" means it can be merged now? (Just checking to be sure).
Jun 12 2019
Yes, losing a bit of history is unfortunate. However, it will make it easier in the future to keep these bindings machine-formatted.
Jun 9 2019
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.
Jun 8 2019
Merged in r362893.
Merged in r362890.
Jun 5 2019
- update to stripPointerCastsSameRepresentation
- move a single line of code (NFC)
May 26 2019
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.
May 18 2019
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.
May 3 2019
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
May 2 2019
May 1 2019
- 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)
Apr 30 2019
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.
Apr 29 2019
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.
Apr 25 2019
Update CaptureTracking to be more conservative and add FunctionAttrs tests.
Can someone merge this commit? I do not have commit access.
Apr 23 2019
Apr 12 2019
Apr 4 2019
I don't have commit access.
Apr 3 2019
Oh, I see. Thank you for explaining the actual behavior of getelementptr.
Mar 31 2019
Some more background:
Feb 14 2019
Can you (or someone else) merge this change? I do not have commit access. That would allow a request for inclusion in LLVM 8 (I hope).
Feb 11 2019
Jan 6 2019
Can someone merge this patch? I don't have commit access.
Can someone commit this change? I don't have commit access.