In D68342#1710616, @lebedev.ri wrote:

In D68342#1710600, @miyuki wrote:

@lebedev.ri, do you agree that all address spaces should be treated the same way as address space 0 (i.e. no signed overflow)?

I wouldn't be surprised if that isn't so, i don't think it's really documented what normal assumptions do and don't apply to non-0-address-spaces.

OK, let's stick to what is documented for GEP (and the status quo about objects larger than half of the address space).

Don't treat non-zero address space specially

@lebedev.ri, do you agree that all address spaces should be treated the same way as address space 0 (i.e. no signed overflow)?

Added the NSW flag. This discussion thread also suggests that signed overflows should not occur in the inbounds GEPs: https://lists.llvm.org/pipermail/llvm-dev/2017-November/118914.html

ping

So clang is perfectly correct here.

Yes, Clang is correct. EmitGEPOffset is doing the wrong thing.
nuw is incorrect because negative offsets are allowed. nsw would also be incorrect because of the quote you mentioned before:

I also thought that "in bounds address of an allocated object" has something to do with the type used in the GEP instruction, but that's not how Clang interprets it.
E.g. for the following code

int read(int *buf) {
  buf -= 2;
  return *buf;
}

It generates the following:

define dso_local i32 @_Z4readPi(i32* nocapture readonly %buf) local_unnamed_addr #0 {
entry:
  %add.ptr = getelementptr inbounds i32, i32* %buf, i64 -2
  %0 = load i32, i32* %add.ptr, align 4, !tbaa !2
  ret i32 %0
}

I do not understand this point, could you elaborate please?

In D68342#1691617, @lebedev.ri wrote:

I'm not fully convinced this is correct, as per
https://llvm.org/docs/LangRef.html#getelementptr-instruction

If the inbounds keyword is present, the result value of the getelementptr is a poison value
if the base pointer is not an in bounds address of an allocated object, or if any of the
addresses that would be formed by successive addition of the offsets implied by the indices
to the base address with infinitely precise signed arithmetic are not an in bounds address
of that allocated object. <...>

I also thought that "in bounds address of an allocated object" has something to do with the type used in the GEP instruction, but that's not how Clang interprets it.
E.g. for the following code

Added from __future__ import print_function

In D68138#1686487, @greened wrote:

What happens when this is run with python 2 in the path? Should this explicitly run via python3?

Could you please look into https://bugs.llvm.org/show_bug.cgi?id=43206 ?

Hi. Could you please look into https://bugs.llvm.org/show_bug.cgi?id=43183 (it seems to be related)

Removed some redundancy.

Uploaded the correct patch

Changed predicate for variants with suffixes

Abandoned in favor of:
https://reviews.llvm.org/rG1ab3fe8a7a6537f882bbb2dac72c479d05eb8c49 and
https://reviews.llvm.org/rGf814dcbafbabd46a1babaeabec6acc3b70951bf4

Shouldn't the type change be guarded with a conditional compilation construct? I could write

std::list<int> l
assert(typeid(void) == typeid(l.unique()));

and expect this assertion to pass when compiling with, say, -std=c++98.

We have a similar problem with our port of libc++ and we also had to conditionalize all uses of long double downstream. Here is a list of files that we had to change: https://pastebin.com/5tTcSYxW
As you see, most of them are tests.
Another problem with long double is that it is used in a couple of places not strictly requiring it (include/locale, include/__mutex_base and include/thread), in those cases we introduced a _LIBCPP_LONGEST_DOUBLE macro which is defined to either double or long double depending on the availability of the latter.

Fix a comment

Subsumed by: https://reviews.llvm.org/D63838

LGTM

Added a test which uses <2 x double>. Alignment issue should be fixed separately, IMHO.

We maintain such library for an embedded toolchain. Besides, only C++17 and later versions are based on C11.

__libcpp_condvar_timedwait needs to be implemented in a different TU, so it can't be a template and at the same time it needs some sort of time point. I think timespec was chosen because it is what pthread_cond_timedwait expects. Do you think a specialization of chrono::time_point (like chrono::time_point<chrono::system_clock, chrono::nanoseconds> used by condition_variable::__do_timed_wait) would make more sense here?

In D63437#1546312, @ostannard wrote:

This doesn't look like the right pace to fix this - the backend can handle vectors of i8 and i16, which are also not legal types, so why can't it correctly handle vectors of f16?