[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D42933#1090384, @jfb wrote:

> In https://reviews.llvm.org/D42933#1090286, @smeenai wrote:
>
> > I'd be fine with adding an option to relax the printf checking if the size 
> > and alignment of the specifier and the actual type match, even if the types 
> > themselves differ (`-Wformat-relaxed` or something similar), so that you'd 
> > still get warnings on cases where the specifier mismatch could cause 
> > runtime issues.
>
>
> What are the cases that you're worried about? The only ones I'm trying to 
> capture here are `NSInteger` with `%zd` and `NSUInteger` with `%zu`, are 
> there others?
>
> > I think that would be preferable to special-casing the Apple types.
>
> If there are more that should be captured and a similar point solution 
> doesn't apply, agreed. However I'd like to understand if we agree on the 
> guarantees that the platform offers for the two specific cases I'm targeting.


I also think that special casing these two specifiers doesn't make sense. The 
problem is a general issue -- and one I've often found irritating. This exact 
same situation comes up all the time in non-Darwin contexts too.

E.g. one I find particularly annoying is "%lld" vs "%ld" in printf.

Some 64-bit platforms define e.g. `int64_t` as `long long`, and others as 
`long`. Although both types are size 8, align 8, and mean exactly the same 
thing, they are still distinct. And so, users write code like `int64_t x = 0; 
printf("%ld", x);`...which then emits warnings on the platform that defines 
int64_t as a `long long`. Obviously, the code _could_ be using `PRId64` 
instead...but it often doesn't. So it'd sure be nice if you could restrict the 
warning to only warn about actual problems, and suppress these 
superficially-incompatible-but-not-really warnings.

So, +1 from me for the general-purpose `-Wformat-relaxed` flag (or whatever 
other flag name is appropriate).


Repository:
  rC Clang

https://reviews.llvm.org/D42933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D42933#1091817, @jfb wrote:

> In https://reviews.llvm.org/D42933#1091809, @jyknight wrote:
>
> > I also think that special casing these two specifiers doesn't make sense. 
> > The problem is a general issue -- and one I've often found irritating. This 
> > exact same situation comes up all the time in non-Darwin contexts too.
>
>
> I don't think that's true. In this very specific case the platform guarantees 
> that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == 
> sizeof(NSUInteger))` for all architectures this platform supports. This exact 
> same situation does not come up all the time in other contexts because the 
> `int` / `long` / `long long` distinction isn't backed by a portability 
> guarantee. The warning is there to say "this code isn't portable!", but in 
> the very specific case of `NSInteger` and `NSUInteger` it is and users rely 
> on it so it cannot be broken. The warning is therefore spurious, users 
> therefore rightly complain.


The printf format specifier warning is not primarily a cross-platform 
portability checker. And, although in a few limited cases it can act somewhat 
like one, in general it does not. Take my previous example -- you get no 
warning on a platform that has int64_t as a typedef for long -- if this feature 
is to be useful as a portability checker, it should require that you used the 
PRId64 macro. Or, given `ssize_t x = 0; printf("%ld", x);`, it doesn't tell you 
to use "%zd" instead if ssize_t is a typedef for long -- although to be 
portable you ought to.

No, the major usefulness of the printf warning is to tell you that your code is 
incorrect for the _current_ platform. And //most// importantly when you chose 
the wrong size for your argument.

Those types which have matched size and alignment are still different types, 
and so it's technically appropriate to warn about using the wrong specifier 
there, too. But it's also entirely reasonable to not want to be bothered by 
such useless trivia, so skipping the warning when there's only a technical and 
not actual mismatch seems entirely sensible. (I might suggest that it should 
even be the default behavior for the warning, and if you want the stricter 
checks you'd ask for them...but I bet I'll get more pushback on that...)


Repository:
  rC Clang

https://reviews.llvm.org/D42933



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47137: [Sparc] Add floating-point register names

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Can you add a test that we support this? 
clang/test/CodeGen/sparcv8-inline-asm.c would be a good place to add a test 
case similar to that in clang/test/CodeGen/aarch64-inline-asm.c : 
test_gcc_registers.


Repository:
  rC Clang

https://reviews.llvm.org/D47137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

There are more leon-based v8 CPUs listed in clang/lib/Basic/Targets/Sparc.cpp, 
which need to be listed here, also.

Separately, I think it'd be a good idea to refactor to put this info into the 
SparcCPUInfo struct, so that it's harder for them to get out of sync.


Repository:
  rC Clang

https://reviews.llvm.org/D47138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D47138#1107637, @dcederman wrote:

> I did not find a good way to access the SparcCPUInfo struct from here. No 
> other arch under Toolchains seems to access TargetInfo.


OK.


https://reviews.llvm.org/D47138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47137: [Sparc] Add floating-point register names

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Basic/Targets/Sparc.cpp:32
+"f22", "f23", "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", 
"f32",
+"f33", "f34", "f35", "f36", "f37", "f38", "f39", "f40", "f41", "f42", 
"f43",
+"f44", "f45", "f46", "f47", "f48", "f49", "f50", "f51", "f52", "f53", 
"f54",

There's no such register as f33 (nor any odd-numbered reg above f32).



Comment at: lib/Basic/Targets/Sparc.cpp:52
+
+// Double precision floating-point register
+{{"d0"}, "f0"},   {{"d1"}, "f2"},   {{"d2"}, "f4"},   {{"d3"}, "f6"},

AFAICT, gcc doesn't actually accept "d" and "q" aliases in its inline asm, so 
probably no point in LLVM doing so either.


https://reviews.llvm.org/D47137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I think the diagnosis on the original issue was incorrect.

It seems to me that it was caused by the prefix being set as "/bin" instead of 
"/usr/bin", because clang _doesn't_ actually canonicalize its prefix, even when 
-no-canonical-prefixes isn't specified! All it does, now, is to make the prefix 
absolute -- without fully canonicalizing. I think that's simply a bug.

If the prefix had been, properly, /usr/bin, then the include path would've been:

  
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.2.0/../../../../include/c++/7.2.0/iostream

And in that case, it would've worked properly with ninja, without adding the 
feature in this patch.

Reference clang/tools/driver/driver.cpp:297:

  // FIXME: We don't actually canonicalize this, we just make it absolute.
  if (CanonicalPrefixes)
llvm::sys::fs::make_absolute(InstalledPath);


Repository:
  rL LLVM

https://reviews.llvm.org/D37954



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I'd still _very_ much like a solution that is acceptable for all libc to use, 
and have that on by default.

However, I guess this is fine.

Only concern I have is that it seems odd that so many test-cases needed to be 
changed. What fails without those test changes?


https://reviews.llvm.org/D34158



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

Typo in commit message? They were added to 2.5, not 2.4 (the code is right, 
just the comment is wrong).


Repository:
  rCXX libc++

https://reviews.llvm.org/D49927



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305394: PR32476: __nop_locale_mgmt.h not needed with newlib 
2.5+ (authored by jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D32146?vs=102265&id=102562#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32146

Files:
  libcxx/trunk/include/support/newlib/xlocale.h


Index: libcxx/trunk/include/support/newlib/xlocale.h
===
--- libcxx/trunk/include/support/newlib/xlocale.h
+++ libcxx/trunk/include/support/newlib/xlocale.h
@@ -16,7 +16,10 @@
 #include 
 #include 
 #include 
+#if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \
+__NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
 #include 
+#endif
 #include 
 #include 
 


Index: libcxx/trunk/include/support/newlib/xlocale.h
===
--- libcxx/trunk/include/support/newlib/xlocale.h
+++ libcxx/trunk/include/support/newlib/xlocale.h
@@ -16,7 +16,10 @@
 #include 
 #include 
 #include 
+#if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \
+__NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
 #include 
+#endif
 #include 
 #include 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34105: Define _GNU_SOURCE for rtems c++

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305399: Define _GNU_SOURCE for rtems c++ (authored by 
jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D34105?vs=102188&id=102565#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34105

Files:
  cfe/trunk/lib/Basic/Targets.cpp
  cfe/trunk/test/Preprocessor/init.c


Index: cfe/trunk/test/Preprocessor/init.c
===
--- cfe/trunk/test/Preprocessor/init.c
+++ cfe/trunk/test/Preprocessor/init.c
@@ -8779,6 +8779,7 @@
 // KFREEBSDI686-DEFINE:#define __GLIBC__ 1
 //
 // RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM 
< /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s
+// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix GNUSOURCE %s
 // GNUSOURCE:#define _GNU_SOURCE 1
 //
 // RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix NORTTI %s
Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -4734,6 +4734,9 @@
 
 Builder.defineMacro("__rtems__");
 Builder.defineMacro("__ELF__");
+// Required by the libc++ locale support.
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
   }
 
 public:


Index: cfe/trunk/test/Preprocessor/init.c
===
--- cfe/trunk/test/Preprocessor/init.c
+++ cfe/trunk/test/Preprocessor/init.c
@@ -8779,6 +8779,7 @@
 // KFREEBSDI686-DEFINE:#define __GLIBC__ 1
 //
 // RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s
+// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s
 // GNUSOURCE:#define _GNU_SOURCE 1
 //
 // RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix NORTTI %s
Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -4734,6 +4734,9 @@
 
 Builder.defineMacro("__rtems__");
 Builder.defineMacro("__ELF__");
+// Required by the libc++ locale support.
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
   }
 
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34294: Rework libcxx strerror_r handling.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
Herald added subscribers: krytarowski, srhines.

The set of #ifdefs used to handle the two incompatible variants of
strerror_r were not complete (they didn't handle newlib appropriately).

Rather than attempting to make the ifdefs more complex, make them
unnecessary by choosing which behavior to use dependent upon the
return type.


https://reviews.llvm.org/D34294

Files:
  src/system_error.cpp


Index: src/system_error.cpp
===
--- src/system_error.cpp
+++ src/system_error.cpp
@@ -73,39 +73,59 @@
   std::snprintf(buffer, strerror_buff_size, "unknown error %d", ev);
   return string(buffer);
 }
-#elif defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC) && 
\
-(!defined(__ANDROID__) || __ANDROID_API__ >= 23)
-// GNU Extended version
-string do_strerror_r(int ev) {
-char buffer[strerror_buff_size];
-char* ret = ::strerror_r(ev, buffer, strerror_buff_size);
-return string(ret);
-}
 #else
-// POSIX version
+
+// Only one of the two following functions will be used, depending on
+// the return type of strerror_r:
+
+// For the GNU variant, a char* return value:
+__attribute__((unused)) const char *
+handle_strerror_r_return(char *strerror_return, char *buffer) {
+  // GNU always returns a string pointer in its return value. The
+  // string might point to either the input buffer, or a static
+  // buffer, but we don't care which.
+  return strerror_return;
+}
+
+// For the POSIX variant: an int return value.
+__attribute__((unused)) const char *
+handle_strerror_r_return(int strerror_return, char *buffer) {
+  // The POSIX variant either:
+  // - fills in the provided buffer and returns 0
+  // - returns a positive error value, or
+  // - returns -1 and fills in errno with an error value.
+  if (strerror_return == 0)
+return buffer;
+
+  // Only handle EINVAL. Other errors abort.
+  int new_errno = strerror_return == -1 ? errno : strerror_return;
+  if (new_errno == EINVAL)
+return "";
+
+  _LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from ::strerror_r");
+  // FIXME maybe? 'strerror_buff_size' is likely to exceed the
+  // maximum error size so ERANGE shouldn't be returned.
+  std::abort();
+}
+
+// This function handles both GNU and POSIX variants, dispatching to
+// one of the two above functions.
 string do_strerror_r(int ev) {
 char buffer[strerror_buff_size];
+// Preserve errno around the call. (The C++ standard requires that
+// system_error functions not modify errno).
 const int old_errno = errno;
-int ret;
-if ((ret = ::strerror_r(ev, buffer, strerror_buff_size)) != 0) {
-// If `ret == -1` then the error is specified using `errno`, otherwise
-// `ret` represents the error.
-const int new_errno = ret == -1 ? errno : ret;
-errno = old_errno;
-if (new_errno == EINVAL) {
-std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev);
-return string(buffer);
-} else {
-_LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from 
::strerr_r");
-// FIXME maybe? 'strerror_buff_size' is likely to exceed the
-// maximum error size so ERANGE shouldn't be returned.
-std::abort();
-}
+const char *error_message = handle_strerror_r_return(
+::strerror_r(ev, buffer, strerror_buff_size), buffer);
+// If we didn't get any message, print one now.
+if (!error_message[0]) {
+  std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev);
+  error_message = buffer;
 }
-return string(buffer);
+errno = old_errno;
+return string(error_message);
 }
 #endif
-
 } // end namespace
 #endif
 


Index: src/system_error.cpp
===
--- src/system_error.cpp
+++ src/system_error.cpp
@@ -73,39 +73,59 @@
   std::snprintf(buffer, strerror_buff_size, "unknown error %d", ev);
   return string(buffer);
 }
-#elif defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC) && \
-(!defined(__ANDROID__) || __ANDROID_API__ >= 23)
-// GNU Extended version
-string do_strerror_r(int ev) {
-char buffer[strerror_buff_size];
-char* ret = ::strerror_r(ev, buffer, strerror_buff_size);
-return string(ret);
-}
 #else
-// POSIX version
+
+// Only one of the two following functions will be used, depending on
+// the return type of strerror_r:
+
+// For the GNU variant, a char* return value:
+__attribute__((unused)) const char *
+handle_strerror_r_return(char *strerror_return, char *buffer) {
+  // GNU always returns a string pointer in its return value. The
+  // string might point to either the input buffer, or a static
+  // buffer, but we don't care which.
+  return strerror_return;
+}
+
+// For the POSIX variant: an int return value.
+__attribute__((unused)) const char *
+handle_strerror_r_return(int strerror_return, char *buf

[PATCH] D34294: Rework libcxx strerror_r handling.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D34294#782806, @krytarowski wrote:

> New one is harder to comprehend and less portable (usage of `__atribute__`).


I can't disagree more strongly.  This is fundamentally portable C++ code -- 
__attribute__((unused)) is simply warning suppression.
I used it directly, because I saw other .cpp files in libcxx already did so. If 
it needs to be conditioned, it could be.

> Or better:



> #elif __GLIBC__ || (__ANDROID_API__ - 0) >= 23
>  If I understand correctly, Android adopted GNU version.

No, we can't use that conditional, because android and glibc aren't the only 
ones to have adopted the GNU variant. As I mentioned, newlib uses it also.

We *may* be able to use a different conditional than what's there or what you 
suggested, but solving the problem once and for all by using the function type 
is certainly a better solution.


https://reviews.llvm.org/D34294



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34294: Rework libcxx strerror_r handling.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

EricWF, wdyt?


https://reviews.llvm.org/D34294



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35755: [Solaris] gcc toolchain handling revamp

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

This looks reasonable on the face of it.

I'm assuming you know the layout for Solaris, and it doesn't seem to change the 
behavior of non-Solaris, so LGTM.


https://reviews.llvm.org/D35755



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D47894#1125653, @efriedma wrote:

> The problem would come from propagating nonnull-ness from something which 
> isn't inherently nonnull.  For example, strlen has a nonnull argument, so 
> `strlen(NULL)` is UB, therefore given `int z = strlen(x); if (x) {...}`, we 
> can remove the null check.  (Not sure we actually do this transform at the 
> moment, but it's something we could do in the future.)


I think the question there is actually whether we need to, in addition to 
supporting null pointer dereference, also cause all __attribute__((nonnull)) 
annotations at the C/C++ level to be ignored.

And, yes, I believe we should.


Repository:
  rC Clang

https://reviews.llvm.org/D47894



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D47894#1125961, @srhines wrote:

> In https://reviews.llvm.org/D47894#1125728, @jyknight wrote:
>
> > In https://reviews.llvm.org/D47894#1125653, @efriedma wrote:
> >
> > > The problem would come from propagating nonnull-ness from something which 
> > > isn't inherently nonnull.  For example, strlen has a nonnull argument, so 
> > > `strlen(NULL)` is UB, therefore given `int z = strlen(x); if (x) {...}`, 
> > > we can remove the null check.  (Not sure we actually do this transform at 
> > > the moment, but it's something we could do in the future.)
> >
> >
> > I think the question there is actually whether we need to, in addition to 
> > supporting null pointer dereference, also cause all 
> > `__attribute__((nonnull))` annotations at the C/C++ level to be ignored.
> >
> > And, yes, I believe we should.
>
>
> Is that what the kernel community actually wants though? There are certainly 
> others who want a way to strip `__attribute__((nonnull))` completely, which 
> seems a bit orthogonal to the removal of null checks in general. I think it 
> would be good to support such a flag, but the presence of `nonnull` inside 
> kernel sources leads me to believe they want those cases to be treated 
> specially.


In GCC, the nonnull attribute still has an effect on warnings but not on 
codegen if you enable -fno-delete-null-pointer-checks: 
https://godbolt.org/g/7SSi2V

That seems a pretty sensible behavior, IMO.


Repository:
  rC Clang

https://reviews.llvm.org/D47894



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52703: Allow ifunc resolvers to accept arguments

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

Given that there's no technical reason for the compiler to prohibit this (it 
was just clang trying to diagnose a probable user-error, which turns out to not 
be as probable as ), this seems like the right solution to me.


https://reviews.llvm.org/D52703



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC344110: ExprConstant: Make __builtin_object_size use 
EM_IgnoreSideEffects. (authored by jyknight, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D52924?vs=168435&id=168935#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52924

Files:
  lib/AST/ExprConstant.cpp


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -759,18 +759,6 @@
   /// context we try to fold them immediately since the optimizer never
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
-
-  /// Evaluate as a constant expression. In certain scenarios, if:
-  /// - we find a MemberExpr with a base that can't be evaluated, or
-  /// - we find a variable initialized with a call to a function that has
-  ///   the alloc_size attribute on it
-  /// then we may consider evaluation to have succeeded.
-  ///
-  /// In either case, the LValue returned shall have an invalid base; in 
the
-  /// former, the base will be the invalid MemberExpr, in the latter, the
-  /// base will be either the alloc_size CallExpr or a CastExpr wrapping
-  /// said CallExpr.
-  EM_OffsetFold,
 } EvalMode;
 
 /// Are we checking whether the expression is a potential constant
@@ -874,7 +862,6 @@
   case EM_PotentialConstantExpression:
   case EM_ConstantExpressionUnevaluated:
   case EM_PotentialConstantExpressionUnevaluated:
-  case EM_OffsetFold:
 HasActiveDiagnostic = false;
 return OptionalDiagnostic();
   }
@@ -966,7 +953,6 @@
   case EM_ConstantExpression:
   case EM_ConstantExpressionUnevaluated:
   case EM_ConstantFold:
-  case EM_OffsetFold:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -985,7 +971,6 @@
   case EM_EvaluateForOverflow:
   case EM_IgnoreSideEffects:
   case EM_ConstantFold:
-  case EM_OffsetFold:
 return true;
 
   case EM_PotentialConstantExpression:
@@ -1021,7 +1006,6 @@
   case EM_ConstantExpressionUnevaluated:
   case EM_ConstantFold:
   case EM_IgnoreSideEffects:
-  case EM_OffsetFold:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -1093,18 +1077,18 @@
 }
   };
 
-  /// RAII object used to treat the current evaluation as the correct pointer
-  /// offset fold for the current EvalMode
-  struct FoldOffsetRAII {
+  /// RAII object used to set the current evaluation mode to ignore
+  /// side-effects.
+  struct IgnoreSideEffectsRAII {
 EvalInfo &Info;
 EvalInfo::EvaluationMode OldMode;
-explicit FoldOffsetRAII(EvalInfo &Info)
+explicit IgnoreSideEffectsRAII(EvalInfo &Info)
 : Info(Info), OldMode(Info.EvalMode) {
   if (!Info.checkingPotentialConstantExpression())
-Info.EvalMode = EvalInfo::EM_OffsetFold;
+Info.EvalMode = EvalInfo::EM_IgnoreSideEffects;
 }
 
-~FoldOffsetRAII() { Info.EvalMode = OldMode; }
+~IgnoreSideEffectsRAII() { Info.EvalMode = OldMode; }
   };
 
   /// RAII object used to optionally suppress diagnostics and side-effects from
@@ -8049,7 +8033,7 @@
 // If there are any, but we can determine the pointed-to object anyway, 
then
 // ignore the side-effects.
 SpeculativeEvaluationRAII SpeculativeEval(Info);
-FoldOffsetRAII Fold(Info);
+IgnoreSideEffectsRAII Fold(Info);
 
 if (E->isGLValue()) {
   // It's possible for us to be given GLValues if we're called via
@@ -8117,7 +8101,6 @@
 case EvalInfo::EM_ConstantFold:
 case EvalInfo::EM_EvaluateForOverflow:
 case EvalInfo::EM_IgnoreSideEffects:
-case EvalInfo::EM_OffsetFold:
   // Leave it to IR generation.
   return Error(E);
 case EvalInfo::EM_ConstantExpressionUnevaluated:


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -759,18 +759,6 @@
   /// context we try to fold them immediately since the optimizer never
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
-
-  /// Evaluate as a constant expression. In certain scenarios, if:
-  /// - we find a MemberExpr with a base that can't be evaluated, or
-  /// - we find a variable initialized with a call to a function that has
-  ///   the alloc_size attribute on it
-  /// then we may consider evaluation to have succeeded.
-  ///
-  /// In either case, the LValue returned shall have an invalid base; in the
-  /// former, the base will be the invalid MemberExpr, in the latter, the
-  /// base will be either the alloc_size CallExpr or a CastExpr wrapping
-  /// said CallExpr.
-

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

It is intended to disable _all_ warnings, even those upgraded to
errors via `-Werror=warningname` or `#pragma clang diagnostic error'


https://reviews.llvm.org/D53199

Files:
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Frontend/warning-mapping-4.c
  clang/test/Frontend/warning-mapping-5.c
  clang/test/Frontend/warning-mapping-6.c


Index: clang/test/Frontend/warning-mapping-6.c
===
--- /dev/null
+++ clang/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: clang/test/Frontend/warning-mapping-5.c
===
--- clang/test/Frontend/warning-mapping-5.c
+++ clang/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: clang/test/Frontend/warning-mapping-4.c
===
--- clang/test/Frontend/warning-mapping-4.c
+++ clang/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 // RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
 // RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -w -Werror=sign-compare %s
 // expected-no-diagnostics
 
 int f0(int x, unsigned y) {
Index: clang/test/Frontend/warning-mapping-2.c
===
--- clang/test/Frontend/warning-mapping-2.c
+++ clang/test/Frontend/warning-mapping-2.c
@@ -1,5 +1,7 @@
-// Check that -w has lower priority than -pedantic-errors.
+// Check that -w takes precedence over -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
 
-void f0() { f1(); } // expected-error {{implicit declaration of function}}
+// Expect *not* to see a diagnostic for "implicit declaration of function"
+// expected-no-diagnostics
 
+void f0() { f1(); }
Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -457,12 +457,15 @@
   if (Result == diag::Severity::Ignored)
 return Result;
 
-  // Honor -w, which is lower in priority than pedantic-errors, but higher than
-  // -Werror.
-  // FIXME: Under GCC, this also suppresses warnings that have been mapped to
-  // errors by -W flags and #pragma diagnostic.
-  if (Result == diag::Severity::Warning && State->IgnoreAllWarnings)
-return diag::Severity::Ignored;
+  // Honor -w: this disables all messages mapped to Warning severity, and 
*also*
+  // any diagnostics which are not Error/Fatal by default (that is, they were
+  // upgraded by any of the mechanisms available: -Werror, -pedantic, or 
#pragma
+  // diagnostic)
+  if (State->IgnoreAllWarnings) {
+if (Result == diag::Severity::Warning ||
+!isDefaultMappingAsError((diag::kind)DiagID))
+  return diag::Severity::Ignored;
+  }
 
   // If -Werror is enabled, map warnings to errors unless explicitly disabled.
   if (Result == diag::Severity::Warning) {


Index: clang/test/Frontend/warning-mapping-6.c
===
--- /dev/null
+++ clang/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: clang/test/Frontend/warning-mapping-5.c
===
--- clang/test/Frontend/warning-mapping-5.c
+++ clang/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: clang/test/Frontend/warning-mapping-4.c
===
--- clang/test/Frontend/warning-mapping-4.c
+++ clang/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 169641.
jyknight marked 11 inline comments as done.
jyknight added a comment.

Address most comments.


https://reviews.llvm.org/D52939

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ExprConstant.cpp

Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -412,52 +412,10 @@
   MostDerivedArraySize = 2;
   MostDerivedPathLength = Entries.size();
 }
-void diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, const Expr *E);
-void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
+bool diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
const APSInt &N);
 /// Add N to the address of this subobject.
-void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) {
-  if (Invalid || !N) return;
-  uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
-  if (isMostDerivedAnUnsizedArray()) {
-diagnoseUnsizedArrayPointerArithmetic(Info, E);
-// Can't verify -- trust that the user is doing the right thing (or if
-// not, trust that the caller will catch the bad behavior).
-// FIXME: Should we reject if this overflows, at least?
-Entries.back().ArrayIndex += TruncatedN;
-return;
-  }
-
-  // [expr.add]p4: For the purposes of these operators, a pointer to a
-  // nonarray object behaves the same as a pointer to the first element of
-  // an array of length one with the type of the object as its element type.
-  bool IsArray = MostDerivedPathLength == Entries.size() &&
- MostDerivedIsArrayElement;
-  uint64_t ArrayIndex =
-  IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd;
-  uint64_t ArraySize =
-  IsArray ? getMostDerivedArraySize() : (uint64_t)1;
-
-  if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) {
-// Calculate the actual index in a wide enough type, so we can include
-// it in the note.
-N = N.extend(std::max(N.getBitWidth() + 1, 65));
-(llvm::APInt&)N += ArrayIndex;
-assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index");
-diagnosePointerArithmetic(Info, E, N);
-setInvalid();
-return;
-  }
-
-  ArrayIndex += TruncatedN;
-  assert(ArrayIndex <= ArraySize &&
- "bounds check succeeded for out-of-bounds index");
-
-  if (IsArray)
-Entries.back().ArrayIndex = ArrayIndex;
-  else
-IsOnePastTheEnd = (ArrayIndex != 0);
-}
+bool adjustIndex(EvalInfo &Info, const Expr *E, APSInt N);
   };
 
   /// A stack frame in the constexpr call stack.
@@ -721,43 +679,75 @@
 bool IsSpeculativelyEvaluating;
 
 enum EvaluationMode {
-  /// Evaluate as a constant expression. Stop if we find that the expression
-  /// is not a constant expression.
+  // The various EvaluationMode kinds vary in terms of what sorts of
+  // expressions they accept.
+  //
+  // Key for the following table:
+  // * N = Expressions which are evaluable, but are not a constant
+  //   expression (according to the language rules) are OK
+  //   (keepEvaluatingAfterNonConstexpr)
+  // * S = Failure to evaluate which only occurs in expressions used for
+  //   side-effect are okay.  (keepEvaluatingAfterSideEffect)
+  // * F = Failure to evaluate is okay. (keepEvaluatingAfterFailure)
+  // * E = Eagerly evaluate certain builtins to a value which would normally
+  //   be deferred until after optimizations.
+  //
+  // +-+---+---+---+---+
+  // | | N | S | F | E |
+  // +-+---+---+---+---+
+  // |EM_ConstantExpression| . | . | . | . |
+  // |EM_ConstantFold  | Y | . | . | . |
+  // |EM_ConstantFoldUnevaluated   | Y | . | . | Y |
+  // |EM_IgnoreSideEffects | Y | Y | . | . |
+  // |EM_PotentialConstantExpression   | . | Y | Y | . |
+  // |EM_PotentialConstantExpressionUnevaluated| . | Y | Y | Y |
+  // |EM_EvaluateForOverflow   | Y | Y | Y | . |
+  // +-+---+---+---+---+
+  //
+  // TODO: Fix EM_ConstantExpression and EM_PotentialConstantExpression to
+  // also eagerly evaluate. (and then delete
+  // EM_PotentialConstantExpressionUnevaluated as a duplicate).  This will
+  // be somewhat tricky to change, because LLVM currently verifies that an
+  // expression is a constant expression (possibly strictly with
+  // EM_ConstantExpression) in Sema/*, while it evaluates the value
+  // separately in CodeGen/* with EM_ConstantF

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:12-32
 // Constant expression evaluation produces four main results:
 //
 //  * A success/failure flag indicating whether constant folding was 
successful.
 //This is the 'bool' return value used by most of the code in this file. A
 //'false' return value indicates that constant folding has failed, and any
 //appropriate diagnostic has already been produced.
 //

rsmith wrote:
> Is this comment still correct?
Yes -- the "potential" constant expression mode still has this semantic.



Comment at: clang/lib/AST/ExprConstant.cpp:682
 enum EvaluationMode {
-  /// Evaluate as a constant expression. Stop if we find that the 
expression
-  /// is not a constant expression.
+  /* The various EvaluationMode kinds vary in terms of what sorts of
+   * expressions they accept.

rsmith wrote:
> Nit: LLVM style avoids `/*...*/` comments even for long comment blocks like 
> this (https://llvm.org/docs/CodingStandards.html#comment-formatting).
Done.



Comment at: clang/lib/AST/ExprConstant.cpp:686-687
+ Key for the following table:
+ * D = Diagnostic notes (about non-constexpr, but still evaluable
+   constructs) are OK (keepEvaluatingAfterNote)
+ * S = Failure to evaluate which only occurs in expressions used for

rsmith wrote:
> I think talking about diagnostic notes here distracts from the purpose, which 
> is that this column indicates that we accept constructs that are not 
> permitted by the language mode's constant expression rules.
Reworded.



Comment at: clang/lib/AST/ExprConstant.cpp:688-690
+ * S = Failure to evaluate which only occurs in expressions used for
+   side-effect are okay.  (keepEvaluatingAfterSideEffect)
+ * F = Failure to evaluate is okay. (keepEvaluatingAfterFailure)

rsmith wrote:
> "is okay" here is misleading, I think. The point of 
> `keepEvaluatingAfter[...]` is that it's safe to stop evaluating if they 
> return false, because later evaluations can't affect the overall result.
The overall return value of the evaluation indicates whether the expression was 
evaluable under a specified set of conditions. The different modes cause 
failure under different conditions -- so I think "is okay" really _is_ what is 
meant here. In this particular case, "failure to evaluate" can still return 
success!

Not sure if some rewording would communicate that better?



Comment at: clang/lib/AST/ExprConstant.cpp:692
+ * E = Eagerly evaluate certain builtins to a value which would 
normally
+   defer until after optimizations.
+

rsmith wrote:
> defer -> be deferred?
Done.



Comment at: clang/lib/AST/ExprConstant.cpp:706-707
+
+ TODO: Fix EM_ConstantExpression and EM_PotentialConstantExpression to
+ also eagerly evaluate. (and then delete
+ EM_PotentialConstantExpressionUnevaluated as a duplicate)

rsmith wrote:
> Allowing `EM_ConstantExpression` to evaluate expressions that 
> `EM_ConstantFold` cannot evaluate would violate our invariants. We rely on 
> being able to check up front that an expression is a constant expression and 
> then later just ask for its value, and for the later query to always succeed 
> and return the same value as the earlier one. (This is one of the reasons why 
> I suggested in D52854 that we add an explicit AST representation for constant 
> expressions.)
I agree, it would right now. But I do think it should be fixed not to in the 
future. Let me reword the TODO, noting that it's not trivial.



Comment at: clang/lib/AST/ExprConstant.cpp:711
+
+  /// Evaluate as a constant expression, as per C++11-and-later constexpr
+  /// rules. Stop if we find that the expression is not a constant

rsmith wrote:
> This is not the intent. The evaluator uses the rules of the current language 
> mode. However, it doesn't enforce the C and C++98 syntactic restrictions on 
> what can appear in a constant expression, because it turns out to be cleaner 
> to check those separately rather than to interleave them with evaluation.
> 
> We should probably document this as evaluating following the evaluation (but 
> not syntactic) constant expression rules of the current language mode.
I'm not really sure what you mean to distinguish between C/C++98 "evaluation 
constant expression rules" and the definition of a C/C++98 ICE?

Perhaps you mean that if evaluation succeeds, it will have produced a value 
consistent with the rules of the current language, but that it may not produce 
the diagnostics in all cases it ought to?

AFAICT, the evaluator is designed to emit diagnostics only for the C++11 (and 
later) constant expression rules, as well as for inability to e

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53199/new/

https://reviews.llvm.org/D53199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57330: Adjust documentation for git migration.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: jlebar, rnk, mehdi_amini.
Herald added subscribers: jsji, jfb, arphaman, christof, delcypher, hiraditya, 
nhaehnle, jvesely, nemanjai, kubamracek, arsenm.
Herald added a reviewer: bollu.
Herald added a reviewer: serge-sans-paille.

This fixes most references to the paths:
 llvm.org/svn/
 llvm.org/git/
 llvm.org/viewvc/
 github.com/llvm-mirror/
 github.com/llvm-project/
 reviews.llvm.org/diffusion/

to instead point to https://github.com/llvm/llvm-project.

This is *not* a trivial substitution, because additionally, all the
checkout instructions had to be migrated to instruct users on how to
use the monorepo layout, setting LLVM_ENABLE_PROJECTS instead of
checking out various projects into various subdirectories.

I've attempted to *NOT* change any scripts here, only
documentation. The scripts will have to be addressed separately.


https://reviews.llvm.org/D57330

Files:
  clang-tools-extra/docs/clang-rename.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/Integrations.rst
  clang/.gitignore
  clang/docs/ClangPlugins.rst
  clang/docs/ClangTools.rst
  clang/docs/ControlFlowIntegrityDesign.rst
  clang/docs/InternalsManual.rst
  clang/docs/LibASTMatchersTutorial.rst
  clang/docs/LibTooling.rst
  clang/docs/Toolchain.rst
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/www/analyzer/checker_dev_manual.html
  clang/www/get_started.html
  clang/www/hacking.html
  clang/www/menu.html.incl
  compiler-rt/include/sanitizer/tsan_interface_atomic.h
  compiler-rt/lib/tsan/rtl/tsan_interface.h
  compiler-rt/www/index.html
  compiler-rt/www/menu.html.incl
  libclc/www/index.html
  libcxx/docs/BuildingLibcxx.rst
  libcxx/docs/index.rst
  libcxx/www/TS_deprecation.html
  libcxx/www/atomic_design.html
  libcxx/www/atomic_design_a.html
  libcxx/www/atomic_design_b.html
  libcxx/www/atomic_design_c.html
  libcxx/www/cxx1y_status.html
  libcxx/www/cxx1z_status.html
  libcxx/www/cxx2a_status.html
  libcxx/www/index.html
  libcxx/www/ts1z_status.html
  libcxx/www/type_traits_design.html
  libcxx/www/upcoming_meeting.html
  libcxxabi/www/index.html
  libunwind/docs/BuildingLibunwind.rst
  libunwind/docs/index.rst
  lld/docs/getting_started.rst
  lld/docs/index.rst
  lldb/docs/building-with-debug-llvm.txt
  
lldb/packages/Python/lldbsuite/test/functionalities/command_source/TestCommandSource.py
  
lldb/packages/Python/lldbsuite/test/lang/objc/objc-optimized/TestObjcOptimized.py
  lldb/utils/vim-lldb/doc/lldb.txt
  lldb/www/adding-language-support.html
  lldb/www/build.html
  lldb/www/index.html
  lldb/www/python-reference.html
  lldb/www/scripting.html
  lldb/www/sidebar.incl
  lldb/www/source.html
  lldb/www/symbolication.html
  lldb/www/varformats.html
  llgo/README.TXT
  llvm/docs/CompileCudaWithLLVM.rst
  llvm/docs/LibFuzzer.rst
  llvm/docs/TestSuiteMakefileGuide.rst
  llvm/docs/TestingGuide.rst
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/test/CodeGen/PowerPC/pr24546.ll
  llvm/utils/lit/setup.py
  openmp/www/index.html
  polly/docs/TipsAndTricks.rst
  polly/www/get_started.html
  polly/www/menu.html.incl
  polly/www/todo.html

Index: polly/www/todo.html
===
--- polly/www/todo.html
+++ polly/www/todo.html
@@ -411,12 +411,6 @@
 >http://llvm.org/svn/llvm-project/polly
  Tobias
 
-
-
- Git mirror
-
-git://llvm.org/git/polly.git
- Tobias
 
 
  Commit mails
Index: polly/www/menu.html.incl
===
--- polly/www/menu.html.incl
+++ polly/www/menu.html.incl
@@ -34,7 +34,7 @@
 http://lab.llvm.org:8080/coverage/coverage-reports/polly/index.html";>Code Coverage
 http://llvm.org/reports/scan-build/";>Static analysis
 Doxygen
-https://github.com/llvm-mirror/polly";>Source @ GitHub
+https://github.com/llvm/llvm-project/tree/master/polly";>Source @ GitHub
   
 
   
Index: polly/www/get_started.html
===
--- polly/www/get_started.html
+++ polly/www/get_started.html
@@ -33,20 +33,14 @@
  Manual 
  Get the code 
 
-Warning: Polly/LLVM/clang need to be checked out at the same time.
-
 
-git clone http://llvm.org/git/llvm.git llvm_git
-git clone http://llvm.org/git/polly.git llvm_git/tools/polly
-
-# Also build the matching clang-version (optional)
-git clone http://llvm.org/git/clang.git llvm_git/tools/clang
+git clone http://github.com/llvm/llvm-project.git llvm_git
 
 Build Polly
 
 
 mkdir llvm_build && cd llvm_build
-cmake ../llvm_git && make
+cmake -DLLVM_ENABLE_PROJECTS='polly;clang' ../llvm_git/llvm && make
 
 
  Test Polly
@@ -59,7 +53,7 @@
 build. To configure Polly to use a pre-built LLVM, set the
 -DCMAKE_PREFIX_PATH option:
 
-cmake -DCMAKE_PREFIX_PATH=${LLVM_PREFIX}/lib/cmake/llvm
+cmake -DCMAKE_PREFIX_PATH=${LLVM_PREFIX}/lib/cmake/llvm ../llvm_git/polly
 
 To run unittests,

[PATCH] D57330: Adjust documentation for git migration.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 183891.
jyknight added a comment.

Fix some warnings I added.

Restore TestSuiteMakefileGuide.rst, which apparently isn't 100%
obsolete. (But it is incorrect, and I'm not sure exactly how to fix
it, so I just left a FIXME).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57330/new/

https://reviews.llvm.org/D57330

Files:
  clang-tools-extra/docs/clang-rename.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/Integrations.rst
  clang/.gitignore
  clang/docs/ClangPlugins.rst
  clang/docs/ClangTools.rst
  clang/docs/ControlFlowIntegrityDesign.rst
  clang/docs/InternalsManual.rst
  clang/docs/LibASTMatchersTutorial.rst
  clang/docs/LibTooling.rst
  clang/docs/Toolchain.rst
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/www/analyzer/checker_dev_manual.html
  clang/www/get_started.html
  clang/www/hacking.html
  clang/www/menu.html.incl
  compiler-rt/include/sanitizer/tsan_interface_atomic.h
  compiler-rt/lib/tsan/rtl/tsan_interface.h
  compiler-rt/www/index.html
  compiler-rt/www/menu.html.incl
  libclc/www/index.html
  libcxx/docs/BuildingLibcxx.rst
  libcxx/docs/index.rst
  libcxx/www/TS_deprecation.html
  libcxx/www/atomic_design.html
  libcxx/www/atomic_design_a.html
  libcxx/www/atomic_design_b.html
  libcxx/www/atomic_design_c.html
  libcxx/www/cxx1y_status.html
  libcxx/www/cxx1z_status.html
  libcxx/www/cxx2a_status.html
  libcxx/www/index.html
  libcxx/www/ts1z_status.html
  libcxx/www/type_traits_design.html
  libcxx/www/upcoming_meeting.html
  libcxxabi/www/index.html
  libunwind/docs/BuildingLibunwind.rst
  libunwind/docs/index.rst
  lld/docs/getting_started.rst
  lld/docs/index.rst
  lldb/docs/building-with-debug-llvm.txt
  
lldb/packages/Python/lldbsuite/test/functionalities/command_source/TestCommandSource.py
  
lldb/packages/Python/lldbsuite/test/lang/objc/objc-optimized/TestObjcOptimized.py
  lldb/utils/vim-lldb/doc/lldb.txt
  lldb/www/adding-language-support.html
  lldb/www/build.html
  lldb/www/index.html
  lldb/www/python-reference.html
  lldb/www/scripting.html
  lldb/www/sidebar.incl
  lldb/www/source.html
  lldb/www/symbolication.html
  lldb/www/varformats.html
  llgo/README.TXT
  llvm/docs/CompileCudaWithLLVM.rst
  llvm/docs/LibFuzzer.rst
  llvm/docs/TestSuiteGuide.md
  llvm/docs/TestSuiteMakefileGuide.rst
  llvm/docs/TestingGuide.rst
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/test/CodeGen/PowerPC/pr24546.ll
  llvm/utils/lit/setup.py
  openmp/www/index.html
  polly/docs/TipsAndTricks.rst
  polly/www/get_started.html
  polly/www/menu.html.incl
  polly/www/todo.html

Index: polly/www/todo.html
===
--- polly/www/todo.html
+++ polly/www/todo.html
@@ -411,12 +411,6 @@
 >http://llvm.org/svn/llvm-project/polly
  Tobias
 
-
-
- Git mirror
-
-git://llvm.org/git/polly.git
- Tobias
 
 
  Commit mails
Index: polly/www/menu.html.incl
===
--- polly/www/menu.html.incl
+++ polly/www/menu.html.incl
@@ -34,7 +34,7 @@
 http://lab.llvm.org:8080/coverage/coverage-reports/polly/index.html";>Code Coverage
 http://llvm.org/reports/scan-build/";>Static analysis
 Doxygen
-https://github.com/llvm-mirror/polly";>Source @ GitHub
+https://github.com/llvm/llvm-project/tree/master/polly";>Source @ GitHub
   
 
   
Index: polly/www/get_started.html
===
--- polly/www/get_started.html
+++ polly/www/get_started.html
@@ -33,20 +33,14 @@
  Manual 
  Get the code 
 
-Warning: Polly/LLVM/clang need to be checked out at the same time.
-
 
-git clone http://llvm.org/git/llvm.git llvm_git
-git clone http://llvm.org/git/polly.git llvm_git/tools/polly
-
-# Also build the matching clang-version (optional)
-git clone http://llvm.org/git/clang.git llvm_git/tools/clang
+git clone http://github.com/llvm/llvm-project.git llvm_git
 
 Build Polly
 
 
 mkdir llvm_build && cd llvm_build
-cmake ../llvm_git && make
+cmake -DLLVM_ENABLE_PROJECTS='polly;clang' ../llvm_git/llvm && make
 
 
  Test Polly
@@ -59,7 +53,7 @@
 build. To configure Polly to use a pre-built LLVM, set the
 -DCMAKE_PREFIX_PATH option:
 
-cmake -DCMAKE_PREFIX_PATH=${LLVM_PREFIX}/lib/cmake/llvm
+cmake -DCMAKE_PREFIX_PATH=${LLVM_PREFIX}/lib/cmake/llvm ../llvm_git/polly
 
 To run unittests, however, you need to have the LLVM source directory around.
 Polly will use the llvm-config of the LLVM you're building against
Index: polly/docs/TipsAndTricks.rst
===
--- polly/docs/TipsAndTricks.rst
+++ polly/docs/TipsAndTricks.rst
@@ -47,7 +47,7 @@
 the regression.
 
 LLVM has a single repository that contains all projects. It can be cloned at:
-``_. How to bisect on a
+``_. Ho

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/Basic/DiagnosticIDs.cpp:460-463
+  // Honor -w: this disables all messages mapped to Warning severity, and 
*also*
+  // any diagnostics which are not Error/Fatal by default (that is, they were
+  // upgraded by any of the mechanisms available: -Werror, -pedantic, or 
#pragma
+  // diagnostic)

rsmith wrote:
> I think this would be clearer if phrased the other way around:
> 
> > [...] disables all messages that are not Error/Fatal by default, and also 
> > any diagnostics that are Error/Fatal by default but that have been 
> > downgraded to Warning severity by any of the mechanisms available: 
> > -Wno-error or #pragma diagnostic
Reworded.



Comment at: clang/lib/Basic/DiagnosticIDs.cpp:466
+if (Result == diag::Severity::Warning ||
+!isDefaultMappingAsError((diag::kind)DiagID))
+  return diag::Severity::Ignored;

rsmith wrote:
> I think this change will also cause `-w` to disable all remarks. Was that 
> your intent?
No, that seems like a bug.

Remarks have their own completely-separate set of command-line options; I don't 
think -R should interact with -w. I've added a conditional here, and a test 
case ensuring that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53199/new/

https://reviews.llvm.org/D53199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 183966.
jyknight marked 2 inline comments as done.
jyknight added a comment.

Fix to not disable remarks, reword comment, adjust implementation-of-module.m 
test-case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53199/new/

https://reviews.llvm.org/D53199

Files:
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/test/Frontend/optimization-remark.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Frontend/warning-mapping-4.c
  clang/test/Frontend/warning-mapping-5.c
  clang/test/Frontend/warning-mapping-6.c
  clang/test/Modules/implementation-of-module.m

Index: clang/test/Modules/implementation-of-module.m
===
--- clang/test/Modules/implementation-of-module.m
+++ clang/test/Modules/implementation-of-module.m
@@ -1,17 +1,17 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -fsyntax-only
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -dM -E -o - 2>&1 | FileCheck %s
 // CHECK-NOT: __building_module
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_left -verify
 
-// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -emit-pch -o %t.pch
-// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -DWITH_PREFIX -fmodules-ignore-macro=WITH_PREFIX -include-pch %t.pch -fmodule-implementation-of category_right
 
 #ifndef WITH_PREFIX
Index: clang/test/Frontend/warning-mapping-6.c
===
--- /dev/null
+++ clang/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: clang/test/Frontend/warning-mapping-5.c
===
--- clang/test/Frontend/warning-mapping-5.c
+++ clang/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: clang/test/Frontend/warning-mapping-4.c
===
--- clang/test/Frontend/warning-mapping-4.c
+++ clang/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 // RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
 // RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -w -Werror=sign-compare %s
 // expected-no-diagnostics
 
 int f0(int x, unsigned y) {
Index: clang/test/Frontend/warning-mapping-2.c
===
--- clang/test/Frontend/warning-mapping-2.c
+++ clang/test/Frontend/warning-mapping-2.c
@@ -1,5 +1,7 @@
-// Check that -w has lower priority than -pedantic-errors.
+// Check that -w takes precedence over -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
 
-void f0() { f1(); } // expected-error {{implicit declaration of function}}
+// Expect *not* to see a diagnostic for "implicit declaration of function"
+// expected-no-diagnostics
 
+void f0() { f1(); }
Index: clang/test/Frontend/optimization-remark.c
===
--- clang/test/Frontend/optimization-remark.c
+++ clang/test/Frontend/optimization-remark.c
@@ -13,6 +13,9 @@
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -emit-llvm -

[PATCH] D57330: Adjust documentation for git migration.

[James Y Knight via Phabricator via cfe-commits]

jyknight marked 8 inline comments as done.
jyknight added a comment.

In D57330#1375096 , @labath wrote:

> I am not sure we should be recommending to people to place the build folder 
> under the llvm-project checkout. Is that how people use the monorepo build 
> nowadays? This is not an full out-of-source build, since the build folder is 
> still a subfolder of the repo root (and without a .gitignore file containing 
> the right build folder name, git will complain about untracked files in the 
> repository)...


Well, it's certainly how I do it. I find it the least confusing, because then I 
don't get mixed up as to which source tree a given build directory belongs to 
(or directories, as I may also have build-release, build-debug, etc).




Comment at: libcxx/docs/BuildingLibcxx.rst:57
   $ cd where-you-want-libcxx-to-live
-  $ # Check out llvm, libc++ and libc++abi.
-  $ ``svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm``
-  $ ``svn co http://llvm.org/svn/llvm-project/libcxx/trunk libcxx``
-  $ ``svn co http://llvm.org/svn/llvm-project/libcxxabi/trunk libcxxabi``
+  $ # Check out the sources (includes everything, but we'll only use libcxx)
+  $ ``git clone https://github.com/llvm/llvm-project.git``

mehdi_amini wrote:
> Wonder if it is worth mentioning somewhere how to sparse-checkout?
Yes, I think it likely is worth mentioning that somewhere in the future -- but 
I'd rather not recommend it yet. There's two things that will impact that:

1. We'll need to decide where we plan to keep shared infrastructure (e.g. cmake 
macros etc) used by multiple subprojects within the repository.
2. It seems as though git is actually gaining new support for this kind of 
thing now, might be worth letting that mature a little bit.



Comment at: libcxxabi/www/index.html:86
   mkdir build && cd build
-  cmake .. # on linux you may need to prefix with CC=clang 
CXX=clang++
+  cmake -DLLVM_ENABLE_PROJECTS=libcxxabi ../llvm # on linux you may 
need to prefix with CC=clang CXX=clang++
   make

labath wrote:
> mehdi_amini wrote:
> > Do you now if prefixing with CC is equivalent to -DCMAKE_C_COMPILER?
> It usually is, but it can differ once you start using cache and toolchain 
> files. In any case, using -DCMAKE_C(XX)_COMPILER is the preferred way to do 
> things in cmake.
Changed to specify the cmake arguments.



Comment at: lld/docs/getting_started.rst:71
+ $ cd llvm-project/build (out of source build required)
+ $ cmake -G "Visual Studio 11" ../llvm
 

mehdi_amini wrote:
> Missing -DLLVM_ENABLE_PROJECTS=lld here I believe
Yep fixed.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/command_source/TestCommandSource.py:4
-
-See also http://llvm.org/viewvc/llvm-project?view=rev&revision=109673.
 """

smeenai wrote:
> Would you want to link to the corresponding GitHub commit?
After viewing said commit, I felt it wasn't actually useful to visit to 
understand this file, which is why I removed the link.



Comment at: 
lldb/packages/Python/lldbsuite/test/lang/objc/objc-optimized/TestObjcOptimized.py:4
 
-http://llvm.org/viewvc/llvm-project?rev=126973&view=rev
 Fixed a bug in the expression parser where the 'this'

smeenai wrote:
> Same here.
Felt the same in this case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57330/new/

https://reviews.llvm.org/D57330



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57330: Adjust documentation for git migration.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jyknight marked 2 inline comments as done.
Closed by commit rC352514: Adjust documentation for git migration. (authored by 
jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57330?vs=183891&id=184098#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57330/new/

https://reviews.llvm.org/D57330

Files:
  .gitignore
  docs/ClangPlugins.rst
  docs/ClangTools.rst
  docs/ControlFlowIntegrityDesign.rst
  docs/InternalsManual.rst
  docs/LibASTMatchersTutorial.rst
  docs/LibTooling.rst
  docs/Toolchain.rst
  lib/CodeGen/CGOpenMPRuntime.cpp
  www/analyzer/checker_dev_manual.html
  www/get_started.html
  www/hacking.html
  www/menu.html.incl

Index: docs/LibTooling.rst
===
--- docs/LibTooling.rst
+++ docs/LibTooling.rst
@@ -196,6 +196,6 @@
 Linking
 ^^^
 
-For a list of libraries to link, look at one of the tools' Makefiles (for
-example `clang-check/Makefile
-`_).
+For a list of libraries to link, look at one of the tools' CMake files (for
+example `clang-check/CMakeList.txt
+`_).
Index: docs/InternalsManual.rst
===
--- docs/InternalsManual.rst
+++ docs/InternalsManual.rst
@@ -1686,7 +1686,7 @@
 ^^^
 The first step to adding a new attribute to Clang is to add its definition to
 `include/clang/Basic/Attr.td
-`_.
+`_.
 This tablegen definition must derive from the ``Attr`` (tablegen, not
 semantic) type, or one of its derivatives. Most attributes will derive from the
 ``InheritableAttr`` type, which specifies that the attribute can be inherited by
@@ -1748,10 +1748,10 @@
 either ``diag::warn_attribute_wrong_decl_type`` or
 ``diag::err_attribute_wrong_decl_type``, and the parameter enumeration is found
 in `include/clang/Sema/ParsedAttr.h
-`_
+`_
 If a previously unused Decl node is added to the ``SubjectList``, the logic used
 to automatically determine the diagnostic parameter in `utils/TableGen/ClangAttrEmitter.cpp
-`_
+`_
 may need to be updated.
 
 By default, all subjects in the SubjectList must either be a Decl node defined
@@ -1773,7 +1773,7 @@
 Documentation is table generated on the public web server by a server-side
 process that runs daily. Generally, the documentation for an attribute is a
 stand-alone definition in `include/clang/Basic/AttrDocs.td 
-`_
+`_
 that is named after the attribute being documented.
 
 If the attribute is not for public consumption, or is an implicitly-created
@@ -1824,7 +1824,7 @@
 optional. The associated C++ type of the argument is determined by the argument
 definition type. If the existing argument types are insufficient, new types can
 be created, but it requires modifying `utils/TableGen/ClangAttrEmitter.cpp
-`_
+`_
 to properly support the type.
 
 Other Properties
@@ -1836,7 +1836,7 @@
 If the parsed form of the attribute is more complex, or differs from the
 semantic form, the ``HasCustomParsing`` bit can be set to ``1`` for the class,
 and the parsing code in `Parser::ParseGNUAttributeArgs()
-`_
+`_
 can be updated for the special case. Note that this only applies to arguments
 with a GNU spelling -- attributes with a __declspec spelling currently ignore
 this flag and are handled by ``Parser::ParseMicrosoftDeclSpec``.
@@ -1899,7 +1899,7 @@
 Boilerplate
 ^^^
 All semantic processing of declaration attributes happens in `lib/Sema/SemaDeclAttr.cpp
-`_,
+`_,
 and generally starts in t

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352535: Fix the behavior of clang's -w flag. (authored 
by jyknight, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53199?vs=183966&id=184142#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53199/new/

https://reviews.llvm.org/D53199

Files:
  cfe/trunk/lib/Basic/DiagnosticIDs.cpp
  cfe/trunk/test/Frontend/optimization-remark.c
  cfe/trunk/test/Frontend/warning-mapping-2.c
  cfe/trunk/test/Frontend/warning-mapping-4.c
  cfe/trunk/test/Frontend/warning-mapping-5.c
  cfe/trunk/test/Frontend/warning-mapping-6.c
  cfe/trunk/test/Modules/implementation-of-module.m

Index: cfe/trunk/test/Frontend/optimization-remark.c
===
--- cfe/trunk/test/Frontend/optimization-remark.c
+++ cfe/trunk/test/Frontend/optimization-remark.c
@@ -13,6 +13,9 @@
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
 //
+// Check that -w doesn't disable remarks.
+// RUN: %clang_cc1 %s -Rpass=inline -w -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+//
 // FIXME: -Reverything should imply -Rpass=.*.
 // RUN: %clang_cc1 %s -Reverything -emit-llvm -o - 2>/dev/null | FileCheck %s --check-prefix=CHECK-NO-REMARKS
 //
Index: cfe/trunk/test/Frontend/warning-mapping-5.c
===
--- cfe/trunk/test/Frontend/warning-mapping-5.c
+++ cfe/trunk/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: cfe/trunk/test/Frontend/warning-mapping-6.c
===
--- cfe/trunk/test/Frontend/warning-mapping-6.c
+++ cfe/trunk/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: cfe/trunk/test/Frontend/warning-mapping-4.c
===
--- cfe/trunk/test/Frontend/warning-mapping-4.c
+++ cfe/trunk/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 // RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
 // RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -w -Werror=sign-compare %s
 // expected-no-diagnostics
 
 int f0(int x, unsigned y) {
Index: cfe/trunk/test/Frontend/warning-mapping-2.c
===
--- cfe/trunk/test/Frontend/warning-mapping-2.c
+++ cfe/trunk/test/Frontend/warning-mapping-2.c
@@ -1,5 +1,7 @@
-// Check that -w has lower priority than -pedantic-errors.
+// Check that -w takes precedence over -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
 
-void f0() { f1(); } // expected-error {{implicit declaration of function}}
+// Expect *not* to see a diagnostic for "implicit declaration of function"
+// expected-no-diagnostics
 
+void f0() { f1(); }
Index: cfe/trunk/test/Modules/implementation-of-module.m
===
--- cfe/trunk/test/Modules/implementation-of-module.m
+++ cfe/trunk/test/Modules/implementation-of-module.m
@@ -1,17 +1,17 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -fsyntax-only
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -dM -E -o - 2>&1 | FileCheck %s
 // CHECK-NOT: __building_module
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_left -verify
 
-// RUN: %clang_cc1 -x

[PATCH] D57315: [opaque pointer types] Add a FunctionCallee wrapper type, and use it.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC352791: [opaque pointer types] Add a FunctionCallee wrapper 
type, and use it. (authored by jyknight, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57315?vs=183795&id=184576#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57315/new/

https://reviews.llvm.org/D57315

Files:
  lib/CodeGen/CGExpr.cpp


Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3056,7 +3056,7 @@
   bool WithDiag = !CGM.getCodeGenOpts().SanitizeTrap.has(Kind);
 
   llvm::CallInst *CheckCall;
-  llvm::Constant *SlowPathFn;
+  llvm::FunctionCallee SlowPathFn;
   if (WithDiag) {
 llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
 auto *InfoPtr =
@@ -3078,7 +3078,8 @@
 CheckCall = Builder.CreateCall(SlowPathFn, {TypeId, Ptr});
   }
 
-  CGM.setDSOLocal(cast(SlowPathFn->stripPointerCasts()));
+  CGM.setDSOLocal(
+  cast(SlowPathFn.getCallee()->stripPointerCasts()));
   CheckCall->setDoesNotThrow();
 
   EmitBlock(Cont);


Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3056,7 +3056,7 @@
   bool WithDiag = !CGM.getCodeGenOpts().SanitizeTrap.has(Kind);
 
   llvm::CallInst *CheckCall;
-  llvm::Constant *SlowPathFn;
+  llvm::FunctionCallee SlowPathFn;
   if (WithDiag) {
 llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
 auto *InfoPtr =
@@ -3078,7 +3078,8 @@
 CheckCall = Builder.CreateCall(SlowPathFn, {TypeId, Ptr});
   }
 
-  CGM.setDSOLocal(cast(SlowPathFn->stripPointerCasts()));
+  CGM.setDSOLocal(
+  cast(SlowPathFn.getCallee()->stripPointerCasts()));
   CheckCall->setDoesNotThrow();
 
   EmitBlock(Cont);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
InitPreprocessor alongside the rest of the SIZEOF macros?

And then use that to determine whether to add float128 to the union? This 
change, as is, will break on any target which is i386 but doesn't define 
__float128, e.g. i386-unknown-unknown.

The only additional target which will be modified with that (that is: the only 
other target which has a float128 type, but for which max_align isn't already 
16) is systemz-*-linux.

But I think that's actually indicating a pre-existing bug in the SystemZ config 
-- it's configured for a 16-byte long double, with 8-byte alignment, but the 
ABI doc seems to call for 16-byte alignment. +Ulrich for comment on that.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55057/new/

https://reviews.llvm.org/D55057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

uweigand wrote:
> jyknight wrote:
> > Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
> > InitPreprocessor alongside the rest of the SIZEOF macros?
> > 
> > And then use that to determine whether to add float128 to the union? This 
> > change, as is, will break on any target which is i386 but doesn't define 
> > __float128, e.g. i386-unknown-unknown.
> > 
> > The only additional target which will be modified with that (that is: the 
> > only other target which has a float128 type, but for which max_align isn't 
> > already 16) is systemz-*-linux.
> > 
> > But I think that's actually indicating a pre-existing bug in the SystemZ 
> > config -- it's configured for a 16-byte long double, with 8-byte alignment, 
> > but the ABI doc seems to call for 16-byte alignment. +Ulrich for comment on 
> > that.
> That's a bug in the ABI doc which we'll fix once we get around to releasing 
> an updated version.
> 
> long double on SystemZ must be 8-byte aligned, which is the maximum alignment 
> of all standard types on Z, and this was chosen because historically the ABI 
> only guarantees an 8-byte stack alignment, so 16-byte aligned standard types 
> would be awkward.
Then perhaps it's a bug that `__alignof__(__float128)` returns 16 with `-target 
systemz-linux`?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55057/new/

https://reviews.llvm.org/D55057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: rsmith, chandlerc.
Herald added a subscriber: arphaman.

This warning, Wexperimental-driver-option, is on by default, exempt
from -Werror, and may be disabled.

Additionally, change the documentation to note that these options are
not intended for common usage.


https://reviews.llvm.org/D55150

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3419,6 +3419,11 @@
   if (options & CXTranslationUnit_KeepGoing)
 Diags->setSuppressAfterFatalError(false);
 
+  // Suppress driver warning for use of -Xclang, since we add it ourselves.
+  Diags->setSeverityForGroup(diag::Flavor::WarningOrError,
+ "experimental-driver-option",
+ diag::Severity::Ignored, SourceLocation());
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar >
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4994,6 +4994,11 @@
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
+  if (Args.hasArg(options::OPT_Xclang))
+D.Diag(diag::warn_drv_xclang_option);
+  if (Args.hasArg(options::OPT_mllvm))
+D.Diag(diag::warn_drv_mllvm_option);
+
   // -finclude-default-header flag is for preprocessor,
   // do not pass it to other cc1 commands when save-temps is enabled
   if (C.getDriver().isSaveTempsEnabled() &&
@@ -5926,6 +5931,8 @@
   CollectArgsForIntegratedAssembler(C, Args, CmdArgs,
 getToolChain().getDriver());
 
+  if (Args.hasArg(options::OPT_mllvm))
+D.Diag(diag::warn_drv_mllvm_option);
   Args.AddAllArgs(CmdArgs, options::OPT_mllvm);
 
   assert(Output.isFilename() && "Unexpected lipo output.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -466,7 +466,7 @@
   HelpText<"Pass  to the assembler">, MetaVarName<"">,
   Group;
 def Xclang : Separate<["-"], "Xclang">,
-  HelpText<"Pass  to the clang compiler">, MetaVarName<"">,
+  HelpText<"Pass  to the clang cc1 frontend. For experimental use only">, MetaVarName<"">,
   Flags<[DriverOption, CoreOption]>, Group;
 def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">,
   HelpText<"Pass  to fatbinary invocation">, MetaVarName<"">;
@@ -2002,7 +2002,7 @@
 def mlinker_version_EQ : Joined<["-"], "mlinker-version=">,
   Flags<[DriverOption]>;
 def mllvm : Separate<["-"], "mllvm">, Flags<[CC1Option,CC1AsOption,CoreOption]>,
-  HelpText<"Additional arguments to forward to LLVM's option processing">;
+  HelpText<"Additional arguments to forward to LLVM's option processing. For experimental use only">;
 def mmacosx_version_min_EQ : Joined<["-"], "mmacosx-version-min=">,
   Group, HelpText<"Set Mac OS X deployment target">;
 def mmacos_version_min_EQ : Joined<["-"], "mmacos-version-min=">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1039,3 +1039,7 @@
 // A warning group specifically for warnings related to function
 // multiversioning.
 def FunctionMultiVersioning : DiagGroup<"function-multiversion">;
+
+// Warning for command-line options that are not a supported part of the clang command-line interface.
+// Warnings in this group should be on by default, and exempt from -Werror.
+def ExperimentalDriverOption : DiagGroup<"experimental-driver-option">;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -405,4 +405,14 @@
 def warn_drv_moutline_unsupported_opt : Warning<
   "The '%0' architecture does not support -moutline; flag ignored">,
   InGroup;
+
+def warn_drv_xclang_option : Warning<
+  "-Xclang options are for experimental use only; future compatibility may not be preserved">,
+  InGroup, DefaultWarnNoWerror;
+
+def warn_drv_mllvm_option : Warning<
+  "-mllvm options are for experimental use only; future compatibility may not be preserved">,
+  InGroup, DefaultWarnNoWerror;
+
 }
+
Index: clang/docs/UsersManual.rst
===
--- clang/docs/User

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

EricWF wrote:
> uweigand wrote:
> > jyknight wrote:
> > > uweigand wrote:
> > > > jyknight wrote:
> > > > > Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
> > > > > InitPreprocessor alongside the rest of the SIZEOF macros?
> > > > > 
> > > > > And then use that to determine whether to add float128 to the union? 
> > > > > This change, as is, will break on any target which is i386 but 
> > > > > doesn't define __float128, e.g. i386-unknown-unknown.
> > > > > 
> > > > > The only additional target which will be modified with that (that is: 
> > > > > the only other target which has a float128 type, but for which 
> > > > > max_align isn't already 16) is systemz-*-linux.
> > > > > 
> > > > > But I think that's actually indicating a pre-existing bug in the 
> > > > > SystemZ config -- it's configured for a 16-byte long double, with 
> > > > > 8-byte alignment, but the ABI doc seems to call for 16-byte 
> > > > > alignment. +Ulrich for comment on that.
> > > > That's a bug in the ABI doc which we'll fix once we get around to 
> > > > releasing an updated version.
> > > > 
> > > > long double on SystemZ must be 8-byte aligned, which is the maximum 
> > > > alignment of all standard types on Z, and this was chosen because 
> > > > historically the ABI only guarantees an 8-byte stack alignment, so 
> > > > 16-byte aligned standard types would be awkward.
> > > Then perhaps it's a bug that `__alignof__(__float128)` returns 16 with 
> > > `-target systemz-linux`?
> > Huh, really __float128 should not be supported at all on SystemZ.  It's not 
> > supported with GCC either (GCC never provides __float128 on targets where 
> > long double is already IEEE-128).  (GCC does support the new _Float128 on 
> > SystemZ, but that's not yet supported by clang anywhere.)
> > 
> > If it were supported, I agree it should be an alias for long double, and 
> > also have an alignof of 8.
> @jyknight Ack. I'll add `__SIZEOF_FLOAT128__`.
@uweigand : One of those fixes needs to land before this, so that systemz's 
max_align_t doesn't change to 16 in the meantime. I think your preference would 
be for it to be simply removed, right? Looks like the type was originally added 
in https://reviews.llvm.org/D19125 -- possibly in error, since the focus was 
x86_64.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55057/new/

https://reviews.llvm.org/D55057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54547: PTH-- Remove feature entirely-

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

Since you've already submitted a fix to Boost, 
https://github.com/boostorg/build/commit/3385fe2aa699a45e722a1013658f824b6a7c761f,
 I think this is fine to remove.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54547/new/

https://reviews.llvm.org/D54547



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1318082 , @chandlerc wrote:

> I think this should be `internal-driver-option` and the text updated? I don't 
> think these are necessarily experimental, they're internals of the compiler 
> implementation, and not a supported interface for users. Unsure how to best 
> explain this.


Yea, I was trying to figure out a good name. I thought "unsupported" at first, 
but that is a tricky word, since it has the connotation of "doesn't work in 
this version", which is not what I mean to imply -- the thing someone wants to 
do likely does work, but we need to indicate no guarantees about the future. 
Which is where "experimental" came from -- the options are useful for 
experimenting with compiler features that are not fully baked.

Perhaps "internal" is okay too...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55150/new/

https://reviews.llvm.org/D55150



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1321046 , @kristina wrote:

> Personally I'm against this type of warning as it's likely anyone using 
> `-mllvm` is actually intending to adjust certain behaviors across one or more 
> passes with a lot of switches supported by it being intentionally hidden from 
> ` --help` output requiring explicitly specifying that hidden flags 
> be shown.


There is a cost to having people encode these flags into their build systems -- 
it can then cause issues if we ever change these internal flags. I do not think 
any Clang maintainer intends to support these as stable APIs, unlike most of 
the driver command-line. But, neither -Xclang nor -mllvm obviously scream out 
"don't use this!", and so people may then add them to their buildsystems 
without causing reviewers to say "Wait...really? Are you sure that's a good 
idea?".

That's why I think a warning is useful -- it'll discourage people from using 
them when they haven't properly understand the consequences, but does not 
prevent them from doing so, when they actually do.

> For example, I routinely use the following with SEH (excuse some of the 
> naming, this is just a downstream fork however):
>  `-mllvm -target-enable-seh=true -mllvm -force-msvc-seh=true -mllvm 
> -wtfabi-opts=0x1EF77F`

If you already are passing that, do you see a problem with instead passing
 `-mllvm -target-enable-seh=true -mllvm -force-msvc-seh=true -mllvm 
-wtfabi-opts=0x1EF77F -Wno-experimental-driver-option`
?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55150/new/

https://reviews.llvm.org/D55150



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1321705 , @kristina wrote:

> > There is a cost to having people encode these flags into their build 
> > systems -- it can then cause issues if we ever change these internal flags. 
> > I do not think any Clang maintainer intends to support these as stable 
> > APIs, unlike most of the driver command-line. But, neither -Xclang nor 
> > -mllvm obviously scream out "don't use this!", and so people may then add 
> > them to their buildsystems without causing reviewers to say "Wait...really? 
> > Are you sure that's a good idea?".
>
> This is why I proposed a compromise, allow this warning to be disabled 
> completely for people actively using those flags, which are pretty much 
> exclusively toolchain developers (well basically what I proposed, since it's 
> not clear what counts as a build made by someone who is working and debugging 
> a pass, being fully aware of those flags, using the subset of them specific 
> to that pass/feature, I would say assertion+dump enabled builds are closest, 
> but having an explicit build flag for it would be nicer). It's more unified 
> than everyone either adding workarounds into build systems or disabling it 
> completely (by just removing it).


I mean, I'm not much opposed to adding that -- just that any new build-time 
options is always a minor shame. But you didn't respond to the other part of my 
message -- is adding `-Wno-experimental-driver-option` to your compile-flags 
not already a completely sufficient solution for your use-case?

> Besides, I don't think this really ever surfaced as an issue, until 
> Chandler's idea with regards to an unsupressable warning for performance 
> tests for 7.x.x

The primary impetus for me was actually the discovery that Boost's build system 
was using "-Xclang -include-pth" a few weeks back.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55150/new/

https://reviews.llvm.org/D55150



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1321759 , @george.karpenkov 
wrote:

> Using `-Xclang` is the only way to pass options to the static analyzer, I 
> don't think we should warn on it.


Well,, that seems unfortunate if we have the only supported interface for the 
static analyzer be an internal interface. Perhaps it can be given a different 
option? Even discounting this change, I that seems like it would be appropriate.

In D55150#1321771 , @arphaman wrote:

> Swift uses `-Xclang` to pass in build settings to its own build and to pass 
> in custom options through its Clang importer that we intentionally don't want 
> to expose to Clang's users. We don't want to warn for those uses for sure.


I'm not sure if I understand correctly, so I'll try to reiterate: Swift calls 
clang internally, and when it does so, intentionally uses options that are not 
generally intended for others to use. Of course we shouldn't emit warnings in 
that case.

Is that a correct understanding? If so, doesn't it just make sense for that 
constructed command-line to disable the warning? And, isn't it good that it 
will warn, otherwise, in order to discourage people from using those flags that 
are intentionally-not-exposed?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55150/new/

https://reviews.llvm.org/D55150



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1321829 , @efriedma wrote:

> I'm not sure that putting a warning that can be disabled really helps here; 
> anyone who needs the option will just disable the warning anyway, and then 
> users adding additional options somewhere else in the build system will miss 
> the warning.
>
> Instead, it would probably be better to rename Xclang and mllvm to something 
> that makes it clear the user is doing something unsupported. Maybe 
> "--unstable-llvm-option" and "--unstable-clang-option" or something like 
> that.  (This will lead to some breakage, but the breakage is roughly 
> equivalent for anyone using -Werror.)


Possibly you missed that this flag is exempt from -Werror -- it _only_ prints a 
warning, and is not an error, even when -Werror is specified.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55150/new/

https://reviews.llvm.org/D55150



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D54355#1327237 , @craig.topper 
wrote:

> Here's the test case that we have https://reviews.llvm.org/P8123   gcc seems 
> to accept it at O0


Smaller test case:

  extern unsigned long long __sdt_unsp;
  
  void foo() {
__asm__ __volatile__(
"" :: "n"( (__builtin_constant_p(__sdt_unsp) || 0) ? 1 : -1));
  }


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54355/new/

https://reviews.llvm.org/D54355



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

The intricate initialization-order workarounds apparently don't work in all 
build modes, so I've updated this code to have constexpr functions and 
initializations in r355278.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57914/new/

https://reviews.llvm.org/D57914



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Hm -- in GCC, -fpermissive has nothing at all to do with 
-pedantic/-pedantic-errors, but I suppose it should be fine to do this way.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58154/new/

https://reviews.llvm.org/D58154



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

The errors disabled by this feature are default-error warnings -- you can 
already get the same effect by using -Wno-. Why is it bad to 
additionally allow -fpermissive to disable them? (If we have any diagnostics 
which are currently default-on-warnings which should not _ever_ be 
disable-able, then maybe we should just fix those?)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58154/new/

https://reviews.llvm.org/D58154



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ah -- I now understand your concern, and managing user expectations 
appropriately does seem like a potential concern.

I did not look too closely before into what exactly GCC categorized as 
"permerror" (errors that can be disabled with -fpermissive) vs what Clang 
categorized as "ExtWarn,DefaultError" diagnostics. Looking into it a little 
more now, I think there may not actually be substantial overlap between the two 
right now.

Many errors we have warning flags for are unconditional in GCC, and many that 
gcc allows disabling with -fpermissive are unconditional errors in clang.

You gave examples of the latter already, and e.g. here's a bunch of bogus code 
which you can currently build with -Wno-everything in clang, but most of which 
cannot be disabled in gcc (and I believe most if not all of these flags are 
ExtWarn, so would be disabled by this proposed -fpermissive):
https://godbolt.org/z/81NkJQ

Given that, yeah, it may not actually make sense to call this behavior 
-fpermissive.

I also can't really tell what the intent is behind classifying some diagnostics 
in Clang as ExtWarn,DefaultError and others Warning,DefaultError -- or if there 
even is any useful purpose there at all. I note with special-puzzlement the 
existence of both `ext_return_missing_expr` and `warn_return_missing_expr`.

I guess my feeling now is that perhaps we should just eliminate that 
categorization as meaningless, and just make -Wno-error=everything work 
properly (for anyone who wants to not abort the compile on broken code, but for 
whatever reason also loves to see build-spam so doesn't want -Wno-everything).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58154/new/

https://reviews.llvm.org/D58154



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Sorry, forgot to re-ping this in a timely manner. :)

The discussion on mailing list concluded positively, so waiting for an LG here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.

Looks good.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59346/new/

https://reviews.llvm.org/D59346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356789: IR: Support parsing numeric block ids, and emit them 
in textual output. (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58548?vs=187994&id=191913#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548

Files:
  test/CodeGenCXX/discard-name-values.cpp


Index: test/CodeGenCXX/discard-name-values.cpp
===
--- test/CodeGenCXX/discard-name-values.cpp
+++ test/CodeGenCXX/discard-name-values.cpp
@@ -10,7 +10,7 @@
   // CHECK: br i1 %pred, label %if.then, label %if.end
 
   if (pred) {
-// DISCARDVALUE: ; :2:
+// DISCARDVALUE: 2:
 // DISCARDVALUE-NEXT: tail call void @branch()
 // DISCARDVALUE-NEXT: br label %3
 
@@ -20,7 +20,7 @@
 branch();
   }
 
-  // DISCARDVALUE: ; :3:
+  // DISCARDVALUE: 3:
   // DISCARDVALUE-NEXT: ret i1 %0
 
   // CHECK: if.end:


Index: test/CodeGenCXX/discard-name-values.cpp
===
--- test/CodeGenCXX/discard-name-values.cpp
+++ test/CodeGenCXX/discard-name-values.cpp
@@ -10,7 +10,7 @@
   // CHECK: br i1 %pred, label %if.then, label %if.end
 
   if (pred) {
-// DISCARDVALUE: ; :2:
+// DISCARDVALUE: 2:
 // DISCARDVALUE-NEXT: tail call void @branch()
 // DISCARDVALUE-NEXT: br label %3
 
@@ -20,7 +20,7 @@
 branch();
   }
 
-  // DISCARDVALUE: ; :3:
+  // DISCARDVALUE: 3:
   // DISCARDVALUE-NEXT: ret i1 %0
 
   // CHECK: if.end:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: rsmith, arphaman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This case should emit an -Wimplicit-function-declaration warning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59711

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/warn-strict-prototypes.c


Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-verify %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-Wno-implicit-function-declaration -verify %s
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // function declaration with unspecified params
@@ -71,3 +71,9 @@
 // rdar://problem/33251668
 void foo13(...) __attribute__((overloadable));
 void foo13(...) __attribute__((overloadable)) {}
+
+// We should not generate a strict-prototype warning for an implicit
+// declaration.  Leave that up to the implicit-function-declaration warning.
+void foo14(void) {
+  foo14_call(); // no-warning
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4995,7 +4995,10 @@
 break;
   case DeclaratorChunk::Function: {
 const DeclaratorChunk::FunctionTypeInfo &FTI = DeclType.Fun;
-if (FTI.NumParams == 0 && !FTI.isVariadic)
+// We supress the warning when there's no LParen location, as this
+// indicates the declaration was an implicit declaration, which gets
+// warned about separately via -Wimplicit-function-declaration.
+if (FTI.NumParams == 0 && !FTI.isVariadic && 
FTI.getLParenLoc().isValid())
   S.Diag(DeclType.Loc, diag::warn_strict_prototypes)
   << IsBlock
   << FixItHint::CreateInsertion(FTI.getRParenLoc(), "void");


Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -verify %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // function declaration with unspecified params
@@ -71,3 +71,9 @@
 // rdar://problem/33251668
 void foo13(...) __attribute__((overloadable));
 void foo13(...) __attribute__((overloadable)) {}
+
+// We should not generate a strict-prototype warning for an implicit
+// declaration.  Leave that up to the implicit-function-declaration warning.
+void foo14(void) {
+  foo14_call(); // no-warning
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4995,7 +4995,10 @@
 break;
   case DeclaratorChunk::Function: {
 const DeclaratorChunk::FunctionTypeInfo &FTI = DeclType.Fun;
-if (FTI.NumParams == 0 && !FTI.isVariadic)
+// We supress the warning when there's no LParen location, as this
+// indicates the declaration was an implicit declaration, which gets
+// warned about separately via -Wimplicit-function-declaration.
+if (FTI.NumParams == 0 && !FTI.isVariadic && FTI.getLParenLoc().isValid())
   S.Diag(DeclType.Loc, diag::warn_strict_prototypes)
   << IsBlock
   << FixItHint::CreateInsertion(FTI.getRParenLoc(), "void");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59509: Make static constructors + destructors minsize + cold (except for in -O0)

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Looks reasonable to me.




Comment at: clang/test/CodeGen/static-attr.cpp:4
+
+// WITHOUT-NOT: cold minsize noinline
+// WITH: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]]]

lebedev.ri wrote:
> This is fragile, it may have false-negative if they appear in other order.
I think we guarantee it's emitted in the same order, but it's certainly the 
case that nobody will ever notice if this is failing to fail the test, if other 
attributes get emitted in between in the future.

You can use 3 separate WITHOUT-NOT lines, instead, to avoid both of this kind 
of issue.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59509/new/

https://reviews.llvm.org/D59509



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.
Herald added a subscriber: dexonsmith.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59711/new/

https://reviews.llvm.org/D59711



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lld/ELF/InputFiles.cpp:402
+  if (Config->DepLibs)
+if (fs::exists(Specifier))
+  Driver->addFile(Specifier, /*WithLOption=*/false);

bd1976llvm wrote:
> jyknight wrote:
> > bd1976llvm wrote:
> > > jyknight wrote:
> > > > This bit seems unfortunate. "-lfoo" won't search for a file named 
> > > > "foo", and having this code do so does not seem like a good idea. I'd 
> > > > want this feature to work just like -l.
> > > > 
> > > > Actually, I think it'd be better to preserve the "-l" prefix in the 
> > > > deplibs section. (I'd note that even if libs in the search path are 
> > > > spelled "-lfoo", that doesn't mean we need accept any other options)
> > > > 
> > > > If we want to support loading a filename which not on the search path 
> > > > (but...do we, even? That seems like a kinda scary feature to me...), 
> > > > then it'd be obvious how to spell that: "libfoo.a" and "/baz/bar/foo.a" 
> > > > would open files directly, vs "-lfoo" and "-l:foo.a" would search for 
> > > > them on the search path.
> > > What you  have described is the behavior of the INPUT linker script 
> > > command:  
> > > https://sourceware.org/binutils/docs/ld/File-Commands.html#File-Commands.
> > > 
> > > We have carefully designed this "dependent libraries" feature to avoid 
> > > mapping "comment lib" pragmas to --library command line options. My 
> > > reasons:
> > > 
> > > - I don't want the compiler doing string processing to try to satisfy the 
> > > command line parsing behaviour of the target linker.
> > > 
> > > - I don't want to couple the source code to a GNU-style linker. After all 
> > > there are other ELF linkers. Your proposal isn't merely an ELF linking 
> > > proposal, it's a proposal for ELF toolchains with GNU-like linkers (e.g. 
> > > the arm linker doesn't support the colon prefix 
> > > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474c/Cjahbdei.html).
> > > 
> > > - The syntax is #pragma comment(lib, ...) not #pragma 
> > > linker-option(library, ...) i.e. the only thing this (frankly rather 
> > > bizarre) syntax definitely implies is that the argument is related to 
> > > libraries (and comments ¯\_(ツ)_/¯); it is a bit of a stretch to interpret 
> > > "comment lib" pragmas as mapping directly to "specifying an additional 
> > > --library command line option".
> > > 
> > > - I want to avoid GNUism's and use a "general" mechanism. MSVC source 
> > > code compatibility is a usecase.
> > > 
> > > - It is important to support loading a filename which not on the search 
> > > path. For example I have seen developers use the following: #pragma 
> > > comment(lib, __FILE__"\\..\\foo.lib")
> > > 
> > > I would like the following to work on for ELF:
> > > 
> > > #pragma comment(lib, "foo") => add libfoo.a to the link.
> > > #pragma comment(lib, "foo.lib") => add foo.lib to the link
> > > #pragma comment(lib, "c:\\foo.lib") => add c:\foo.lib to the link
> > > 
> > > The way the code is now these will work on both LLD and MSVC (and I think 
> > > it will work for Apple's linker as well although I haven't checked). In 
> > > addition, we have been careful to come up with a design that can be 
> > > implemented by the GNU linkers... with the cost that some complicated 
> > > links will not be possible to do via autolinking; in these (IMO rare) 
> > > cases developers will need to use the command line directly instead.
> > > 
> > > What I am trying to accomplish is to try to keep #pragma comment(lib, 
> > > "foo") compatible across linkers. Otherwise we will be in a situation 
> > > where you will have to have the equivalent of...
> > > 
> > > #ifdef COFF_LIBRARIES:
> > > #pragma comment(lib, "/DEFAULTLIB:foo.lib")
> > > #'elif ELF_LIBRARIES_GNU_STYLE_LINKER:
> > > #pragma comment(lib, "-lfoo")
> > > #'elif DARWIN_FRAMEWORKS:
> > > #pragma comment(lib, "-framework foo")
> > > #esle
> > > #error Please specify linking model
> > > #endif
> > > 
> > > ... in your source code (or the moral equivalent somewhere else). We can 
> > > avoid this.
> > > 
> > > Also note that I am not proposing to remove the .linker-options 
> > > extension. We can add support for .linker-options to LLD in the future if 
> > > we find a usecase where developers want pragmas that map directly to the 
> > > linkers command line options for ELF. If this is a usecase then, in the 
> > > future, we can implement support for #pragma comment(linker, ) 
> > > pragmas that lower to .linker-options; #pragma comment(lib, ...) pragmas 
> > > can continue to lower to .deplibs.
> > It just seems to me a really bad idea to have a situation where specifying 
> > `#pragma comment(lib, "foo")` can either open a file named "foo" in the 
> > current directory, or search for libfoo.{a,so} in the library search path.
> > 
> > That is pretty much guaranteed to cause problems due to unintentional 
> > filename collision in the current-directory.
> > 
> Linkers al

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59711/new/

https://reviews.llvm.org/D59711



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

There shouldn't be an empty string ("") in the asm output -- that should be a 
reference to the "l_yes" label, not the empty string. That seems very weird...

Even odder: running clang -S on the above without -fno-integrated-as emits a 
".quad .Ltmp00" (note the extra "0"!), while with -fno-integrated-as, it 
properly refers to ".Ltmp0".


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56571/new/

https://reviews.llvm.org/D56571



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360084: PR41183: Don't emit strict-prototypes warning 
for an implicit function (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59711?vs=191924&id=198341#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59711/new/

https://reviews.llvm.org/D59711

Files:
  lib/Sema/SemaType.cpp
  test/Sema/warn-strict-prototypes.c


Index: test/Sema/warn-strict-prototypes.c
===
--- test/Sema/warn-strict-prototypes.c
+++ test/Sema/warn-strict-prototypes.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-verify %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-Wno-implicit-function-declaration -verify %s
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // function declaration with unspecified params
@@ -71,3 +71,9 @@
 // rdar://problem/33251668
 void foo13(...) __attribute__((overloadable));
 void foo13(...) __attribute__((overloadable)) {}
+
+// We should not generate a strict-prototype warning for an implicit
+// declaration.  Leave that up to the implicit-function-declaration warning.
+void foo14(void) {
+  foo14_call(); // no-warning
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5010,7 +5010,10 @@
 break;
   case DeclaratorChunk::Function: {
 const DeclaratorChunk::FunctionTypeInfo &FTI = DeclType.Fun;
-if (FTI.NumParams == 0 && !FTI.isVariadic)
+// We supress the warning when there's no LParen location, as this
+// indicates the declaration was an implicit declaration, which gets
+// warned about separately via -Wimplicit-function-declaration.
+if (FTI.NumParams == 0 && !FTI.isVariadic && 
FTI.getLParenLoc().isValid())
   S.Diag(DeclType.Loc, diag::warn_strict_prototypes)
   << IsBlock
   << FixItHint::CreateInsertion(FTI.getRParenLoc(), "void");


Index: test/Sema/warn-strict-prototypes.c
===
--- test/Sema/warn-strict-prototypes.c
+++ test/Sema/warn-strict-prototypes.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -verify %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // function declaration with unspecified params
@@ -71,3 +71,9 @@
 // rdar://problem/33251668
 void foo13(...) __attribute__((overloadable));
 void foo13(...) __attribute__((overloadable)) {}
+
+// We should not generate a strict-prototype warning for an implicit
+// declaration.  Leave that up to the implicit-function-declaration warning.
+void foo14(void) {
+  foo14_call(); // no-warning
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5010,7 +5010,10 @@
 break;
   case DeclaratorChunk::Function: {
 const DeclaratorChunk::FunctionTypeInfo &FTI = DeclType.Fun;
-if (FTI.NumParams == 0 && !FTI.isVariadic)
+// We supress the warning when there's no LParen location, as this
+// indicates the declaration was an implicit declaration, which gets
+// warned about separately via -Wimplicit-function-declaration.
+if (FTI.NumParams == 0 && !FTI.isVariadic && FTI.getLParenLoc().isValid())
   S.Diag(DeclType.Loc, diag::warn_strict_prototypes)
   << IsBlock
   << FixItHint::CreateInsertion(FTI.getRParenLoc(), "void");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

Previously, it had been using CK_BitCast even for casts that only
change const/restrict/volatile. Now it will use CK_Noop where
appropriate.

This is an alternate solution to r336746.


https://reviews.llvm.org/D52918

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5846,7 +5846,9 @@
   // pointers.  Everything else should be possible.
 
   QualType SrcTy = Src.get()->getType();
-  if (Context.hasSameUnqualifiedType(SrcTy, DestTy))
+  // We can use a no-op cast if the types are the same, or only adding/removing
+  // const, volatile, or restricted qualifiers.
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
 return CK_NoOp;
 
   switch (Type::ScalarTypeKind SrcKind = SrcTy->getScalarTypeKind()) {
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5861,11 +5861,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5846,7 +5846,9 @@
   // pointers.  Everything else should be possible.
 
   QualType SrcTy = Src.get()->getType();
-  if (Context.hasSameUnqualifiedType(SrcTy, DestTy))
+  // We can use a no-op cast if the types are the same, or only adding/removing
+  // const, volatile, or restricted qualifiers.
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
 return CK_NoOp;
 
   switch (Type::ScalarTypeKind SrcKind = SrcTy->getScalarTypeKind()) {
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5861,11 +5861,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52919: Emit diagnostic note when calling an invalid function declaration.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

The comment said it was intentionally not emitting any diagnostic
because the declaration itself was already diagnosed. However,
everywhere else that wants to not emit a diagnostic without an extra
note emits note_invalid_subexpr_in_const_expr instead, which gets
suppressed later.

This was the only place which did not emit a diagnostic note.


https://reviews.llvm.org/D52919

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/enable_if.cpp


Index: clang/test/SemaCXX/enable_if.cpp
===
--- clang/test/SemaCXX/enable_if.cpp
+++ clang/test/SemaCXX/enable_if.cpp
@@ -414,7 +414,8 @@
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a 
constant expression}} expected-error@-2{{no matching function for call to 
'templated'}} expected-note{{in instantiation of function template}} 
expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the 
problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant 
expression}} expected-error@-3{{no matching function for call to 'templated'}} 
expected-note{{in instantiation of function template}} 
expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4330,10 +4330,13 @@
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&


Index: clang/test/SemaCXX/enable_if.cpp
===
--- clang/test/SemaCXX/enable_if.cpp
+++ clang/test/SemaCXX/enable_if.cpp
@@ -414,7 +414,8 @@
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-2{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4330,10 +4330,13 @@
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

And, since EM_OffsetFold is now unused, remove it.

While builtin_object_size intends to ignore the presence of
side-effects in its argument, the EM_OffsetFold mode was NOT
configured to ignore side-effects. Rather it was effectively identical
to EM_ConstantFold -- its explanatory comment
notwithstanding.

However, currently, keepEvaluatingAfterSideEffect() is not always
honored -- sometimes evaluation continues despite it returning
false. Therefore, since the b_o_s code was only checking the return
value from evaluation, and not additionally checking the
HasSideEffects flag, side-effects _were_ in many cases actually being
ignored.

This change is a prerequisite cleanup towards fixing that issue.


https://reviews.llvm.org/D52924

Files:
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -759,18 +759,6 @@
   /// context we try to fold them immediately since the optimizer never
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
-
-  /// Evaluate as a constant expression. In certain scenarios, if:
-  /// - we find a MemberExpr with a base that can't be evaluated, or
-  /// - we find a variable initialized with a call to a function that has
-  ///   the alloc_size attribute on it
-  /// then we may consider evaluation to have succeeded.
-  ///
-  /// In either case, the LValue returned shall have an invalid base; in 
the
-  /// former, the base will be the invalid MemberExpr, in the latter, the
-  /// base will be either the alloc_size CallExpr or a CastExpr wrapping
-  /// said CallExpr.
-  EM_OffsetFold,
 } EvalMode;
 
 /// Are we checking whether the expression is a potential constant
@@ -874,7 +862,6 @@
   case EM_PotentialConstantExpression:
   case EM_ConstantExpressionUnevaluated:
   case EM_PotentialConstantExpressionUnevaluated:
-  case EM_OffsetFold:
 HasActiveDiagnostic = false;
 return OptionalDiagnostic();
   }
@@ -966,7 +953,6 @@
   case EM_ConstantExpression:
   case EM_ConstantExpressionUnevaluated:
   case EM_ConstantFold:
-  case EM_OffsetFold:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -985,7 +971,6 @@
   case EM_EvaluateForOverflow:
   case EM_IgnoreSideEffects:
   case EM_ConstantFold:
-  case EM_OffsetFold:
 return true;
 
   case EM_PotentialConstantExpression:
@@ -1021,7 +1006,6 @@
   case EM_ConstantExpressionUnevaluated:
   case EM_ConstantFold:
   case EM_IgnoreSideEffects:
-  case EM_OffsetFold:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -1093,18 +1077,18 @@
 }
   };
 
-  /// RAII object used to treat the current evaluation as the correct pointer
-  /// offset fold for the current EvalMode
-  struct FoldOffsetRAII {
+  /// RAII object used to set the current evaluation mode to ignore
+  /// side-effects.
+  struct IgnoreSideEffectsRAII {
 EvalInfo &Info;
 EvalInfo::EvaluationMode OldMode;
-explicit FoldOffsetRAII(EvalInfo &Info)
+explicit IgnoreSideEffectsRAII(EvalInfo &Info)
 : Info(Info), OldMode(Info.EvalMode) {
   if (!Info.checkingPotentialConstantExpression())
-Info.EvalMode = EvalInfo::EM_OffsetFold;
+Info.EvalMode = EvalInfo::EM_IgnoreSideEffects;
 }
 
-~FoldOffsetRAII() { Info.EvalMode = OldMode; }
+~IgnoreSideEffectsRAII() { Info.EvalMode = OldMode; }
   };
 
   /// RAII object used to optionally suppress diagnostics and side-effects from
@@ -8049,7 +8033,7 @@
 // If there are any, but we can determine the pointed-to object anyway, 
then
 // ignore the side-effects.
 SpeculativeEvaluationRAII SpeculativeEval(Info);
-FoldOffsetRAII Fold(Info);
+IgnoreSideEffectsRAII Fold(Info);
 
 if (E->isGLValue()) {
   // It's possible for us to be given GLValues if we're called via
@@ -8117,7 +8101,6 @@
 case EvalInfo::EM_ConstantFold:
 case EvalInfo::EM_EvaluateForOverflow:
 case EvalInfo::EM_IgnoreSideEffects:
-case EvalInfo::EM_OffsetFold:
   // Leave it to IR generation.
   return Error(E);
 case EvalInfo::EM_ConstantExpressionUnevaluated:


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -759,18 +759,6 @@
   /// context we try to fold them immediately since the optimizer never
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
-
-  /// Evaluate as a constant expression. In certain scenarios, if:
-  /// - we find a MemberExpr with a base that can't be evaluated, or
-

[PATCH] D52926: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

And, configure it to explicitly request equivalent behavior to
ConstantFold, instead of getting that behavior accidentally.

While it seems that diagnose_if and enable_if were intended to require
a constexpr argument, the code was actually only checking whether it
could be constant-folded, due to the confusing way the constant
evaluator code currently works.

It _probably_ should be fixed to actually check for
constexpr-ness. However, r290584 now depends on it NOT doing so -- and
it's possible that some users are, too.

I'd like to land this, to unblock the follow-on cleanup -- without
changing the semantics first -- and maybe loop back to convert it to
require a constexpr later, if it looks like users aren't depending on
the semantics it has now.

This change is a prerequisite for fixing the confusion in the
ExprConstant code which led to this situation.


https://reviews.llvm.org/D52926

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/enable_if.c

Index: clang/test/Sema/enable_if.c
===
--- clang/test/Sema/enable_if.c
+++ clang/test/Sema/enable_if.c
@@ -170,4 +170,24 @@
   variadic_enable_if(0, m); // expected-error{{no matching}}
   variadic_enable_if(0, m, 3); // expected-error{{no matching}}
 }
+
+// Test of the current weird semantics of enable_if (and diagnose_if).
+//
+// There's two kinds of checking of the expression going on:
+//  1. Whether it could potentially be a constexpr -- without access to the
+// parameters.
+//  2. Checking if the condition evaluates to true with concrete
+// arguments. While #1 requires that it could potentially be a constexpr,
+// this step does not require that it actually _IS_ a constexpr, only that
+// it's evaluable! (Note: this looseness might be unintended)
+
+void non_constexpr() __attribute__((enable_if((int*)(char*)0 == 0, ""))); // expected-error{{'enable_if' attribute expression never produces a constant expression}} expected-note{{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
+void non_constexpr_call() {
+non_constexpr();
+}
+
+void non_constexpr_conditional(long x) __attribute__((enable_if(x?1: (int*)(char*)0 == 0, "")));
+void non_constexpr_conditional_call() {
+non_constexpr_conditional(0);
+}
 #endif
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -744,12 +744,11 @@
   /// can't be modeled.
   EM_IgnoreSideEffects,
 
-  /// Evaluate as a constant expression. Stop if we find that the expression
-  /// is not a constant expression. Some expressions can be retried in the
+  /// Evaluate as a constant fold. Some expressions can be retried in the
   /// optimizer if we don't constant fold them here, but in an unevaluated
-  /// context we try to fold them immediately since the optimizer never
-  /// gets a chance to look at it.
-  EM_ConstantExpressionUnevaluated,
+  /// context we try to fold them immediately since the optimizer never gets
+  /// a chance to look at it.
+  EM_ConstantFoldUnevaluated,
 
   /// Evaluate as a potential constant expression. Keep going if we hit a
   /// construct that we can't evaluate yet (because we don't yet know the
@@ -854,13 +853,13 @@
   case EM_ConstantFold:
   case EM_IgnoreSideEffects:
   case EM_EvaluateForOverflow:
+  case EM_ConstantFoldUnevaluated:
 if (!HasFoldFailureDiagnostic)
   break;
 // We've already failed to fold something. Keep that diagnostic.
 LLVM_FALLTHROUGH;
   case EM_ConstantExpression:
   case EM_PotentialConstantExpression:
-  case EM_ConstantExpressionUnevaluated:
   case EM_PotentialConstantExpressionUnevaluated:
 HasActiveDiagnostic = false;
 return OptionalDiagnostic();
@@ -951,7 +950,7 @@
 return true;
 
   case EM_ConstantExpression:
-  case EM_ConstantExpressionUnevaluated:
+  case EM_ConstantFoldUnevaluated:
   case EM_ConstantFold:
 return false;
   }
@@ -971,12 +970,12 @@
   case EM_EvaluateForOverflow:
   case EM_IgnoreSideEffects:
   case EM_ConstantFold:
+  case EM_ConstantFoldUnevaluated:
 return true;
 
   case EM_PotentialConstantExpression:
   case EM_PotentialConstantExpressionUnevaluated:
   case EM_ConstantExpression:
-  case EM_ConstantExpressionUnevaluated:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -1003,7 +1002,7 @@
 return true;
 
   case EM_ConstantExpression:
-  case EM_ConstantExpressionUnevaluated:
+  case EM_ConstantFoldUnevaluated:
   case EM_ConstantFold:
   case EM_IgnoreSideEffects:

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D52918#1256420, @aaron.ballman wrote:

> Patch is missing tests -- perhaps you could dump the AST and check the 
> casting notation from the output?


It would appear that which casts get emitted in C mode is almost completely 
untested. I added tests just for this change in a new file, and left a TODO to 
fill it out for the remainder of those functions.


https://reviews.llvm.org/D52918



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 168477.
jyknight added a comment.

Added test.


https://reviews.llvm.org/D52918

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/c-casts.c


Index: clang/test/Sema/c-casts.c
===
--- /dev/null
+++ clang/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5861,11 +5861,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();


Index: clang/test/Sema/c-casts.c
===
--- /dev/null
+++ clang/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSTyp

[PATCH] D52919: Emit diagnostic note when calling an invalid function declaration.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343867: Emit diagnostic note when calling an invalid 
function declaration. (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52919?vs=168417&id=168490#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52919

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/enable_if.cpp


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4330,10 +4330,13 @@
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&
Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -414,7 +414,8 @@
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a 
constant expression}} expected-error@-2{{no matching function for call to 
'templated'}} expected-note{{in instantiation of function template}} 
expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the 
problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant 
expression}} expected-error@-3{{no matching function for call to 'templated'}} 
expected-note{{in instantiation of function template}} 
expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4330,10 +4330,13 @@
   Declaration->isConstexpr())
 return false;
 
-  // Bail out with no diagnostic if the function declaration itself is invalid.
-  // We will have produced a relevant diagnostic while parsing it.
-  if (Declaration->isInvalidDecl())
+  // Bail out if the function declaration itself is invalid.  We will
+  // have produced a relevant diagnostic while parsing it, so just
+  // note the problematic sub-expression.
+  if (Declaration->isInvalidDecl()) {
+Info.FFDiag(CallLoc, diag::note_invalid_subexpr_in_const_expr);
 return false;
+  }
 
   // Can we evaluate this function call?
   if (Definition && Definition->isConstexpr() &&
Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -414,7 +414,8 @@
 
 template  constexpr int callTemplated() { return templated(); }
 
-constexpr int B = callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-2{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-9{{candidate disabled}}
+constexpr int B = 10 + // the carat for the error should be pointing to the problematic call (on the next line), not here.
+callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-3{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-10{{candidate disabled}}
 static_assert(callTemplated<1>() == 1, "");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

The constant evaluation now returns false whenever indicating failure
would be appropriate for the requested mode, instead of returning
"true" for success, and depending on the caller examining the various
status variables after the fact, in some circumstances.

In EM_ConstantExpression mode, evaluation is aborted as soon as a
diagnostic note is generated, and therefore, a "true" return value now
actually indicates that the expression was a constexpr. (You no longer
have to additionally check for a lack of diagnostic messages.)

In EM_ConstantFold, evaluation is now aborted upon running into a
side-effect -- so a "true" return value now actually indicates that
there were no side-effects

Also -- update and clarify documentation for the evaluation modes.

This is a cleanup, not intending to change the functionality, as
callers had been checking those properties manually before.


https://reviews.llvm.org/D52939

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ExprConstant.cpp

Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -412,52 +412,10 @@
   MostDerivedArraySize = 2;
   MostDerivedPathLength = Entries.size();
 }
-void diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, const Expr *E);
 void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
const APSInt &N);
 /// Add N to the address of this subobject.
-void adjustIndex(EvalInfo &Info, const Expr *E, APSInt N) {
-  if (Invalid || !N) return;
-  uint64_t TruncatedN = N.extOrTrunc(64).getZExtValue();
-  if (isMostDerivedAnUnsizedArray()) {
-diagnoseUnsizedArrayPointerArithmetic(Info, E);
-// Can't verify -- trust that the user is doing the right thing (or if
-// not, trust that the caller will catch the bad behavior).
-// FIXME: Should we reject if this overflows, at least?
-Entries.back().ArrayIndex += TruncatedN;
-return;
-  }
-
-  // [expr.add]p4: For the purposes of these operators, a pointer to a
-  // nonarray object behaves the same as a pointer to the first element of
-  // an array of length one with the type of the object as its element type.
-  bool IsArray = MostDerivedPathLength == Entries.size() &&
- MostDerivedIsArrayElement;
-  uint64_t ArrayIndex =
-  IsArray ? Entries.back().ArrayIndex : (uint64_t)IsOnePastTheEnd;
-  uint64_t ArraySize =
-  IsArray ? getMostDerivedArraySize() : (uint64_t)1;
-
-  if (N < -(int64_t)ArrayIndex || N > ArraySize - ArrayIndex) {
-// Calculate the actual index in a wide enough type, so we can include
-// it in the note.
-N = N.extend(std::max(N.getBitWidth() + 1, 65));
-(llvm::APInt&)N += ArrayIndex;
-assert(N.ugt(ArraySize) && "bounds check failed for in-bounds index");
-diagnosePointerArithmetic(Info, E, N);
-setInvalid();
-return;
-  }
-
-  ArrayIndex += TruncatedN;
-  assert(ArrayIndex <= ArraySize &&
- "bounds check succeeded for out-of-bounds index");
-
-  if (IsArray)
-Entries.back().ArrayIndex = ArrayIndex;
-  else
-IsOnePastTheEnd = (ArrayIndex != 0);
-}
+bool adjustIndex(EvalInfo &Info, const Expr *E, APSInt N);
   };
 
   /// A stack frame in the constexpr call stack.
@@ -721,43 +679,68 @@
 bool IsSpeculativelyEvaluating;
 
 enum EvaluationMode {
-  /// Evaluate as a constant expression. Stop if we find that the expression
-  /// is not a constant expression.
+  /* The various EvaluationMode kinds vary in terms of what sorts of
+   * expressions they accept.
+
+ Key for the following table:
+ * D = Diagnostic notes (about non-constexpr, but still evaluable
+   constructs) are OK (keepEvaluatingAfterNote)
+ * S = Failure to evaluate which only occurs in expressions used for
+   side-effect are okay.  (keepEvaluatingAfterSideEffect)
+ * F = Failure to evaluate is okay. (keepEvaluatingAfterFailure)
+ * E = Eagerly evaluate certain builtins to a value which would normally
+   defer until after optimizations.
+
+ +-+---+---+---+---+
+ | | D | S | F | E |
+ +-+---+---+---+---+
+ |EM_ConstantExpression| . | . | . | . |
+ |EM_ConstantFold  | Y | . | . | . |
+ |EM_ConstantFoldUnevaluated   | Y | . | . | Y |
+ |EM_IgnoreSideEffects | Y | Y | . | . |
+ |EM_PotentialConstantExpression   | . | Y | Y | . |
+ |EM_PotentialConstan

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343892: Emit CK_NoOp casts in C mode, not just C++. 
(authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52918?vs=168477&id=168541#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52918

Files:
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/c-casts.c


Index: test/Sema/c-casts.c
===
--- test/Sema/c-casts.c
+++ test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 


Index: test/Sema/c-casts.c
===
--- test/Sema/c-casts.c
+++ test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Resu

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343892: Emit CK_NoOp casts in C mode, not just C++. 
(authored by jyknight, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52918?vs=168477&id=168542#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52918

Files:
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/c-casts.c


Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: cfe/trunk/test/Sema/c-casts.c
===
--- cfe/trunk/test/Sema/c-casts.c
+++ cfe/trunk/test/Sema/c-casts.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -w -ast-dump %s | FileCheck %s
+
+// The cast construction code both for implicit and c-style casts is very
+// different in C vs C++. This file is intended to test the C behavior.
+
+// TODO: add tests covering the rest of the code in
+// Sema::CheckAssignmentConstraints and Sema::PrepareScalarCast
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_cvr_pointer
+void cast_cvr_pointer(char volatile * __restrict * const * p) {
+  char*** x;
+  // CHECK: ImplicitCastExpr {{.*}} 'char ***' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'char ***' 
+  x = (char***)p;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} cast_pointer_type
+void cast_pointer_type(char *p) {
+  void *x;
+  // CHECK: ImplicitCastExpr {{.*}} 'void *' 
+  x = p;
+  // CHECK: CStyleCastExpr {{.*}} 'void *' 
+  x = (void*)p;
+}


Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -5864,11 +5864,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5862,6 +5862,8 @@
   LangAS DestAS = DestTy->getPointeeType().getAddressSpace();
   if (SrcAS != DestAS)
 return CK_AddressSpaceConversion;
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
+return CK_NoOp;
   return CK_BitCast;
 }
 case Type::STK_BlockPointer:
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace()

[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: dblaikie, rsmith.
Herald added a subscriber: Anastasia.
Herald added a project: clang.

Currently, EmitCall emits a call instruction with a function type
derived from the pointee-type of the callee. This *should* be the same
as the type created from the CallInfo parameter, but in some cases an
incorrect CallInfo was being passed.

All of these fixes were discovered by the addition of the assert in
EmitCall which verifies that the passed-in CallInfo matches the
Callee's function type.

As far as I know, these issues caused no bugs at the moment, as the
correct types were ultimately being emitted. But, some would become
problematic when pointee types are removed.

List of fixes:

- arrangeCXXConstructorCall was passing an incorrect value for the number of 
Required args, when calling an inheriting constructor where the inherited 
constructor is variadic. (The inheriting constructor doesn't actually get 
passed any of the user's args, but the code was calculating it as if it did).

- arrangeFreeFunctionLikeCall was not including the count of the 
pass_object_size arguments in the count of required args.

- OpenCL uses other address spaces for the "this" pointer. However, 
commonEmitCXXMemberOrOperatorCall was not annotating the address space on the 
"this" argument of the call.

- Destructor calls were being created with EmitCXXMemberOrOperatorCall instead 
of EmitCXXDestructorCall in a few places. This was a problem because the 
calling convention sometimes has destructors returning "this" rather than void, 
and the latter function knows about that, and sets up the types properly 
(through calling arrangeCXXStructorDeclaration), while the former does not.

- generateObjCGetterBody: the 'objc_getProperty' function returns type 'id', 
but was being called as if it returned the particular property's type. (That is 
of course the *dynamic* return type, and there's a downcast immediately after.)

- OpenMP user-defined reduction functions (#pragma omp declare reduction) can 
be called with a subclass of the declared type. In such case, the call was 
being setup as if the function had been actually declared to take the subtype, 
rather than the base type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57664

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenTypes.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/CodeGenObjC/getter-property-mismatch.m

Index: clang/test/CodeGenObjC/getter-property-mismatch.m
===
--- clang/test/CodeGenObjC/getter-property-mismatch.m
+++ clang/test/CodeGenObjC/getter-property-mismatch.m
@@ -15,6 +15,4 @@
 
 // CHECK:  [[CALL:%.*]] = tail call i8* @objc_getProperty
 // CHECK:  [[ONE:%.*]] = bitcast i8* [[CALL:%.*]] to [[T1:%.*]]*
-// CHECK:  [[TWO:%.*]] = bitcast [[T1]]* [[ONE]] to [[T2:%.*]]*
-// CHECK:  ret [[T2]]* [[TWO]]
-
+// CHECK:  ret [[T1]]* [[ONE]]
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -10747,7 +10747,8 @@
   return D;
 return nullptr;
   }))
-return SemaRef.BuildDeclRefExpr(VD, Ty, VK_LValue, Loc);
+return SemaRef.BuildDeclRefExpr(VD, VD->getType().getNonReferenceType(),
+VK_LValue, Loc);
   if (auto *VD = filterLookupForUDR(
   Lookups, [&SemaRef, Ty, Loc](ValueDecl *D) -> ValueDecl * {
 if (!D->isInvalidDecl() &&
@@ -10765,7 +10766,8 @@
  /*DiagID=*/0) !=
 Sema::AR_inaccessible) {
   SemaRef.BuildBasePathArray(Paths, BasePath);
-  return SemaRef.BuildDeclRefExpr(VD, Ty, VK_LValue, Loc);
+  return SemaRef.BuildDeclRefExpr(
+  VD, VD->getType().getNonReferenceType(), VK_LValue, Loc);
 }
   }
 }
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1566,9 +1566,8 @@
 Callee = CGCallee::forDirect(
 CGM.getAddrOfCXXStructor(DD, getFromDtorType(Type)), GD);
 
-  CGF.EmitCXXMemberOrOperatorCall(DD, Callee, ReturnValueSlot(),
-  This.getPointer(), VTT, VTTTy,
-  nullptr, nullptr);
+  CGF.EmitCXXDestructorCall(DD, Callee, This.getPointer(), VTT, VTTTy, nullptr,
+getFromDtorType(Type));
 }
 
 void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
@@ -1766,9 +1765,8 @@
   CGCallee Callee =
   CGCallee::forVirtual(CE, GlobalDecl(Dtor, DtorType), This, Ty);
 
-  CGF.EmitCXXMemberOrOperatorCall(Dtor, Callee, ReturnValueSlot(),
-  

[PATCH] D57668: [opaque pointer types] Pass function types for runtime function calls.

[James Y Knight via Phabricator via cfe-commits]

jyknight marked an inline comment as done.
jyknight added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:1028-1030
+llvm::Constant *getPropertyFn = cast(
+CGM.getObjCRuntime().GetPropertyGetFunction().getCallee());
 if (!getPropertyFn) {

dblaikie wrote:
> This code looks like it implies that the 'if' is never hit? (since cast 
> doesn't propagate null (it asserts/fails/UB on null)) - should this be 
> cast_or_null instead? Or "if(!CGM.getObjCRuntime().getPropertyGetFunction())" 
> ?)
> 
> (there are a few similar instances later on)
Indeed, good catch! I don't know if that actually happens, but this code also 
didn't even need these casts.

Removed the casts, both simplifying the code, and fixing that potential issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57668/new/

https://reviews.llvm.org/D57668



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

[James Y Knight via Phabricator via cfe-commits]

jyknight marked an inline comment as done.
jyknight added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3837
+// having pointee types).
+llvm::FunctionType *IRFuncTyFromInfo = 
getTypes().GetFunctionType(CallInfo);
+assert(IRFuncTy == IRFuncTyFromInfo);

dblaikie wrote:
> This will be warned as unused in a release build.
> 
> Would this be hideous if it's just all one big assert?
> 
>   assert((CallInfo.isVariadic && CallInfo.getArgStruct) || IRFuncTy == 
> getTypes().GetFunctionType(CallInfo));
> 
> (I think that's accurate?)
Clearer IMO to just put #ifndef NDEBUG around the block, so I'll do that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57664/new/

https://reviews.llvm.org/D57664



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353181: [opaque pointer types] Fix the CallInfo passed to 
EmitCall in some (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57664?vs=184975&id=185313#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57664/new/

https://reviews.llvm.org/D57664

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenTypes.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Sema/SemaOpenMP.cpp
  test/CodeGenObjC/getter-property-mismatch.m

Index: test/CodeGenObjC/getter-property-mismatch.m
===
--- test/CodeGenObjC/getter-property-mismatch.m
+++ test/CodeGenObjC/getter-property-mismatch.m
@@ -15,6 +15,4 @@
 
 // CHECK:  [[CALL:%.*]] = tail call i8* @objc_getProperty
 // CHECK:  [[ONE:%.*]] = bitcast i8* [[CALL:%.*]] to [[T1:%.*]]*
-// CHECK:  [[TWO:%.*]] = bitcast [[T1]]* [[ONE]] to [[T2:%.*]]*
-// CHECK:  ret [[T2]]* [[TWO]]
-
+// CHECK:  ret [[T1]]* [[ONE]]
Index: lib/CodeGen/CodeGenTypes.h
===
--- lib/CodeGen/CodeGenTypes.h
+++ lib/CodeGen/CodeGenTypes.h
@@ -182,6 +182,10 @@
   /// Convert clang calling convention to LLVM callilng convention.
   unsigned ClangCallConvToLLVMCallConv(CallingConv CC);
 
+  /// Derives the 'this' type for codegen purposes, i.e. ignoring method CVR
+  /// qualification.
+  CanQualType DeriveThisType(const CXXRecordDecl *RD, const CXXMethodDecl *MD);
+
   /// ConvertType - Convert type T into a llvm::Type.
   llvm::Type *ConvertType(QualType T);
 
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1566,9 +1566,8 @@
 Callee = CGCallee::forDirect(
 CGM.getAddrOfCXXStructor(DD, getFromDtorType(Type)), GD);
 
-  CGF.EmitCXXMemberOrOperatorCall(DD, Callee, ReturnValueSlot(),
-  This.getPointer(), VTT, VTTTy,
-  nullptr, nullptr);
+  CGF.EmitCXXDestructorCall(DD, Callee, This.getPointer(), VTT, VTTTy, nullptr,
+getFromDtorType(Type));
 }
 
 void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
@@ -1766,9 +1765,8 @@
   CGCallee Callee =
   CGCallee::forVirtual(CE, GlobalDecl(Dtor, DtorType), This, Ty);
 
-  CGF.EmitCXXMemberOrOperatorCall(Dtor, Callee, ReturnValueSlot(),
-  This.getPointer(), /*ImplicitParam=*/nullptr,
-  QualType(), CE, nullptr);
+  CGF.EmitCXXDestructorCall(Dtor, Callee, This.getPointer(), nullptr,
+QualType(), nullptr, getFromDtorType(DtorType));
   return nullptr;
 }
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -67,10 +67,17 @@
 }
 
 /// Derives the 'this' type for codegen purposes, i.e. ignoring method CVR
-/// qualification.
-static CanQualType GetThisType(ASTContext &Context, const CXXRecordDecl *RD,
-   const CXXMethodDecl *MD) {
-  QualType RecTy = Context.getTagDeclType(RD)->getCanonicalTypeInternal();
+/// qualification. Either or both of RD and MD may be null. A null RD indicates
+/// that there is no meaningful 'this' type, and a null MD can occur when
+/// calling a method pointer.
+CanQualType CodeGenTypes::DeriveThisType(const CXXRecordDecl *RD,
+ const CXXMethodDecl *MD) {
+  QualType RecTy;
+  if (RD)
+RecTy = Context.getTagDeclType(RD)->getCanonicalTypeInternal();
+  else
+RecTy = Context.VoidTy;
+
   if (MD)
 RecTy = Context.getAddrSpaceQualType(RecTy, MD->getMethodQualifiers().getAddressSpace());
   return Context.getPointerType(CanQualType::CreateUnsafe(RecTy));
@@ -235,7 +242,7 @@
 
 /// Arrange the argument and result information for a call to an
 /// unknown C++ non-static member function of the given abstract type.
-/// (Zero value of RD means we don't have any meaningful "this" argument type,
+/// (A null RD means we don't have any meaningful "this" argument type,
 ///  so fall back to a generic pointer type).
 /// The member function must be an ordinary function, i.e. not a
 /// constructor or destructor.
@@ -246,10 +253,7 @@
   SmallVector argTypes;
 
   // Add the 'this' pointer.
-  if (RD)
-argTypes.push_back(GetThisType(Context, RD, MD));
-  else
-argTypes.push_back(Context.VoidPtrTy);
+  argTypes.push_back(DeriveThisType(RD, MD));
 
   return ::arrangeLLVMFunctionInfo(
   *this, true, argTypes,
@@ -303,7 +307,7 @@
 
   SmallVector argTypes;
   SmallVector paramInfos;
-  argTypes.push_back(GetThisType(Context, MD->getParent(), MD));
+  argTypes.push_back(DeriveThisType(MD->getParent(), MD));
 

[PATCH] D57767: [opaque pointer types] Cleanup CGBuilder's Create*GEP.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: dblaikie.
Herald added subscribers: cfe-commits, jfb, jholewinski.
Herald added a project: clang.

The various EltSize, Offset, DataLayout, and StructLayout arguments
are all computable from the Address's element type and the DataLayout
which the CGBuilder already has access to.

After having previously asserted that the computed values are the same
as those passed in, now remove the redundant arguments from
CGBuilder's Create*GEP functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57767

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp

Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -3642,8 +3642,8 @@
 
 static Address EmitX86_64VAArgFromMemory(CodeGenFunction &CGF,
  Address VAListAddr, QualType Ty) {
-  Address overflow_arg_area_p = CGF.Builder.CreateStructGEP(
-  VAListAddr, 2, CharUnits::fromQuantity(8), "overflow_arg_area_p");
+  Address overflow_arg_area_p =
+  CGF.Builder.CreateStructGEP(VAListAddr, 2, "overflow_arg_area_p");
   llvm::Value *overflow_arg_area =
 CGF.Builder.CreateLoad(overflow_arg_area_p, "overflow_arg_area");
 
@@ -3714,18 +3714,14 @@
   Address gp_offset_p = Address::invalid(), fp_offset_p = Address::invalid();
   llvm::Value *gp_offset = nullptr, *fp_offset = nullptr;
   if (neededInt) {
-gp_offset_p =
-CGF.Builder.CreateStructGEP(VAListAddr, 0, CharUnits::Zero(),
-"gp_offset_p");
+gp_offset_p = CGF.Builder.CreateStructGEP(VAListAddr, 0, "gp_offset_p");
 gp_offset = CGF.Builder.CreateLoad(gp_offset_p, "gp_offset");
 InRegs = llvm::ConstantInt::get(CGF.Int32Ty, 48 - neededInt * 8);
 InRegs = CGF.Builder.CreateICmpULE(gp_offset, InRegs, "fits_in_gp");
   }
 
   if (neededSSE) {
-fp_offset_p =
-CGF.Builder.CreateStructGEP(VAListAddr, 1, CharUnits::fromQuantity(4),
-"fp_offset_p");
+fp_offset_p = CGF.Builder.CreateStructGEP(VAListAddr, 1, "fp_offset_p");
 fp_offset = CGF.Builder.CreateLoad(fp_offset_p, "fp_offset");
 llvm::Value *FitsInFP =
   llvm::ConstantInt::get(CGF.Int32Ty, 176 - neededSSE * 16);
@@ -3754,9 +3750,7 @@
   // loads than necessary. Can we clean this up?
   llvm::Type *LTy = CGF.ConvertTypeForMem(Ty);
   llvm::Value *RegSaveArea = CGF.Builder.CreateLoad(
-  CGF.Builder.CreateStructGEP(
-  VAListAddr, 3, CharUnits::fromQuantity(8) + CGF.getPointerSize()),
-  "reg_save_area");
+  CGF.Builder.CreateStructGEP(VAListAddr, 3), "reg_save_area");
 
   Address RegAddr = Address::invalid();
   if (neededInt && neededSSE) {
@@ -3782,16 +3776,13 @@
 llvm::Value *V = CGF.Builder.CreateAlignedLoad(
 TyLo, CGF.Builder.CreateBitCast(RegLoAddr, PTyLo),
 CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(TyLo)));
-CGF.Builder.CreateStore(V,
-CGF.Builder.CreateStructGEP(Tmp, 0, CharUnits::Zero()));
+CGF.Builder.CreateStore(V, CGF.Builder.CreateStructGEP(Tmp, 0));
 
 // Copy the second element.
 V = CGF.Builder.CreateAlignedLoad(
 TyHi, CGF.Builder.CreateBitCast(RegHiAddr, PTyHi),
 CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(TyHi)));
-CharUnits Offset = CharUnits::fromQuantity(
-   getDataLayout().getStructLayout(ST)->getElementOffset(1));
-CGF.Builder.CreateStore(V, CGF.Builder.CreateStructGEP(Tmp, 1, Offset));
+CGF.Builder.CreateStore(V, CGF.Builder.CreateStructGEP(Tmp, 1));
 
 RegAddr = CGF.Builder.CreateElementBitCast(Tmp, LTy);
   } else if (neededInt) {
@@ -3838,12 +3829,10 @@
 Tmp = CGF.Builder.CreateElementBitCast(Tmp, ST);
 V = CGF.Builder.CreateLoad(CGF.Builder.CreateElementBitCast(
 RegAddrLo, ST->getStructElementType(0)));
-CGF.Builder.CreateStore(V,
-   CGF.Builder.CreateStructGEP(Tmp, 0, CharUnits::Zero()));
+CGF.Builder.CreateStore(V, CGF.Builder.CreateStructGEP(Tmp, 0));
 V = CGF.Builder.CreateLoad(CGF.Builder.CreateElementBitCast(
 RegAddrHi, ST->getStructElementType(1)));
-CGF.Builder.CreateStore(V,
-  CGF.Builder.CreateStructGEP(Tmp, 1, CharUnits::

[PATCH] D57794: Fix MSVC constructor call extension after b92d290e48e9 (r353181).

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: thakis, rsmith.
Herald added subscribers: cfe-commits, mstorsjo.
Herald added a project: clang.

The assert added to EmitCall there was triggering in Windows Chromium
builds, due to a mismatch of the return type.

The MSVC constructor call extension (`this->Foo::Foo()`) was emitting
the constructor call from 'EmitCXXMemberOrOperatorMemberCallExpr' via
calling 'EmitCXXMemberOrOperatorCall', instead of
'EmitCXXConstructorCall'. On targets where HasThisReturn is true, that
was failing to set the proper return type in the call info.

Switching to calling EmitCXXConstructorCall also allowed removing some
code e.g. the trivial copy/move support, which is already handled in
EmitCXXConstructorCall.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57794

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/test/CodeGenCXX/constructor-direct-call.cpp

Index: clang/test/CodeGenCXX/constructor-direct-call.cpp
===
--- clang/test/CodeGenCXX/constructor-direct-call.cpp
+++ clang/test/CodeGenCXX/constructor-direct-call.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK32 --check-prefix=CHECK
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK64 --check-prefix=CHECK
 
 class Test1 {
 public:
@@ -9,7 +10,8 @@
   Test1 var;
   var.Test1::Test1();
 
-  // CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
+  // CHECK32:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
+  // CHECK64:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 4, i1 false)
   var.Test1::Test1(var);
 }
 
@@ -22,13 +24,16 @@
 
 void f2() {
   // CHECK:  %var = alloca %class.Test2, align 4
-  // CHECK-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK32-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK64-NEXT:  %call = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var)
   Test2 var;
 
-  // CHECK-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK32-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK64-NEXT:  %call1 = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var)
   var.Test2::Test2();
 
-  // CHECK:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false)
+  // CHECK32:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false)
+  // CHECK64:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 8, i1 false)
   var.Test2::Test2(var);
 }
 
@@ -45,15 +50,19 @@
 };
 
 void f3() {
-  // CHECK: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK32: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK64: %call = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var)
   Test3 var;
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2)
+  // CHECK64-NEXT: %call1 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var2)
   Test3 var2;
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK64-NEXT: %call2 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var)
   var.Test3::Test3();
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
+  // CHECK64-NEXT: %call3 = call %class.Test3* @"??0Test3@@QEAA@AEBV0@@Z"(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
   var.Test3::Test3(var2);
 }
Index: clang/lib/CodeGen/CGExprCXX.cpp
===
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -249,13 +249,25 @@
 This = EmitLValue(Base);
   }
 
+  if (const CXXConstructorDecl *Ctor = dyn_cast(MD)) {
+// This is the MSVC p->Ctor::Ctor(...) extension. We assume that's
+// constructing a new complete object of type Ctor.
+assert(!RtlArgs);
+assert(ReturnValue.isNull() && "Constructor shouldn't have return value");
+CallArgList Args;
+commonEmitCXXMemberOrOperatorCall(
+*this, Ctor, This.getPointer(), /*ImplicitParam=*/nullptr,
+/*ImplicitParamTy=*/QualType(), CE, A

[PATCH] D57794: Fix MSVC constructor call extension after b92d290e48e9 (r353181).

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353246: Fix MSVC constructor call extension after 
b92d290e48e9 (r353181). (authored by jyknight, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57794?vs=185426&id=185430#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57794/new/

https://reviews.llvm.org/D57794

Files:
  cfe/trunk/lib/CodeGen/CGExprCXX.cpp
  cfe/trunk/test/CodeGenCXX/constructor-direct-call.cpp

Index: cfe/trunk/test/CodeGenCXX/constructor-direct-call.cpp
===
--- cfe/trunk/test/CodeGenCXX/constructor-direct-call.cpp
+++ cfe/trunk/test/CodeGenCXX/constructor-direct-call.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK32 --check-prefix=CHECK
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK64 --check-prefix=CHECK
 
 class Test1 {
 public:
@@ -9,7 +10,8 @@
   Test1 var;
   var.Test1::Test1();
 
-  // CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
+  // CHECK32:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
+  // CHECK64:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 4, i1 false)
   var.Test1::Test1(var);
 }
 
@@ -22,13 +24,16 @@
 
 void f2() {
   // CHECK:  %var = alloca %class.Test2, align 4
-  // CHECK-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK32-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK64-NEXT:  %call = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var)
   Test2 var;
 
-  // CHECK-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK32-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK64-NEXT:  %call1 = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var)
   var.Test2::Test2();
 
-  // CHECK:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false)
+  // CHECK32:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false)
+  // CHECK64:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 8, i1 false)
   var.Test2::Test2(var);
 }
 
@@ -45,15 +50,19 @@
 };
 
 void f3() {
-  // CHECK: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK32: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK64: %call = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var)
   Test3 var;
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2)
+  // CHECK64-NEXT: %call1 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var2)
   Test3 var2;
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK64-NEXT: %call2 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var)
   var.Test3::Test3();
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
+  // CHECK64-NEXT: %call3 = call %class.Test3* @"??0Test3@@QEAA@AEBV0@@Z"(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
   var.Test3::Test3(var2);
 }
Index: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp
@@ -249,13 +249,25 @@
 This = EmitLValue(Base);
   }
 
+  if (const CXXConstructorDecl *Ctor = dyn_cast(MD)) {
+// This is the MSVC p->Ctor::Ctor(...) extension. We assume that's
+// constructing a new complete object of type Ctor.
+assert(!RtlArgs);
+assert(ReturnValue.isNull() && "Constructor shouldn't have return value");
+CallArgList Args;
+commonEmitCXXMemberOrOperatorCall(
+*this, Ctor, This.getPointer(), /*ImplicitParam=*/nullptr,
+/*ImplicitParamTy=*/QualType(), CE, Args, nullptr);
+
+EmitCXXConstructorCall(Ctor, Ctor_Complete, /*ForVirtualBase=*/false,
+   /*Delegating=*/false, This.getAddress(), Args,
+   AggValueSlot::DoesNotOverlap, CE->getExprLoc(),
+   /*NewPointerIsChecked=*/false);
+return 

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: dblaikie.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57801

Files:
  clang/lib/CodeGen/CGCXX.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp

Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -394,7 +394,8 @@
llvm::GlobalVariable *DeclPtr,
bool PerformInit) override;
   void registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D,
-  llvm::Constant *Dtor, llvm::Constant *Addr) override;
+  llvm::FunctionCallee Dtor,
+  llvm::Constant *Addr) override;
 
   //  Notes on array cookies =
   //
@@ -,7 +2223,7 @@
 }
 
 static void emitGlobalDtorWithTLRegDtor(CodeGenFunction &CGF, const VarDecl &VD,
-llvm::Constant *Dtor,
+llvm::FunctionCallee Dtor,
 llvm::Constant *Addr) {
   // Create a function which calls the destructor.
   llvm::Constant *DtorStub = CGF.createAtExitStub(VD, Dtor, Addr);
@@ -2241,7 +2242,7 @@
 }
 
 void MicrosoftCXXABI::registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D,
- llvm::Constant *Dtor,
+ llvm::FunctionCallee Dtor,
  llvm::Constant *Addr) {
   if (D.isNoDestroy(CGM.getContext()))
 return;
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -328,7 +328,8 @@
llvm::GlobalVariable *DeclPtr,
bool PerformInit) override;
   void registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D,
-  llvm::Constant *dtor, llvm::Constant *addr) override;
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) override;
 
   llvm::Function *getOrCreateThreadLocalWrapper(const VarDecl *VD,
 llvm::Value *Val);
@@ -2284,9 +2285,8 @@
 
 /// Register a global destructor using __cxa_atexit.
 static void emitGlobalDtorWithCXAAtExit(CodeGenFunction &CGF,
-llvm::Constant *dtor,
-llvm::Constant *addr,
-bool TLS) {
+llvm::FunctionCallee dtor,
+llvm::Constant *addr, bool TLS) {
   const char *Name = "__cxa_atexit";
   if (TLS) {
 const llvm::Triple &T = CGF.getTarget().getTriple();
@@ -2322,11 +2322,10 @@
 // function.
 addr = llvm::Constant::getNullValue(CGF.Int8PtrTy);
 
-  llvm::Value *args[] = {
-llvm::ConstantExpr::getBitCast(dtor, dtorTy),
-llvm::ConstantExpr::getBitCast(addr, CGF.Int8PtrTy),
-handle
-  };
+  llvm::Value *args[] = {llvm::ConstantExpr::getBitCast(
+ cast(dtor.getCallee()), dtorTy),
+ llvm::ConstantExpr::getBitCast(addr, CGF.Int8PtrTy),
+ handle};
   CGF.EmitNounwindRuntimeCall(atexit, args);
 }
 
@@ -2375,9 +2374,8 @@
 }
 
 /// Register a global destructor as best as we know how.
-void ItaniumCXXABI::registerGlobalDtor(CodeGenFunction &CGF,
-   const VarDecl &D,
-   llvm::Constant *dtor,
+void ItaniumCXXABI::registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D,
+   llvm::FunctionCallee dtor,
llvm::Constant *addr) {
   if (D.isNoDestroy(CGM.getContext()))
 return;
@@ -2541,6 +2539,8 @@
   getMangleContext().mangleItaniumThreadLocalInit(VD, Out);
 }
 
+llvm::FunctionType *InitFnTy = llvm::FunctionType::get(CGM.VoidTy, false);
+
 // If we have a definition for the variable, emit the initialization
 // function as an alias to the global Init function (if any). Otherwise,
 // produce a declaration of the initialization function.
@@ -2559,8 +2559,7 @@
   // This function will not exist if the TU defining the thread_local
   // variable in question does not need any dynamic initialization for
   // its thread_local variables.
-  llvm::FunctionType *FnTy = llvm::FunctionType::get(CGM.VoidTy, false);
-  Init = llvm::Function::Crea

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: dblaikie.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Also, remove the getFunctionType() function from CGCallee, since it
accesses the pointee type of the value. The only use was in EmitCall,
so just inline it into the debug assertion.

This is the last of the changes for Call and Invoke in clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57804

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h

Index: clang/lib/CodeGen/CGCall.h
===
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -204,12 +204,9 @@
   assert(isVirtual());
   return VirtualInfo.Addr;
 }
-
-llvm::FunctionType *getFunctionType() const {
-  if (isVirtual())
-return VirtualInfo.FTy;
-  return cast(
-  getFunctionPointer()->getType()->getPointerElementType());
+llvm::FunctionType *getVirtualFunctionType() const {
+  assert(isVirtual());
+  return VirtualInfo.FTy;
 }
 
 /// If this is a delayed callee computation of some sort, prepare
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -3826,7 +3826,7 @@
   QualType RetTy = CallInfo.getReturnType();
   const ABIArgInfo &RetAI = CallInfo.getReturnInfo();
 
-  llvm::FunctionType *IRFuncTy = Callee.getFunctionType();
+  llvm::FunctionType *IRFuncTy = getTypes().GetFunctionType(CallInfo);
 
 #ifndef NDEBUG
   if (!(CallInfo.isVariadic() && CallInfo.getArgStruct())) {
@@ -3837,8 +3837,13 @@
 //
 // In other cases, we assert that the types match up (until pointers stop
 // having pointee types).
-llvm::FunctionType *IRFuncTyFromInfo = getTypes().GetFunctionType(CallInfo);
-assert(IRFuncTy == IRFuncTyFromInfo);
+llvm::Type *TypeFromVal;
+if (Callee.isVirtual())
+  TypeFromVal = Callee.getVirtualFunctionType();
+else
+  TypeFromVal =
+  Callee.getFunctionPointer()->getType()->getPointerElementType();
+assert(IRFuncTy == TypeFromVal);
   }
 #endif
 
@@ -4207,8 +4212,8 @@
   // cases, we can't do any parameter mismatch checks.  Give up and bitcast
   // the callee.
   unsigned CalleeAS = CalleePtr->getType()->getPointerAddressSpace();
-  auto FnTy = getTypes().GetFunctionType(CallInfo)->getPointerTo(CalleeAS);
-  CalleePtr = Builder.CreateBitCast(CalleePtr, FnTy);
+  CalleePtr =
+  Builder.CreateBitCast(CalleePtr, IRFuncTy->getPointerTo(CalleeAS));
 } else {
   llvm::Type *LastParamTy =
   IRFuncTy->getParamType(IRFuncTy->getNumParams() - 1);
@@ -4240,19 +4245,20 @@
   //
   // This makes the IR nicer, but more importantly it ensures that we
   // can inline the function at -O0 if it is marked always_inline.
-  auto simplifyVariadicCallee = [](llvm::Value *Ptr) -> llvm::Value* {
-llvm::FunctionType *CalleeFT =
-  cast(Ptr->getType()->getPointerElementType());
+  auto simplifyVariadicCallee = [](llvm::FunctionType *CalleeFT,
+   llvm::Value *Ptr) -> llvm::Function * {
 if (!CalleeFT->isVarArg())
-  return Ptr;
+  return nullptr;
 
-llvm::ConstantExpr *CE = dyn_cast(Ptr);
-if (!CE || CE->getOpcode() != llvm::Instruction::BitCast)
-  return Ptr;
+// Get underlying value if it's a bitcast
+if (llvm::ConstantExpr *CE = dyn_cast(Ptr)) {
+  if (CE->getOpcode() == llvm::Instruction::BitCast)
+Ptr = CE->getOperand(0);
+}
 
-llvm::Function *OrigFn = dyn_cast(CE->getOperand(0));
+llvm::Function *OrigFn = dyn_cast(Ptr);
 if (!OrigFn)
-  return Ptr;
+  return nullptr;
 
 llvm::FunctionType *OrigFT = OrigFn->getFunctionType();
 
@@ -4261,15 +4267,19 @@
 if (OrigFT->isVarArg() ||
 OrigFT->getNumParams() != CalleeFT->getNumParams() ||
 OrigFT->getReturnType() != CalleeFT->getReturnType())
-  return Ptr;
+  return nullptr;
 
 for (unsigned i = 0, e = OrigFT->getNumParams(); i != e; ++i)
   if (OrigFT->getParamType(i) != CalleeFT->getParamType(i))
-return Ptr;
+return nullptr;
 
 return OrigFn;
   };
-  CalleePtr = simplifyVariadicCallee(CalleePtr);
+
+  if (llvm::Function *OrigFn = simplifyVariadicCallee(IRFuncTy, CalleePtr)) {
+CalleePtr = OrigFn;
+IRFuncTy = OrigFn->getFunctionType();
+  }
 
   // 3. Perform the actual call.
 
@@ -4364,10 +4374,10 @@
   // Emit the actual call/invoke instruction.
   llvm::CallBase *CI;
   if (!InvokeDest) {
-CI = Builder.CreateCall(CalleePtr, IRCallArgs, BundleList);
+CI = Builder.CreateCall(IRFuncTy, CalleePtr, IRCallArgs, BundleList);
   } else {
 llvm::BasicBlock *Cont = createBasicBlock("invoke.cont");
-CI = Builder.CreateInvoke(CalleePtr, Cont, InvokeDest, IRCallArgs,
+CI = Builder.CreateInvoke(IRFuncTy, Calle

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353355: [opaque pointer types] Pass through function types 
for TLS (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57801?vs=185467&id=185677#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57801/new/

https://reviews.llvm.org/D57801

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGCXXABI.h
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp

Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -227,10 +227,11 @@
   return Fn;
 }
 
-llvm::Constant *CodeGenModule::getAddrOfCXXStructor(
+llvm::FunctionCallee CodeGenModule::getAddrAndTypeOfCXXStructor(
 const CXXMethodDecl *MD, StructorType Type, const CGFunctionInfo *FnInfo,
 llvm::FunctionType *FnType, bool DontDefer,
 ForDefinition_t IsForDefinition) {
+
   GlobalDecl GD;
   if (auto *CD = dyn_cast(MD)) {
 GD = GlobalDecl(CD, toCXXCtorType(Type));
@@ -249,9 +250,10 @@
 FnType = getTypes().GetFunctionType(*FnInfo);
   }
 
-  return GetOrCreateLLVMFunction(
+  llvm::Constant *Ptr = GetOrCreateLLVMFunction(
   getMangledName(GD), FnType, GD, /*ForVTable=*/false, DontDefer,
   /*isThunk=*/false, /*ExtraAttrs=*/llvm::AttributeList(), IsForDefinition);
+  return {FnType, Ptr};
 }
 
 static CGCallee BuildAppleKextVirtualCall(CodeGenFunction &CGF,
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -394,7 +394,8 @@
llvm::GlobalVariable *DeclPtr,
bool PerformInit) override;
   void registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D,
-  llvm::Constant *Dtor, llvm::Constant *Addr) override;
+  llvm::FunctionCallee Dtor,
+  llvm::Constant *Addr) override;
 
   //  Notes on array cookies =
   //
@@ -,7 +2223,7 @@
 }
 
 static void emitGlobalDtorWithTLRegDtor(CodeGenFunction &CGF, const VarDecl &VD,
-llvm::Constant *Dtor,
+llvm::FunctionCallee Dtor,
 llvm::Constant *Addr) {
   // Create a function which calls the destructor.
   llvm::Constant *DtorStub = CGF.createAtExitStub(VD, Dtor, Addr);
@@ -2241,7 +2242,7 @@
 }
 
 void MicrosoftCXXABI::registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D,
- llvm::Constant *Dtor,
+ llvm::FunctionCallee Dtor,
  llvm::Constant *Addr) {
   if (D.isNoDestroy(CGM.getContext()))
 return;
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3924,12 +3924,12 @@
   void EmitCXXGlobalVarDeclInit(const VarDecl &D, llvm::Constant *DeclPtr,
 bool PerformInit);
 
-  llvm::Constant *createAtExitStub(const VarDecl &VD, llvm::Constant *Dtor,
+  llvm::Function *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee Dtor,
llvm::Constant *Addr);
 
   /// Call atexit() with a function that passes the given argument to
   /// the given function.
-  void registerGlobalDtorWithAtExit(const VarDecl &D, llvm::Constant *fn,
+  void registerGlobalDtorWithAtExit(const VarDecl &D, llvm::FunctionCallee fn,
 llvm::Constant *addr);
 
   /// Call atexit() with function dtorStub.
@@ -3962,8 +3962,8 @@
   /// variables.
   void GenerateCXXGlobalDtorsFunc(
   llvm::Function *Fn,
-  const std::vector>
-  &DtorsAndObjects);
+  const std::vector> &DtorsAndObjects);
 
   void GenerateCXXGlobalVarDeclInitFunc(llvm::Function *Fn,
 const VarDecl *D,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -330,7 +330,7 @@
   switch (M->getStorageDuration()) {
   case SD_Static:
   case SD_Thread: {
-llvm::Constant *CleanupFn;
+llvm::FunctionCallee CleanupFn;
 llvm::Constant *CleanupArg;
 if (E->getType()->isArrayType()) {
   CleanupFn = CodeGenFunction(CGF.CGM).generateDestroyHelper(
@@ -339,8 +339,8 @@
   dyn_cast_or_null(M->getExtendingDecl()));
   CleanupArg = llvm::Constant::getNullValue(CGF.Int8PtrTy);
 } else {
-  CleanupFn = CGF.CGM.getAddrOfCXXStructor(ReferenceTemporaryDtor,
-

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353356: [opaque pointer types] Make EmitCall pass Function 
Types to (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57804?vs=185476&id=185678#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57804/new/

https://reviews.llvm.org/D57804

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGCall.h

Index: lib/CodeGen/CGCall.h
===
--- lib/CodeGen/CGCall.h
+++ lib/CodeGen/CGCall.h
@@ -204,12 +204,9 @@
   assert(isVirtual());
   return VirtualInfo.Addr;
 }
-
-llvm::FunctionType *getFunctionType() const {
-  if (isVirtual())
-return VirtualInfo.FTy;
-  return cast(
-  getFunctionPointer()->getType()->getPointerElementType());
+llvm::FunctionType *getVirtualFunctionType() const {
+  assert(isVirtual());
+  return VirtualInfo.FTy;
 }
 
 /// If this is a delayed callee computation of some sort, prepare
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -3826,7 +3826,7 @@
   QualType RetTy = CallInfo.getReturnType();
   const ABIArgInfo &RetAI = CallInfo.getReturnInfo();
 
-  llvm::FunctionType *IRFuncTy = Callee.getFunctionType();
+  llvm::FunctionType *IRFuncTy = getTypes().GetFunctionType(CallInfo);
 
 #ifndef NDEBUG
   if (!(CallInfo.isVariadic() && CallInfo.getArgStruct())) {
@@ -3837,8 +3837,13 @@
 //
 // In other cases, we assert that the types match up (until pointers stop
 // having pointee types).
-llvm::FunctionType *IRFuncTyFromInfo = getTypes().GetFunctionType(CallInfo);
-assert(IRFuncTy == IRFuncTyFromInfo);
+llvm::Type *TypeFromVal;
+if (Callee.isVirtual())
+  TypeFromVal = Callee.getVirtualFunctionType();
+else
+  TypeFromVal =
+  Callee.getFunctionPointer()->getType()->getPointerElementType();
+assert(IRFuncTy == TypeFromVal);
   }
 #endif
 
@@ -4207,8 +4212,8 @@
   // cases, we can't do any parameter mismatch checks.  Give up and bitcast
   // the callee.
   unsigned CalleeAS = CalleePtr->getType()->getPointerAddressSpace();
-  auto FnTy = getTypes().GetFunctionType(CallInfo)->getPointerTo(CalleeAS);
-  CalleePtr = Builder.CreateBitCast(CalleePtr, FnTy);
+  CalleePtr =
+  Builder.CreateBitCast(CalleePtr, IRFuncTy->getPointerTo(CalleeAS));
 } else {
   llvm::Type *LastParamTy =
   IRFuncTy->getParamType(IRFuncTy->getNumParams() - 1);
@@ -4240,19 +4245,20 @@
   //
   // This makes the IR nicer, but more importantly it ensures that we
   // can inline the function at -O0 if it is marked always_inline.
-  auto simplifyVariadicCallee = [](llvm::Value *Ptr) -> llvm::Value* {
-llvm::FunctionType *CalleeFT =
-  cast(Ptr->getType()->getPointerElementType());
+  auto simplifyVariadicCallee = [](llvm::FunctionType *CalleeFT,
+   llvm::Value *Ptr) -> llvm::Function * {
 if (!CalleeFT->isVarArg())
-  return Ptr;
+  return nullptr;
 
-llvm::ConstantExpr *CE = dyn_cast(Ptr);
-if (!CE || CE->getOpcode() != llvm::Instruction::BitCast)
-  return Ptr;
+// Get underlying value if it's a bitcast
+if (llvm::ConstantExpr *CE = dyn_cast(Ptr)) {
+  if (CE->getOpcode() == llvm::Instruction::BitCast)
+Ptr = CE->getOperand(0);
+}
 
-llvm::Function *OrigFn = dyn_cast(CE->getOperand(0));
+llvm::Function *OrigFn = dyn_cast(Ptr);
 if (!OrigFn)
-  return Ptr;
+  return nullptr;
 
 llvm::FunctionType *OrigFT = OrigFn->getFunctionType();
 
@@ -4261,15 +4267,19 @@
 if (OrigFT->isVarArg() ||
 OrigFT->getNumParams() != CalleeFT->getNumParams() ||
 OrigFT->getReturnType() != CalleeFT->getReturnType())
-  return Ptr;
+  return nullptr;
 
 for (unsigned i = 0, e = OrigFT->getNumParams(); i != e; ++i)
   if (OrigFT->getParamType(i) != CalleeFT->getParamType(i))
-return Ptr;
+return nullptr;
 
 return OrigFn;
   };
-  CalleePtr = simplifyVariadicCallee(CalleePtr);
+
+  if (llvm::Function *OrigFn = simplifyVariadicCallee(IRFuncTy, CalleePtr)) {
+CalleePtr = OrigFn;
+IRFuncTy = OrigFn->getFunctionType();
+  }
 
   // 3. Perform the actual call.
 
@@ -4364,10 +4374,10 @@
   // Emit the actual call/invoke instruction.
   llvm::CallBase *CI;
   if (!InvokeDest) {
-CI = Builder.CreateCall(CalleePtr, IRCallArgs, BundleList);
+CI = Builder.CreateCall(IRFuncTy, CalleePtr, IRCallArgs, BundleList);
   } else {
 llvm::BasicBlock *Cont = createBasicBlock("invoke.cont");
-CI = Builder.CreateInvoke(CalleePtr, Cont, InvokeDest, IRCallArgs,
+CI = Builder.CreateInvoke(IRFuncTy, CalleePtr, Cont, InvokeDest, IRCallArgs,
   BundleList);
 EmitBlock(Cont

[PATCH] D57767: [opaque pointer types] Cleanup CGBuilder's Create*GEP.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353629: [opaque pointer types] Cleanup CGBuilder's 
Create*GEP. (authored by jyknight, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57767?vs=185335&id=186134#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57767/new/

https://reviews.llvm.org/D57767

Files:
  cfe/trunk/lib/CodeGen/CGAtomic.cpp
  cfe/trunk/lib/CodeGen/CGBlocks.cpp
  cfe/trunk/lib/CodeGen/CGBuilder.h
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/CodeGen/CGCleanup.cpp
  cfe/trunk/lib/CodeGen/CGDecl.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGExprAgg.cpp
  cfe/trunk/lib/CodeGen/CGExprComplex.cpp
  cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
  cfe/trunk/lib/CodeGen/CGObjC.cpp
  cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
  cfe/trunk/lib/CodeGen/CGObjCMac.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/CodeGen/TargetInfo.cpp

Index: cfe/trunk/lib/CodeGen/CGObjC.cpp
===
--- cfe/trunk/lib/CodeGen/CGObjC.cpp
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp
@@ -158,9 +158,8 @@
 if (ALE) {
   // Emit the element and store it to the appropriate array slot.
   const Expr *Rhs = ALE->getElement(i);
-  LValue LV = MakeAddrLValue(
-  Builder.CreateConstArrayGEP(Objects, i, getPointerSize()),
-  ElementType, AlignmentSource::Decl);
+  LValue LV = MakeAddrLValue(Builder.CreateConstArrayGEP(Objects, i),
+ ElementType, AlignmentSource::Decl);
 
   llvm::Value *value = EmitScalarExpr(Rhs);
   EmitStoreThroughLValue(RValue::get(value), LV, true);
@@ -170,17 +169,15 @@
 } else {
   // Emit the key and store it to the appropriate array slot.
   const Expr *Key = DLE->getKeyValueElement(i).Key;
-  LValue KeyLV = MakeAddrLValue(
-  Builder.CreateConstArrayGEP(Keys, i, getPointerSize()),
-  ElementType, AlignmentSource::Decl);
+  LValue KeyLV = MakeAddrLValue(Builder.CreateConstArrayGEP(Keys, i),
+ElementType, AlignmentSource::Decl);
   llvm::Value *keyValue = EmitScalarExpr(Key);
   EmitStoreThroughLValue(RValue::get(keyValue), KeyLV, /*isInit=*/true);
 
   // Emit the value and store it to the appropriate array slot.
   const Expr *Value = DLE->getKeyValueElement(i).Value;
-  LValue ValueLV = MakeAddrLValue(
-  Builder.CreateConstArrayGEP(Objects, i, getPointerSize()),
-  ElementType, AlignmentSource::Decl);
+  LValue ValueLV = MakeAddrLValue(Builder.CreateConstArrayGEP(Objects, i),
+  ElementType, AlignmentSource::Decl);
   llvm::Value *valueValue = EmitScalarExpr(Value);
   EmitStoreThroughLValue(RValue::get(valueValue), ValueLV, /*isInit=*/true);
   if (TrackNeededObjects) {
@@ -1666,8 +1663,8 @@
   // Save the initial mutations value.  This is the value at an
   // address that was written into the state object by
   // countByEnumeratingWithState:objects:count:.
-  Address StateMutationsPtrPtr = Builder.CreateStructGEP(
-  StatePtr, 2, 2 * getPointerSize(), "mutationsptr.ptr");
+  Address StateMutationsPtrPtr =
+  Builder.CreateStructGEP(StatePtr, 2, "mutationsptr.ptr");
   llvm::Value *StateMutationsPtr
 = Builder.CreateLoad(StateMutationsPtrPtr, "mutationsptr");
 
@@ -1748,8 +1745,8 @@
   // Fetch the buffer out of the enumeration state.
   // TODO: this pointer should actually be invariant between
   // refreshes, which would help us do certain loop optimizations.
-  Address StateItemsPtr = Builder.CreateStructGEP(
-  StatePtr, 1, getPointerSize(), "stateitems.ptr");
+  Address StateItemsPtr =
+  Builder.CreateStructGEP(StatePtr, 1, "stateitems.ptr");
   llvm::Value *EnumStateItems =
 Builder.CreateLoad(StateItemsPtr, "stateitems");
 
Index: cfe/trunk/lib/CodeGen/CGExprComplex.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprComplex.cpp
+++ cfe/trunk/lib/CodeGen/CGExprComplex.cpp
@@ -327,15 +327,12 @@
 
 Address CodeGenFunction::emitAddrOfRealComponent(Address addr,
  QualType complexType) {
-  CharUnits offset = CharUnits::Zero();
-  return Builder.CreateStructGEP(addr, 0, offset, addr.getName() + ".realp");
+  return Builder.CreateStructGEP(addr, 0, addr.getName() + ".realp");
 }
 
 Address CodeGenFunction::emitAddrOfImagComponent(Address addr,
  QualType complexType) {
-  QualType eltType = complexType->castAs()->getElementType();
-  CharUnits offset = getContext().getTypeSizeInC

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

Looks reasonable to me.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58120/new/

https://reviews.llvm.org/D58120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58091: Customize warnings for missing built-in type

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I think this warning (-Wbuiltin-requires-header) doesn't really make sense as 
its own warning.

We already have two related (on-by-default) warnings.

For declarations:

  test.c:1:6: warning: incompatible redeclaration of library function 'exit' 
[-Wincompatible-library-redeclaration]
  long exit(char *);
   ^
  test.c:1:6: note: 'exit' is a builtin with type 'void (int) 
__attribute__((noreturn))'

And for uses:

  test2.c:1:13: warning: implicitly declaring library function 'exit' with type 
'void (int) __attribute__((noreturn))' [-Wimplicit-function-declaration]
  int foo() { exit(0); }
  ^
  test2.c:1:13: note: include the header  or explicitly provide a 
declaration for 'exit'

I think for a declaration, if we cannot construct the appropriate type, we 
should be treating all declarations as an incompatible redeclaration, and 
explain why in an attached note, like:

  warning: incompatible redeclaration of library function 'exit' 
[-Wincompatible-library-redeclaration]
  note: missing declaration of type 'jmp_buf' for argument 1 of standard 
function signature.

For a usage, we could emit something like:

  warning: implicit declaration of library function 'setjmp' 
[-Wimplicit-function-declaration]
  note: missing declaration of type 'jmp_buf' for argument 1.
  note: include the header  or explicitly provide a declaration for 
'setjmp'


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58091/new/

https://reviews.llvm.org/D58091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Basic/Targets/RISCV.h:94
+if (HasA)
+  MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
+  }

MaxAtomicPromoteWidth is an incompatible ABI-change kind of thing.

We should set that to the maximum atomic size this target ABI will _EVER_ 
support with any hardware, since it changes the data layout for atomic types.

MaxAtomicInlineWidth can change based on hardware, and be increased in the 
future if other hardware is introduced, but MaxAtomicPromoteWidth shouldn't.

I think it should be set it to 64 for most 32-bit platforms, and 128 for most 
64-bit platforms.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57450/new/

https://reviews.llvm.org/D57450



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: philip.pfaffe.
Herald added subscribers: cfe-commits, jdoerfert, jfb, dexonsmith, steven_wu, 
hiraditya, mehdi_amini.
Herald added projects: clang, LLVM.

Just as as llvm IR supports explicitly specifying numeric value ids
for instructions, and emits them by default in textual output, now do
the same for blocks.

This is a slightly incompatible change in the textual IR format.

Previously, llvm would parse numeric labels as string names. E.g.

  define void @f() {
br label %"55"
  55:
ret void
  }

defined a label *named* "55", even without needing to be quoted, while
the reference required quoting. Now, if you intend a block label which
looks like a value number to be a name, you must quote it in the
definition too (e.g. `"55":`).

Previously, llvm would print nameless blocks only as a comment, and
would omit it if there was no predecessor. This could cause confusion
for readers of the IR, just as unnamed instructions did prior to the
addition of "%5 = " syntax, back in 2008 (PR2480).

Now, it will always print a label for an unnamed block, with the
exception of the entry block. (IMO it may be better to print it for
the entry-block as well. However, that requires updating many more
tests.)

Thus, the following is supported, and is the canonical printing:

  define i32 @f(i32, i32) {
%3 = add i32 %0, %1
br label %4
  
  4:
ret i32 %3
  }

New test cases covering this behavior are added, and other tests
updated as required.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 187963.
jyknight added a comment.

Add some wording to LangRef and clang-format.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/docs/LangRef.rst
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 undef, label %4, label %10
 
-;CHECK:   :4
+;CHECK:   4:
 ;CHECK-NEXT:%5 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:invoke void @f1()
 
-; :4:
+4:
   %5 = load i32*, i32** undef, align 8
   invoke void @f1()
   to label %6 unwind label %1
 
-;CHECK:   :6
+;CHECK:   6:
 ;CHECK-NEXT:%7 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:%8 = load i32*, i32** undef, align 8
 
-; :6:
+6:
   %7 = load i32*, i32** undef, align 8
   %8 = load i32*, i32** undef, align 8
   br i1 true, label %9, label %17
 
-; :9:
+9:
   invoke void @f0()
   to label %10 unwind label %1
 
-; :10:
+10:
   invoke void @f2()
   to label %11 unwind label %1
 
-; :11:
+11:
   %12 = invoke signext i32 undef(i32* null, i32 signext undef, i1 zeroext undef)
   to label %13 unwind label %14
 
-; :13:
+13:
   unreachable
 
-; :14:
+14:
   %15 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :16:
+16:
   unreachable
 
-; :17:
+17:
   ret void
 
 ; uselistorder directives
Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/Sanitize

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D58548#1407164 , @dexonsmith wrote:

> I like this idea, and I don’t think the textual IR central is too important.  
> A few things:
>
> - Changes to the IR should always be discussed on llvm-dev.  Did this already 
> happen?


I sent a message ("Improving textual IR format for nameless blocks") 
concurrently with sending this review, and will wait a bit to make sure there's 
no objections.

> - Please update LangRef.

Done.

> - Did you write a script for updating the test cases?  If so, you might 
> consider attaching it to the RFC and linking to it from the commit message as 
> a courtesy to downstream maintainers.

Nope, I didn't; there were few enough instances that it didn't seem worth 
scripting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 187994.
jyknight marked 4 inline comments as done.
jyknight added a comment.

Minor tweaks per comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58548/new/

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/docs/LangRef.rst
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 undef, label %4, label %10
 
-;CHECK:   :4
+;CHECK:   4:
 ;CHECK-NEXT:%5 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:invoke void @f1()
 
-; :4:
+4:
   %5 = load i32*, i32** undef, align 8
   invoke void @f1()
   to label %6 unwind label %1
 
-;CHECK:   :6
+;CHECK:   6:
 ;CHECK-NEXT:%7 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:%8 = load i32*, i32** undef, align 8
 
-; :6:
+6:
   %7 = load i32*, i32** undef, align 8
   %8 = load i32*, i32** undef, align 8
   br i1 true, label %9, label %17
 
-; :9:
+9:
   invoke void @f0()
   to label %10 unwind label %1
 
-; :10:
+10:
   invoke void @f2()
   to label %11 unwind label %1
 
-; :11:
+11:
   %12 = invoke signext i32 undef(i32* null, i32 signext undef, i1 zeroext undef)
   to label %13 unwind label %14
 
-; :13:
+13:
   unreachable
 
-; :14:
+14:
   %15 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :16:
+16:
   unreachable
 
-; :17:
+17:
   ret void
 
 ; uselistorder directives
Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/

[PATCH] D62035: [AST] const-ify ObjC inherited class search

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I don't really have much to say about this, and the patch is probably fine, but 
I do note that most of the other accessors on this class also return mutable 
objects.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62035/new/

https://reviews.llvm.org/D62035



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60748: Fix i386 struct and union parameter alignment

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I don't think this was correct (where by "correct", there, I mean "what GCC 
does", as this patch is intended to match GCC behavior).

I think this change may well break more cases than it fixes, so IMO, this 
should be reverted, until it's implemented properly.

Consider one example:

  #include 
  
  typedef __attribute__((aligned(16))) int alignedint;
  
  struct __attribute__((aligned(64))) X {
  int x;
  //alignedint y;
  //__m128 y;
  };
  void g(int x, struct X);
  
  _Static_assert(_Alignof(struct X) == 64);
  
  struct X gx;
  
  void f() {
  g(1, gx);
  }

Note that when compiling this as is GCC does _not_ align X when calling g(). 
But, as of this change, now clang does. If you uncomment either the __m128 or 
alignedint lines, and now GCC aligns to 64 bytes too.

This is because GCC's algorithm is a whole lot more complex than what you've 
implemented. See its function ix86_function_arg_boundary.

The way I interpret GCC, it's doing effectively the following:
StackAlignmentForType(T):

1. If T's alignment is < 16 bytes, return 4.
2. If T is a struct/union/array type, then:
  - recursively call StackAlignmentForType() on each member's type (note -- 
this ignores any attribute((aligned(N))) directly on the //fields// of a 
struct, but not those that appear on typedefs, or the underlying types).
  - If all of those calls return alignments < 16, then return 4.
3. Otherwise, return the alignment of T.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60748/new/

https://reviews.llvm.org/D60748



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64487: [clang, test] Fix Clang :: Headers/max_align.c on 64-bit SPARC

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64487/new/

https://reviews.llvm.org/D64487



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Is this really necessary? Users don't typically pass -std= to the driver for 
linking anyways (what do you even pass if you've compiled both C and C++ code?) 
so this seems a rather odd way to control behavior.

How about instead just adding "values-xpg6.o" unconditionally, alongside the 
current unconditional "values-Xa.o", and just forget about the xpg4 and Xc 
modes?




Comment at: lib/Driver/ToolChains/Solaris.cpp:16
 #include "clang/Driver/Options.h"
+#include "clang/Frontend/LangStandard.h"
 #include "llvm/Option/ArgList.h"

I'm not sure that's an acceptable dependency -- I think Driver probably is not 
supposed to depend on Frontend. If so, I guess LangStandard should move to 
clang/Basic/. Which also means moving InputKind from 
clang/include/clang/Frontend/FrontendOptions.h.

(Maybe someone else can weigh in on this question?)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64793/new/

https://reviews.llvm.org/D64793



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

> I fear it is necessary: at least it matches documented behaviour of both the 
> Sun/Oracle Studio compilers and gcc.

I will defer to your opinion here. But -- one last attempt at dissuading you. :)

Is this really doing something _important_, or is it just legacy cruft which 
can be safely ignored by now? With your "logb" example, it seems to me that it 
is probably preferable to always use the new correct "xpg6" implementation, and 
just ignore the legacy/incorrect version. Similarly, the example given in 
https://gcc.gnu.org/PR40411 of freopen -- again, seems like it'd be better to 
just use the new xpg6 behavior, always.

> The -std= options usually get passed to the linking step because CFLAGS is 
> added to the options as well

With gnu make they are not (unless it's doing a single-step compiling+linking 
step). Other tools like CMake also don't pass standards versions to linking. 
This makes sense, because a program can contain code compiled with multiple 
standards versions, and multiple languages. Thus, I'd expect most users to just 
get the default xpg6 and Xa objects, even if they do specify -std=c90 for 
compilation.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64793/new/

https://reviews.llvm.org/D64793



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I'm not sure the history behind why these were added as default-on 
warningsthey don't really seem appropriate as default warnings to me, 
either.

But I think someone else probably ought to approve the change.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65192/new/

https://reviews.llvm.org/D65192



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65582: IR: accept and print numbered %N names for function args

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.
Herald added a subscriber: wuzish.

+1 for doing this. I started looking at fixing this when I modified the printer 
to print proper labels for numbered basic-blocks (instead of comments), but I 
didn't do so because of the amount of test churn was off-putting.

I think that after this change, there's only one local entity left which uses a 
local-value-number but doesn't print it: the entry block. That also would cause 
a quite-large amount of test-churn to add.




Comment at: llvm/lib/IR/AsmWriter.cpp:3561
+else
+  Out << "";
   }

I think you need a space before this string? Although, then shouldn't 
llvm/unittests/IR/AsmWriterTest.cpp be failing? (it has a space there...)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65582/new/

https://reviews.llvm.org/D65582



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62268: Add back --sysroot support for darwin header search.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: ldionne, jfb.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Before e97b5f5cf37e 
 
([clang][Darwin] Refactor header search path logic
into the driver), both --sysroot and -isysroot worked to specify where
to look for system and C++ headers on Darwin. However, that change
caused clang to start ignoring --sysroot.

This fixes the regression, and adds tests.

(I also note that on all other platforms, clang seems to almost
completely ignore -isysroot, but that's another issue...)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62268

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-header-search-system.cpp

Index: clang/test/Driver/darwin-header-search-system.cpp
===
--- clang/test/Driver/darwin-header-search-system.cpp
+++ clang/test/Driver/darwin-header-search-system.cpp
@@ -3,7 +3,8 @@
 // General tests that the system header search paths detected by the driver
 // and passed to CC1 are correct on Darwin platforms.
 
-// Check system headers (everything below  and )
+// Check system headers (everything below  and ).  Ensure
+// that both sysroot and isysroot are checked, and that isysroot has precedence.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
 // RUN: -target x86_64-apple-darwin \
@@ -13,6 +14,26 @@
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN:   -DRESOURCE=%S/Inputs/resource_dir \
 // RUN:   --check-prefix=CHECK-SYSTEM %s
+//
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: --sysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
+// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
+// RUN:   --check-prefix=CHECK-SYSTEM %s
+//
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
+// RUN: --sysroot / \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
+// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
+// RUN:   --check-prefix=CHECK-SYSTEM %s
+//
 // CHECK-SYSTEM: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
 // CHECK-SYSTEM: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-SYSTEM: "-internal-isystem" "[[RESOURCE]]/include"
Index: clang/test/Driver/darwin-header-search-libcxx.cpp
===
--- clang/test/Driver/darwin-header-search-libcxx.cpp
+++ clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -36,6 +36,7 @@
 
 // Check with both headers in the sysroot and headers alongside the installation
 // (the headers in  should be added after the toolchain headers).
+// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
 // RUN: -target x86_64-apple-darwin \
@@ -46,6 +47,28 @@
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
+//
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: --sysroot %S/Inputs/basic_darwin_sdk_usr \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
+// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
+//
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr \
+// RUN: --sysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
+// RUN:   --check-prefix=CHECK-LIBCX

[PATCH] D62268: Add back --sysroot support for darwin header search.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D62268#1512672 , @ldionne wrote:

> This LGTM.
>
> When I did the refactor, all the code was only using `-isysroot` (and never 
> accessing `--sysroot`), so I thought only `-isysroot` was relevant on Darwin. 
> Seems like I was wrong.


The isysroot argument to the cc1 compiler invocation got populated from both 
isysroot and --sysroot in the driver, so that's why the behavior changed here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62268/new/

https://reviews.llvm.org/D62268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62268: Add back --sysroot support for darwin header search.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC361429: Add back --sysroot support for darwin header search. 
(authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62268?vs=200813&id=200815#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62268/new/

https://reviews.llvm.org/D62268

Files:
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  test/Driver/darwin-header-search-libcxx.cpp
  test/Driver/darwin-header-search-system.cpp

Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1805,13 +1805,21 @@
   }
 }
 
+// Returns the effective header sysroot path to use. This comes either from
+// -isysroot or --sysroot.
+llvm::StringRef DarwinClang::GetHeaderSysroot(const llvm::opt::ArgList &DriverArgs) const {
+  if(DriverArgs.hasArg(options::OPT_isysroot))
+return DriverArgs.getLastArgValue(options::OPT_isysroot);
+  if (!getDriver().SysRoot.empty())
+return getDriver().SysRoot;
+  return "/";
+}
+
 void DarwinClang::AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const {
   const Driver &D = getDriver();
 
-  llvm::StringRef Sysroot = "/";
-  if (const Arg *A = DriverArgs.getLastArg(options::OPT_isysroot))
-Sysroot = A->getValue();
+  llvm::StringRef Sysroot = GetHeaderSysroot(DriverArgs);
 
   bool NoStdInc = DriverArgs.hasArg(options::OPT_nostdinc);
   bool NoStdlibInc = DriverArgs.hasArg(options::OPT_nostdlibinc);
@@ -1897,12 +1905,7 @@
   DriverArgs.hasArg(options::OPT_nostdincxx))
 return;
 
-  llvm::SmallString<128> Sysroot;
-  if (const Arg *A = DriverArgs.getLastArg(options::OPT_isysroot)) {
-Sysroot = A->getValue();
-  } else {
-Sysroot = "/";
-  }
+  llvm::StringRef Sysroot = GetHeaderSysroot(DriverArgs);
 
   switch (GetCXXStdlibType(DriverArgs)) {
   case ToolChain::CST_Libcxx: {
Index: lib/Driver/ToolChains/Darwin.h
===
--- lib/Driver/ToolChains/Darwin.h
+++ lib/Driver/ToolChains/Darwin.h
@@ -539,6 +539,8 @@
llvm::StringRef Version,
llvm::StringRef ArchDir,
llvm::StringRef BitDir) const;
+
+  llvm::StringRef GetHeaderSysroot(const llvm::opt::ArgList &DriverArgs) const;
 };
 
 } // end namespace toolchains
Index: test/Driver/darwin-header-search-libcxx.cpp
===
--- test/Driver/darwin-header-search-libcxx.cpp
+++ test/Driver/darwin-header-search-libcxx.cpp
@@ -36,6 +36,7 @@
 
 // Check with both headers in the sysroot and headers alongside the installation
 // (the headers in  should be added after the toolchain headers).
+// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence.
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
 // RUN: -target x86_64-apple-darwin \
@@ -46,6 +47,28 @@
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
+//
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: --sysroot %S/Inputs/basic_darwin_sdk_usr \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
+// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
+//
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr \
+// RUN: --sysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
+// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
+//
 // CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
 // CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
 // CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
Index: test/Driver/darwin-header-search-system.cpp
===
--- test/Driver/darwin-head

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D54355#1328557 , @void wrote:

> The issue is that "`n`" is expecting an immediate value, but the result of 
> the ternary operator isn't calculated by the front-end, because it doesn't 
> "know" that the evaluation of `__builtin_constant_p` shouldn't be delayed (it 
> being compiled at `-O0`). I suspect that this issue will also happen with the 
> "`i`" constraint.


Indeed. We _do_ actually already know that in clang too, however -- clang 
already knows that "n" requires an immediate, and does constant expression 
evaluation for it, e.g. to expand constexpr functions. Grep for 
"requiresImmediateConstant". I guess it's just not hooked up to the __b_c_p 
correctly, though.

(Note, as a workaround, you could use `|` instead of `||`, or put the 
expression as the value of an enumeration constant).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54355/new/

https://reviews.llvm.org/D54355



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55616: Emit ASM input in a constant context

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

This seems like a good start, but not complete.

"n" and "i" both should require that their argument is a constant expression. 
For "n", it actually must be an immediate constant integer, so 
setRequiresImmediate() should be used there. For "i", you may use an lvalue 
constant as well. The way we seem to indicate that, today, is with `if 
(!Info.allowsRegister() && !Info.allowsMemory())`. However the code in that 
block does not today *require* that the evaluation as a constant succeed...and 
it should.

It should also only require that the result is an integer when 
`requiresImmediateConstant()`.

Additionally, the Sema code needs to have the same conditions as the CodeGen 
code for when a constant expression is required, but it doesn't. (before or 
after your change).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55616/new/

https://reviews.llvm.org/D55616



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

That example cannot be expected to ever evaluate the expression as "1" -- it 
doesn't in GCC, nor should it in Clang. An asm constraint of "n" or "i" (but 
not, e.g., "nr") must require a constant expression, and evaluating the 
argument as a constant expression necessarily means always evaluating it to -1.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54355/new/

https://reviews.llvm.org/D54355



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53199/new/

https://reviews.llvm.org/D53199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67573: Fix __atomic_is_lock_free's return type.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.
Herald added subscribers: cfe-commits, jfb, kristof.beyls.
Herald added a project: clang.

It is specified to return a bool in GCC's documentation and
implementation, but the clang builtin says it returns an int. This
would be pretty much harmless, if it was just the builtin.

However, when clang translates the builtin into a libcall, it _also_
generates the libcall with an int return type, while the actual
library function in libatomic returns a bool.

This mismatch in return types results in an actual ABI mismatch and
thus incorrect results (interpreting false return value as true) on at
least x86_64.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67573

Files:
  clang/include/clang/Basic/Builtins.def
  clang/test/CodeGen/atomic-ops.c
  clang/test/CodeGen/big-atomic-ops.c


Index: clang/test/CodeGen/big-atomic-ops.c
===
--- clang/test/CodeGen/big-atomic-ops.c
+++ clang/test/CodeGen/big-atomic-ops.c
@@ -198,20 +198,20 @@
 int lock_free(struct Incomplete *incomplete) {
   // CHECK: @lock_free
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 3, i8* null)
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i64 3, i8* null)
   __c11_atomic_is_lock_free(3);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 16, i8* {{.*}}@sixteen{{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i64 16, i8* 
{{.*}}@sixteen{{.*}})
   __atomic_is_lock_free(16, &sixteen);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 17, i8* {{.*}}@seventeen{{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i64 17, i8* 
{{.*}}@seventeen{{.*}})
   __atomic_is_lock_free(17, &seventeen);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 4, {{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i64 4, {{.*}})
   __atomic_is_lock_free(4, incomplete);
 
   char cs[20];
-  // CHECK: call i32 @__atomic_is_lock_free(i64 4, {{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i64 4, {{.*}})
   __atomic_is_lock_free(4, cs+1);
 
   // CHECK-NOT: call
Index: clang/test/CodeGen/atomic-ops.c
===
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -343,20 +343,20 @@
 int lock_free(struct Incomplete *incomplete) {
   // CHECK-LABEL: @lock_free
 
-  // CHECK: call i32 @__atomic_is_lock_free(i32 3, i8* null)
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i32 3, i8* null)
   __c11_atomic_is_lock_free(3);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i32 16, i8* {{.*}}@sixteen{{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i32 16, i8* 
{{.*}}@sixteen{{.*}})
   __atomic_is_lock_free(16, &sixteen);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i32 17, i8* {{.*}}@seventeen{{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i32 17, i8* 
{{.*}}@seventeen{{.*}})
   __atomic_is_lock_free(17, &seventeen);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i32 4, {{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i32 4, {{.*}})
   __atomic_is_lock_free(4, incomplete);
 
   char cs[20];
-  // CHECK: call i32 @__atomic_is_lock_free(i32 4, {{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i32 4, {{.*}})
   __atomic_is_lock_free(4, cs+1);
 
   // CHECK-NOT: call
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -720,7 +720,7 @@
 ATOMIC_BUILTIN(__c11_atomic_fetch_xor, "v.", "t")
 BUILTIN(__c11_atomic_thread_fence, "vi", "n")
 BUILTIN(__c11_atomic_signal_fence, "vi", "n")
-BUILTIN(__c11_atomic_is_lock_free, "iz", "n")
+BUILTIN(__c11_atomic_is_lock_free, "bz", "n")
 
 // GNU atomic builtins.
 ATOMIC_BUILTIN(__atomic_load, "v.", "t")
@@ -748,7 +748,7 @@
 BUILTIN(__atomic_thread_fence, "vi", "n")
 BUILTIN(__atomic_signal_fence, "vi", "n")
 BUILTIN(__atomic_always_lock_free, "izvCD*", "n")
-BUILTIN(__atomic_is_lock_free, "izvCD*", "n")
+BUILTIN(__atomic_is_lock_free, "bzvCD*", "n")
 
 // OpenCL 2.0 atomic builtins.
 ATOMIC_BUILTIN(__opencl_atomic_init, "v.", "t")


Index: clang/test/CodeGen/big-atomic-ops.c
===
--- clang/test/CodeGen/big-atomic-ops.c
+++ clang/test/CodeGen/big-atomic-ops.c
@@ -198,20 +198,20 @@
 int lock_free(struct Incomplete *incomplete) {
   // CHECK: @lock_free
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 3, i8* null)
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i64 3, i8* null)
   __c11_atomic_is_lock_free(3);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 16, i8* {{.*}}@sixteen{{.*}})
+  // CHECK: call zeroext i1 @__atomic_is_lock_free(i64 16, i8* {{.*}}@sixteen{{.*}})
   __atomic_is_lock_free(16, &sixteen);
 
-  // CHECK: call i32 @__atomic_is_lock_free(i64 17, i8* {{.*}}@seventeen{{.*}})
+  // CHECK: call zeroext i1 @__