[PATCH] D50199: [MinGW] Predefine UNICODE if -municode is specified during compilation
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! https://reviews.llvm.org/D50199 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50261: [AST] Remove unnecessary indirections in DeclarationNameTable
rnk added a comment. Neat, thanks for the optimization. My only concern would be to find a way to avoid including Type.h, but it's already a very popular and very necessary header, so I don't think there is any issue here. Repository: rL LLVM https://reviews.llvm.org/D50261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50055: Update the coding standard about NFC changes and whitespace
rnk added a comment. In https://reviews.llvm.org/D50055#1188479, @chandlerc wrote: > Maybe double check with Reid and/or Hal to make sure we've all ended up on > the same page before landing though. lgtm, thanks https://reviews.llvm.org/D50055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence
rnk added a subscriber: beanz. rnk added a comment. In https://reviews.llvm.org/D15225#1191304, @george.karpenkov wrote: > @rnk As discussed, would it be acceptable for you to just have empty > sanitizer runtime files in the resource directory? I was talking to @beanz, and he suggested adding a cmake flag, something like CLANG_UNSUPPORTED_SANITIZERS=asan;ubsan;tsan;msan etc to control this. Alternatively, it could be a positive list of supported sanitizers, whatever is preferable. I think my main objection to this is that while it's convenient from a packaging perspective, it ascribes too much significance to the existence or non-existence of some library files that aren't needed during compilation in the first place. Changing the wording from "not supported" to "not available" doesn't seem that helpful. It would still lead someone down the path of needing to read the clang source code to understand that some library files are missing, whereas a link error would be more obvious. It's also easy to imagine scenarios where the user has a slightly non-standard link setup and the runtime library ultimately doesn't come from the resource directory. For example, users checking out compiler-rt and building these libraries themselves, perhaps with additional instrumentation. Overall, I feel like this is too tight coupling. Repository: rL LLVM https://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence
rnk added a comment. I got some actual data. The way we package clang today, the extracted installation is 134.83M, and lib/clang/7.0.0/lib/darwin/* is 13M, so it's a 10% increase. It would be worth it to us to replace these with empty files if this change does land, but again, I'd rather not go this direction, which would require special logic just for the darwin parts of compiler-rt. Repository: rL LLVM https://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50513: clang-cl: Support /guard:cf,nochecks
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm! There is some additional work to do to put .gfids$y sections into appropriate comdat sections, but that's future work. https://reviews.llvm.org/D50513 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY
rnk added a comment. In https://reviews.llvm.org/D49240#1195237, @ldionne wrote: > In https://reviews.llvm.org/D49240#1195125, @thakis wrote: > > > When we updated out clang bundle in chromium (which includes libc++ > > headers), our ios simulator bots regressed debug info size by ~50% due to > > this commit > > (https://bugs.chromium.org/p/chromium/issues/detail?id=872926#c13). Is that > > expected? > > > No, this is quite surprising. What happened with the size of the Release > builds? We should investigate, perhaps this change exposed something in > Clang's debug info generation. It looks like the increase is entirely from more symbols in the symbol table. It's not a problem with clang's debug info. It would help a lot if ld64 de-duplicated strings in the symbol table, if that's possible. $ bloaty after_base_unittests -- before_base_unittests VM SIZE FILE SIZE -- -- +103% +51.4Mi String Table +51.4Mi +103% +105% +17.6Mi Symbol Table +17.6Mi +105% +49% +827Ki Code Signature +827Ki +49% +92% +263Ki Function Start Addresses +263Ki +92% +15% +4.50Ki __TEXT,__unwind_info +4.50Ki +15% +74% +3.52Ki Table of Non-instructions +3.52Ki +74% -0.0%-128 __TEXT,__const -128 -0.0% -2.0%-136 [__TEXT] -136 -2.0% [ = ] 0 [Unmapped] -480 -3.4% -1.0%-516 __TEXT,__gcc_except_tab -516 -1.0% -39.3% -1.07Ki [__LINKEDIT] +4 +33% -9.8% -5.61Mi __TEXT,__text -5.61Mi -9.8% +50% +64.5Mi TOTAL +64.5Mi +50% Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY
rnk added a comment. In https://reviews.llvm.org/D49240#1195733, @ldionne wrote: > Ah, thanks a lot for taking a look! Yes, this makes a lot of sense, since now > we're not inlining everything anymore. So the code size is actually smaller > (-9.8%), but there's more symbols because more functions are emitted. In this > case, I would say this is expected, if unfortunate. Also, a similar effect > would probably be witnessed if Chromium were to change their standard library > to libstdc++, for example, since libstdc++ does not abuse inlining like > libc++ used to. I think if we used libstdc++, the situation would be much better because the inline functions would be linkonce_odr, and there would be far fewer symbols in the symbol table. We'd get code size wins from deduplicating the code, and symbol table size wins from dropping duplicate long mangled names. Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50564: Add support for SEH unwinding on Windows.
rnk added inline comments.
Comment at: src/libunwind_ext.h:43
+ #if defined(__x86_64__) && !defined(__MINGW64__)
+typedef struct _DISPATCHER_CONTEXT {
+ ULONG64 ControlPc;
cdavis5x wrote:
> mstorsjo wrote:
> > What's this all about? winnt.h (from both MSVC and mingw-w64) should define
> > this struct, no?
> Not my copy of `` from the Win7 SDK. Dunno about WIn8 and WIn10.
If we have to pick between SDK versions to support, I'd much prefer to support
8+.
Repository:
rUNW libunwind
https://reviews.llvm.org/D50564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI
rnk added a comment. I'd prefer not to do this, since internal_linkage generates smaller, more debuggable code by default. I think the symbol table size increase may be specific to MachO, and it may be possible to work around this by changing ld64 to pool strings for symbols by default. I don't know enough about MachO to say if this is possible. I also think the symbol table size problem may be limited to "large" users of C++: people with 500+ object files using libc++ in a DSO. I'm more comfortable imposing a cost on these users to ask them to opt in to some setting that disables internal_linkage and always_inline than I am leaving always_inline on by default, which adversely affects all users. Repository: rCXX libc++ https://reviews.llvm.org/D50652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI
rnk added a comment. In https://reviews.llvm.org/D50652#1197780, @ldionne wrote: > One thing to keep in mind is that we do not have a solution that allows > removing both `internal_linkage` and `always_inline`. It's either > `internal_linkage` or `always_inline`, but you can't get rid of both until we > fix some problems with extern template declarations and visibility > attributes. What I can do, however, is reverse the default to use > `internal_linkage`, and then have a temporary hook that allows Chromium to > stay on `always_inline`. In the future, we'd remove that hook and the choice > would be between `internal_linkage` and nothing at all. It's probably worth it to me to debug and understand that problem. Is there a good explanation of it? Also, if a user could define _LIBCPP_ABI_UNSTABLE, does this problem still apply? Could we remove the problematic visibility attributes if the ABI is unstable? Repository: rCXX libc++ https://reviews.llvm.org/D50652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50547: [Driver] Use normalized triples for multiarch runtime path
rnk added a comment. Would it be to much to check both the normalized and non-normalized? Repository: rC Clang https://reviews.llvm.org/D50547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50678: [InlineAsm] Update the min-legal-vector-width function attribute based on inputs and outputs to inline assembly
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D50678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50526: Model type attributes as regular Attrs
rnk added a comment. I suspect that somehow this change broke compiling ATL: https://ci.chromium.org/buildbot/chromium.clang/ToTWin64%28dbg%29/993 https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.clang%2FToTWin64_dbg_%2F995%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Flogs%2Fraw_io.output_failure_summary_%2F0 Now we get these diagnostics: [1011/50166] CXX obj/chrome/chrome_cleaner/http/http/error_utils.obj FAILED: obj/chrome/chrome_cleaner/http/http/error_utils.obj ... In file included from ../../chrome/chrome_cleaner/http/error_utils.cc:11: In file included from ../..\base/win/atl.h:18: In file included from ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlctl.h:33: In file included from ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlwin.h:5107: ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2777,9): error: no matching function for call to 'AtlAxDialogCreateT' return AtlAxDialogCreateT( ^~~~ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2715,29): note: candidate template ignored: invalid explicitly-specified argument for template parameter 'pFunc' typename Helper::ReturnType AtlAxDialogCreateT( ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2788,9): error: no matching function for call to 'AtlAxDialogCreateT' return AtlAxDialogCreateT( ^~~ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2715,29): note: candidate template ignored: invalid explicitly-specified argument for template parameter 'pFunc' typename Helper::ReturnType AtlAxDialogCreateT( ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2799,9): error: no matching function for call to 'AtlAxDialogCreateT' return AtlAxDialogCreateT( ^~~~ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2715,29): note: candidate template ignored: invalid explicitly-specified argument for template parameter 'pFunc' typename Helper::ReturnType AtlAxDialogCreateT( ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2810,9): error: no matching function for call to 'AtlAxDialogCreateT' return AtlAxDialogCreateT(hInstance, lpTemplateName, hWndParent, lpDialogProc, dwInitParam); ^~~ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2715,29): note: candidate template ignored: invalid explicitly-specified argument for template parameter 'pFunc' typename Helper::ReturnType AtlAxDialogCreateT( ^ 4 errors generated. Repository: rC Clang https://reviews.llvm.org/D50526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50526: Model type attributes as regular Attrs
rnk added a comment. I went ahead and reverted this in https://reviews.llvm.org/rC339638, and will produce a reduction soon. Repository: rC Clang https://reviews.llvm.org/D50526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50526: Model type attributes as regular Attrs
rnk added a comment.
The issue was related to ignored calling convention attributes on Win64:
int a(int, const int *, int, int, __int64);
class b {
public:
typedef int c;
};
template
void AtlAxDialogCreateT(int, d, int, int, __int64);
int g, i, j, k;
char *h;
void l() { AtlAxDialogCreateT(g, h, i, j, k); }
Compiled with:
clang -c -fms-extensions -fms-compatibility --target=x86_64-windows-msvc
Repository:
rC Clang
https://reviews.llvm.org/D50526
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50564: Add support for SEH unwinding on Windows.
rnk added a comment.
In https://reviews.llvm.org/D50564#1198996, @cdavis5x wrote:
> Could somebody verify that the `DISPATCHER_CONTEXT` struct is defined in
> `` for the Win8 and Win10 SDKs? I can't install them right now.
I checked both, and they are available, so I think we should guard the
definition like this:
// Provide a definition for _DISPATCHER_CONTEXT for Win7 and earlier SDK
versions.
// Mingw64 has always provided this struct.
#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && !defined(__MINGW64__) &&
defined(WINVER) && WINVER < _WIN32_WINNT_WIN8
# if defined(__x86_64__)
typedef struct _DISPATCHER_CONTEXT { ... } DISPATCHER_CONTEXT;
# elif defined(__arm__)
typedef struct _DISPATCHER_CONTEXT { ... } DISPATCHER_CONTEXT;
# endif
#endif
Does that seem right? I'm not super familiar with SDK version detection, so I
might have the wrong macros.
Repository:
rUNW libunwind
https://reviews.llvm.org/D50564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50805: Don't warn on returning the address of a label
rnk created this revision.
rnk added reviewers: niravd, rsmith, nickdesaulniers.
There aren't any lifetime issues inherent in returning a local label.
Hypothetically, it could be used to drive a state machine driven by
computed goto the next time that same scope is re-entered. In any case,
the Linux kernel uses this code pattern to get PCs, and we don't want to
warn on that code pattern.
Fixes PR38569
https://reviews.llvm.org/D50805
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaInit.cpp
clang/test/Analysis/stack-addr-ps.cpp
clang/test/Sema/statements.c
Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -29,9 +29,22 @@
}
-void *test10() {
+// PR38569: Clang used to warn on returning the address of a label, but we
don't
+// anymore. Labels aren't exactly destroyed when they go out of scope, and the
+// Linux kernel uses this functionality.
+void *test10() {
bar:
- return &&bar; // expected-warning {{returning address of label, which is
local}}
+ return &&bar;
+}
+
+// Linux actually does something more like this, and we don't want to warn on
+// it.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
}
// PR6034
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -74,10 +74,12 @@
return &x; // expected-warning{{Address of stack memory associated with
local variable 's1' returned}} expected-warning {{address of stack memory
associated with local variable 's1' returned}}
}
+// PR38569: Clang used to warn when returning label addresses, but now it
+// doesn't.
void *lf() {
label:
-void *const &x = &&label; // expected-note {{binding reference variable
'x' here}}
-return x; // expected-warning {{returning address of label, which is
local}}
+void *const &x = &&label;
+return x;
}
template
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6291,7 +6291,6 @@
/// A temporary or local variable. This will be one of:
/// * A MaterializeTemporaryExpr.
/// * A DeclRefExpr whose declaration is a local.
-/// * An AddrLabelExpr.
/// * A BlockExpr for a block with captures.
using Local = Expr*;
@@ -6735,11 +6734,6 @@
}
break;
- case Stmt::AddrLabelExprClass:
-// We want to warn if the address of a label would escape the function.
-Visit(Path, Local(cast(Init)), RK_ReferenceBinding);
-break;
-
default:
break;
}
@@ -6923,8 +6917,6 @@
<< isa(DRE->getDecl()) << DiagRange;
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
- } else if (isa(L)) {
-Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
<< Entity.getType()->isReferenceType() << DiagRange;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7869,9 +7869,6 @@
def warn_ret_local_temp_addr_ref : Warning<
"returning %select{address of|reference to}0 local temporary object">,
InGroup;
-def warn_ret_addr_label : Warning<
- "returning address of label, which is local">,
- InGroup;
def err_ret_local_block : Error<
"returning block that lives on the local stack">;
def note_local_var_initializer : Note<
Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -29,9 +29,22 @@
}
-void *test10() {
+// PR38569: Clang used to warn on returning the address of a label, but we don't
+// anymore. Labels aren't exactly destroyed when they go out of scope, and the
+// Linux kernel uses this functionality.
+void *test10() {
bar:
- return &&bar; // expected-warning {{returning address of label, which is local}}
+ return &&bar;
+}
+
+// Linux actually does something more like this, and we don't want to warn on
+// it.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
}
// PR6034
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -74,10 +74,12 @@
return &x; // expected-warning{{Address of stack memory associated with local variable 's1' returned}} expected-warning {{address of stack memory associated with local variable 's1' returned}}
}
+// PR38569
[PATCH] D50805: Don't warn on returning the address of a label
rnk added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7872-7874 -def warn_ret_addr_label : Warning< - "returning address of label, which is local">, - InGroup; lebedev.ri wrote: > Why completely drop the diagnostic just because it is undesired in linux code? > Why not just add an `-Wreturn-stack-address` diag option instead, and disable > it if undesired? There's just no use for it. There is no actual lifetime issue here. Just because a label goes out of scope doesn't invalidate it for use with some future execution of the scope. Labels aren't variables, we should never have had this check in the first place, IMO. https://reviews.llvm.org/D50805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50805: Don't warn on returning the address of a label
rnk added a comment.
In https://reviews.llvm.org/D50805#1201805, @rsmith wrote:
> There is no guarantee that you can use an address-of-label value from one
> function invocation in another invocation of the same function. GCC's
> documentation says it's the user's own problem to prevent inlining and
> cloning if the program requires an address-of-label extension to always
> produce the same value for multiple invocations of the function. It might
> make sense to suppress the warning in the case where the function is
> `__attribute__((noinline))`, though.
That's interesting. I'm pretty sure LLVM will not inline a function that has
`indirectbr` to avoid this problem.
I think the state machine use case is real, though, something like:
void *f(void *startlabel) {
common_work();
goto *startlabel;
state1:
return &&state2;
state2:
return &&state3;
...
}
I think that, ultimately, this warning isn't worth the code complexity in clang
to support it. Suppressing it under noinline doesn't solve the original
statement expression issue either, and creates more unnecessary code complexity.
Are we really worried about users doing this?
void *f() {
return &&next;
next:
...
}
void g() {
void *pc = f();
goto *pc; // cross-functional frame re-entry! =D
}
https://reviews.llvm.org/D50805
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50805: [Sema] Don't warn on returning the address of a label
rnk added a comment. Sounds good. I think, in the meantime, we all agree we can stop emitting this warning in the statement expression case. I'll upload a patch for that. Repository: rC Clang https://reviews.llvm.org/D50805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50805: [Sema] Don't warn on returning the address of a label
rnk updated this revision to Diff 161096.
rnk added a comment.
- keep the warning, suppress stmt expr case
https://reviews.llvm.org/D50805
Files:
clang/lib/Sema/SemaInit.cpp
clang/test/Sema/statements.c
Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -34,6 +34,15 @@
return &&bar; // expected-warning {{returning address of label, which is
local}}
}
+// PR38569: Don't warn when returning a label from a statement expression.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
+}
+
// PR6034
void test11(int bit) {
switch (bit)
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6923,7 +6923,7 @@
<< isa(DRE->getDecl()) << DiagRange;
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
- } else if (isa(L)) {
+ } else if (isa(L) && LK == LK_Return) {
Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -34,6 +34,15 @@
return &&bar; // expected-warning {{returning address of label, which is local}}
}
+// PR38569: Don't warn when returning a label from a statement expression.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
+}
+
// PR6034
void test11(int bit) {
switch (bit)
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6923,7 +6923,7 @@
<< isa(DRE->getDecl()) << DiagRange;
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
- } else if (isa(L)) {
+ } else if (isa(L) && LK == LK_Return) {
Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50805: Don't warn on returning the address of a label from a statement expression
rnk updated this revision to Diff 161098.
rnk added a comment.
- fix it right
https://reviews.llvm.org/D50805
Files:
clang/lib/Sema/SemaInit.cpp
clang/test/Sema/statements.c
Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -34,6 +34,15 @@
return &&bar; // expected-warning {{returning address of label, which is
local}}
}
+// PR38569: Don't warn when returning a label from a statement expression.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
+}
+
// PR6034
void test11(int bit) {
switch (bit)
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6924,7 +6924,8 @@
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
} else if (isa(L)) {
-Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
+if (LK == LK_Return)
+ Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
<< Entity.getType()->isReferenceType() << DiagRange;
Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -34,6 +34,15 @@
return &&bar; // expected-warning {{returning address of label, which is local}}
}
+// PR38569: Don't warn when returning a label from a statement expression.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
+}
+
// PR6034
void test11(int bit) {
switch (bit)
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6924,7 +6924,8 @@
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
} else if (isa(L)) {
-Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
+if (LK == LK_Return)
+ Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
<< Entity.getType()->isReferenceType() << DiagRange;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces
rnk created this revision.
rnk added reviewers: majnemer, inglorion, hans.
Herald added subscribers: dexonsmith, JDevlieghere, aprantl, mehdi_amini.
This is needed to avoid conflicts in mangled names for codeview types in
anonymous namespaces. In CodeView, types refer to each other typically
through forward declarations, which contain mangled names. These names
have to be unique, otherwise the debugger will look up the mangled name
and find the wrong definition.
Furthermore, ThinLTO will deduplicate the types, and debug info
verification can fail when the types have the wrong sizes. This is
PR38608.
Fixes PR38609.
https://reviews.llvm.org/D50877
Files:
clang/lib/AST/MicrosoftMangle.cpp
clang/test/CodeGenCXX/cfi-cross-dso.cpp
clang/test/CodeGenCXX/cfi-icall.cpp
clang/test/CodeGenCXX/debug-info-thunk.cpp
clang/test/CodeGenCXX/dllexport.cpp
clang/test/CodeGenCXX/mangle-ms.cpp
clang/test/CodeGenCXX/microsoft-abi-structors.cpp
clang/test/CodeGenCXX/microsoft-abi-throw.cpp
clang/test/CodeGenCXX/microsoft-abi-thunks.cpp
clang/test/CodeGenCXX/microsoft-abi-vftables.cpp
clang/test/CodeGenCXX/microsoft-abi-virtual-member-pointers.cpp
clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
clang/test/CodeGenCXX/pragma-init_seg.cpp
clang/test/CodeGenCXX/type-metadata.cpp
Index: clang/test/CodeGenCXX/type-metadata.cpp
===
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -82,8 +82,8 @@
// MS: comdat($"??_7B@@6B0@@"), !type [[B8:![0-9]+]]
// MS: comdat($"??_7B@@6BA@@@"), !type [[A8]]
// MS: comdat($"??_7C@@6B@"), !type [[A8]]
-// MS: comdat($"??_7D@?A@@6BB@@@"), !type [[B8]], !type [[D8:![0-9]+]]
-// MS: comdat($"??_7D@?A@@6BA@@@"), !type [[A8]]
+// MS: comdat($"??_7D@?A0x{{[^@]*}}@@6BB@@@"), !type [[B8]], !type [[D8:![0-9]+]]
+// MS: comdat($"??_7D@?A0x{{[^@]*}}@@6BA@@@"), !type [[A8]]
// MS: comdat($"??_7FA@?1??foo@@YAXXZ@6B@"), !type [[A8]], !type [[FA8:![0-9]+]]
struct A {
@@ -161,7 +161,7 @@
}
// ITANIUM: define internal void @_Z3df1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void df1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"?AUA@@")
@@ -171,7 +171,7 @@
}
// ITANIUM: define internal void @_Z3dg1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?dg1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?dg1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void dg1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"_ZTS1B")
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"?AUB@@")
@@ -181,7 +181,7 @@
}
// ITANIUM: define internal void @_Z3dh1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?dh1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?dh1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void dh1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE]])
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
@@ -191,7 +191,7 @@
}
// ITANIUM: define internal void @_Z3df2PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df2@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df2@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
__attribute__((no_sanitize("cfi")))
void df2(D *d) {
// CFI-NVT-NOT: call i1 @llvm.type.test
@@ -201,7 +201,7 @@
}
// ITANIUM: define internal void @_Z3df3PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df3@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df3@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
__attribute__((no_sanitize("address"))) __attribute__((no_sanitize("cfi-vcall")))
void df3(D *d) {
// CFI-NVT-NOT: call i1 @llvm.type.test
Index: clang/test/CodeGenCXX/pragma-init_seg.cpp
===
--- clang/test/CodeGenCXX/pragma-init_seg.cpp
+++ clang/test/CodeGenCXX/pragma-init_seg.cpp
@@ -28,8 +28,8 @@
namespace internal_init {
namespace {
int x = f();
-// CHECK: @"?x@?A@internal_init@@3HA" = internal global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr.2 = private constant void ()* @"??__Ex@?A@internal_init@@YAXXZ", section ".asdf"
+// CHECK: @"?x@?A0x{{[^@]*}}@internal_init@@3HA" = internal global i32 0, align 4
+// CHECK: @__cxx_init_fn_ptr.2 = private constant void ()* @"??__Ex@?A0x{{[^@]*}}@internal_init@@YAXXZ", section ".asdf"
}
}
Index: clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
===
--- clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
+++ clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
@@ -12,8 +12,8 @@
namespace {
void __attribute__((__swiftcall__)) __attribute__((__used__)) f() { }
}
-// CHECK-DAG: @"?f@?A@@YSXXZ"
-// CHECK-64-DAG: @"?f@?A@@YSXXZ"
+// CHECK-DAG: @"?f@?A0x{{[^@]*}}
[PATCH] D50805: Don't warn on returning the address of a label from a statement expression
rnk updated this revision to Diff 161148.
rnk added a comment.
- return to avoid bad notes
https://reviews.llvm.org/D50805
Files:
clang/lib/Sema/SemaInit.cpp
clang/test/Sema/statements.c
Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -34,6 +34,15 @@
return &&bar; // expected-warning {{returning address of label, which is
local}}
}
+// PR38569: Don't warn when returning a label from a statement expression.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
+}
+
// PR6034
void test11(int bit) {
switch (bit)
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6924,6 +6924,10 @@
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
} else if (isa(L)) {
+// Don't warn when returning a label from a statement expression.
+// Leaving the scope doesn't end its lifetime.
+if (LK == LK_StmtExprResult)
+ return false;
Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -34,6 +34,15 @@
return &&bar; // expected-warning {{returning address of label, which is local}}
}
+// PR38569: Don't warn when returning a label from a statement expression.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
+}
+
// PR6034
void test11(int bit) {
switch (bit)
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6924,6 +6924,10 @@
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
} else if (isa(L)) {
+// Don't warn when returning a label from a statement expression.
+// Leaving the scope doesn't end its lifetime.
+if (LK == LK_StmtExprResult)
+ return false;
Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces
rnk updated this revision to Diff 161293.
rnk added a comment.
- Use xxHash64
https://reviews.llvm.org/D50877
Files:
clang/lib/AST/MicrosoftMangle.cpp
clang/test/CodeGenCXX/cfi-cross-dso.cpp
clang/test/CodeGenCXX/cfi-icall.cpp
clang/test/CodeGenCXX/debug-info-thunk.cpp
clang/test/CodeGenCXX/dllexport.cpp
clang/test/CodeGenCXX/mangle-ms.cpp
clang/test/CodeGenCXX/microsoft-abi-structors.cpp
clang/test/CodeGenCXX/microsoft-abi-throw.cpp
clang/test/CodeGenCXX/microsoft-abi-thunks.cpp
clang/test/CodeGenCXX/microsoft-abi-vftables.cpp
clang/test/CodeGenCXX/microsoft-abi-virtual-member-pointers.cpp
clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
clang/test/CodeGenCXX/pragma-init_seg.cpp
clang/test/CodeGenCXX/type-metadata.cpp
Index: clang/test/CodeGenCXX/type-metadata.cpp
===
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -82,8 +82,8 @@
// MS: comdat($"??_7B@@6B0@@"), !type [[B8:![0-9]+]]
// MS: comdat($"??_7B@@6BA@@@"), !type [[A8]]
// MS: comdat($"??_7C@@6B@"), !type [[A8]]
-// MS: comdat($"??_7D@?A@@6BB@@@"), !type [[B8]], !type [[D8:![0-9]+]]
-// MS: comdat($"??_7D@?A@@6BA@@@"), !type [[A8]]
+// MS: comdat($"??_7D@?A0x{{[^@]*}}@@6BB@@@"), !type [[B8]], !type [[D8:![0-9]+]]
+// MS: comdat($"??_7D@?A0x{{[^@]*}}@@6BA@@@"), !type [[A8]]
// MS: comdat($"??_7FA@?1??foo@@YAXXZ@6B@"), !type [[A8]], !type [[FA8:![0-9]+]]
struct A {
@@ -161,7 +161,7 @@
}
// ITANIUM: define internal void @_Z3df1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void df1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"?AUA@@")
@@ -171,7 +171,7 @@
}
// ITANIUM: define internal void @_Z3dg1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?dg1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?dg1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void dg1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"_ZTS1B")
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"?AUB@@")
@@ -181,7 +181,7 @@
}
// ITANIUM: define internal void @_Z3dh1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?dh1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?dh1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void dh1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE]])
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
@@ -191,7 +191,7 @@
}
// ITANIUM: define internal void @_Z3df2PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df2@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df2@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
__attribute__((no_sanitize("cfi")))
void df2(D *d) {
// CFI-NVT-NOT: call i1 @llvm.type.test
@@ -201,7 +201,7 @@
}
// ITANIUM: define internal void @_Z3df3PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df3@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df3@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
__attribute__((no_sanitize("address"))) __attribute__((no_sanitize("cfi-vcall")))
void df3(D *d) {
// CFI-NVT-NOT: call i1 @llvm.type.test
Index: clang/test/CodeGenCXX/pragma-init_seg.cpp
===
--- clang/test/CodeGenCXX/pragma-init_seg.cpp
+++ clang/test/CodeGenCXX/pragma-init_seg.cpp
@@ -28,8 +28,8 @@
namespace internal_init {
namespace {
int x = f();
-// CHECK: @"?x@?A@internal_init@@3HA" = internal global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr.2 = private constant void ()* @"??__Ex@?A@internal_init@@YAXXZ", section ".asdf"
+// CHECK: @"?x@?A0x{{[^@]*}}@internal_init@@3HA" = internal global i32 0, align 4
+// CHECK: @__cxx_init_fn_ptr.2 = private constant void ()* @"??__Ex@?A0x{{[^@]*}}@internal_init@@YAXXZ", section ".asdf"
}
}
Index: clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
===
--- clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
+++ clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
@@ -12,8 +12,8 @@
namespace {
void __attribute__((__swiftcall__)) __attribute__((__used__)) f() { }
}
-// CHECK-DAG: @"?f@?A@@YSXXZ"
-// CHECK-64-DAG: @"?f@?A@@YSXXZ"
+// CHECK-DAG: @"?f@?A0x{{[^@]*}}@@YSXXZ"
+// CHECK-64-DAG: @"?f@?A0x{{[^@]*}}@@YSXXZ"
namespace n {
void __attribute__((__swiftcall__)) f() {}
@@ -44,8 +44,8 @@
namespace {
void __attribute__((__preserve_most__)) __attribute__((__used__)) g() {}
}
-// CHECK-DAG: @"?g@?A@@YUXXZ"
-// CHECK-64-DAG: @"?g@?A@@YUXXZ"
+// CHECK-DAG: @"?g@?A0x{{[^@]*}}@@YUXXZ"
+// CHECK-64-DAG: @"?g@?A0x{{[^@]*}}@@YUXXZ"
namespace n {
void __attribute__((__preserve_most__)) g() {}
Index: clang/test/CodeGenCXX/microsoft-abi-virtual-member-pointers.cpp
===
[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces
rnk marked an inline comment as done.
rnk added a comment.
Exactly, this makes our names match MSVC more closely. Their hash depends on
the path to the main source file. It doesn't care if the file is in a header.
However, they use the absolute path to the file instead of the (probably
relative) path to the file passed by the build system on the command line. I
think my approach of using the spelling of the path on the command line will
play better with distributed build systems, since you can imagine multiple
distributed workers compiling at different absolute paths.
Comment at: clang/lib/AST/MicrosoftMangle.cpp:389
+ if (FE) {
+llvm::MD5 Hasher;
+llvm::MD5::MD5Result Hash;
inglorion wrote:
> Instead of MD5, can we use xxhash (or whatever the fastest hash algorithm in
> LLVM is these days)? I don't think we need a particular hash algorithm, so we
> might as well go with the fastest one we have (as long as it does a good job
> avoiding collisions, of course).
Sure. It's not visible in the ABI, so we're free to change it at any point
later.
https://reviews.llvm.org/D50877
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50907: Make __shiftleft128 / __shiftright128 real compiler built-ins.
rnk accepted this revision. rnk added a comment. lgtm https://reviews.llvm.org/D50907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces
rnk updated this revision to Diff 161310.
rnk marked an inline comment as done.
rnk added a comment.
- improve comment
https://reviews.llvm.org/D50877
Files:
clang/lib/AST/MicrosoftMangle.cpp
clang/test/CodeGenCXX/cfi-cross-dso.cpp
clang/test/CodeGenCXX/cfi-icall.cpp
clang/test/CodeGenCXX/debug-info-thunk.cpp
clang/test/CodeGenCXX/dllexport.cpp
clang/test/CodeGenCXX/mangle-ms.cpp
clang/test/CodeGenCXX/microsoft-abi-structors.cpp
clang/test/CodeGenCXX/microsoft-abi-throw.cpp
clang/test/CodeGenCXX/microsoft-abi-thunks.cpp
clang/test/CodeGenCXX/microsoft-abi-vftables.cpp
clang/test/CodeGenCXX/microsoft-abi-virtual-member-pointers.cpp
clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
clang/test/CodeGenCXX/pragma-init_seg.cpp
clang/test/CodeGenCXX/type-metadata.cpp
Index: clang/test/CodeGenCXX/type-metadata.cpp
===
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -82,8 +82,8 @@
// MS: comdat($"??_7B@@6B0@@"), !type [[B8:![0-9]+]]
// MS: comdat($"??_7B@@6BA@@@"), !type [[A8]]
// MS: comdat($"??_7C@@6B@"), !type [[A8]]
-// MS: comdat($"??_7D@?A@@6BB@@@"), !type [[B8]], !type [[D8:![0-9]+]]
-// MS: comdat($"??_7D@?A@@6BA@@@"), !type [[A8]]
+// MS: comdat($"??_7D@?A0x{{[^@]*}}@@6BB@@@"), !type [[B8]], !type [[D8:![0-9]+]]
+// MS: comdat($"??_7D@?A0x{{[^@]*}}@@6BA@@@"), !type [[A8]]
// MS: comdat($"??_7FA@?1??foo@@YAXXZ@6B@"), !type [[A8]], !type [[FA8:![0-9]+]]
struct A {
@@ -161,7 +161,7 @@
}
// ITANIUM: define internal void @_Z3df1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void df1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"?AUA@@")
@@ -171,7 +171,7 @@
}
// ITANIUM: define internal void @_Z3dg1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?dg1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?dg1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void dg1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"_ZTS1B")
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata !"?AUB@@")
@@ -181,7 +181,7 @@
}
// ITANIUM: define internal void @_Z3dh1PN12_GLOBAL__N_11DE
-// MS: define internal void @"?dh1@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?dh1@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
void dh1(D *d) {
// TT-ITANIUM: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE]])
// TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(i8* {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
@@ -191,7 +191,7 @@
}
// ITANIUM: define internal void @_Z3df2PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df2@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df2@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
__attribute__((no_sanitize("cfi")))
void df2(D *d) {
// CFI-NVT-NOT: call i1 @llvm.type.test
@@ -201,7 +201,7 @@
}
// ITANIUM: define internal void @_Z3df3PN12_GLOBAL__N_11DE
-// MS: define internal void @"?df3@@YAXPEAUD@?A@@@Z"
+// MS: define internal void @"?df3@@YAXPEAUD@?A0x{{[^@]*}}@@@Z"
__attribute__((no_sanitize("address"))) __attribute__((no_sanitize("cfi-vcall")))
void df3(D *d) {
// CFI-NVT-NOT: call i1 @llvm.type.test
Index: clang/test/CodeGenCXX/pragma-init_seg.cpp
===
--- clang/test/CodeGenCXX/pragma-init_seg.cpp
+++ clang/test/CodeGenCXX/pragma-init_seg.cpp
@@ -28,8 +28,8 @@
namespace internal_init {
namespace {
int x = f();
-// CHECK: @"?x@?A@internal_init@@3HA" = internal global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr.2 = private constant void ()* @"??__Ex@?A@internal_init@@YAXXZ", section ".asdf"
+// CHECK: @"?x@?A0x{{[^@]*}}@internal_init@@3HA" = internal global i32 0, align 4
+// CHECK: @__cxx_init_fn_ptr.2 = private constant void ()* @"??__Ex@?A0x{{[^@]*}}@internal_init@@YAXXZ", section ".asdf"
}
}
Index: clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
===
--- clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
+++ clang/test/CodeGenCXX/msabi-swiftcall-cc.cpp
@@ -12,8 +12,8 @@
namespace {
void __attribute__((__swiftcall__)) __attribute__((__used__)) f() { }
}
-// CHECK-DAG: @"?f@?A@@YSXXZ"
-// CHECK-64-DAG: @"?f@?A@@YSXXZ"
+// CHECK-DAG: @"?f@?A0x{{[^@]*}}@@YSXXZ"
+// CHECK-64-DAG: @"?f@?A0x{{[^@]*}}@@YSXXZ"
namespace n {
void __attribute__((__swiftcall__)) f() {}
@@ -44,8 +44,8 @@
namespace {
void __attribute__((__preserve_most__)) __attribute__((__used__)) g() {}
}
-// CHECK-DAG: @"?g@?A@@YUXXZ"
-// CHECK-64-DAG: @"?g@?A@@YUXXZ"
+// CHECK-DAG: @"?g@?A0x{{[^@]*}}@@YUXXZ"
+// CHECK-64-DAG: @"?g@?A0x{{[^@]*}}@@YUXXZ"
namespace n {
void __attribute__((__preserve_most__)) g() {}
Index: clang/test/CodeGenCXX/microsoft-abi-virtual-member-pointer
[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces
rnk added a comment. In https://reviews.llvm.org/D50877#1204609, @thakis wrote: > Can you explicitly mention that this intentionally doesn't use an absolute > path in MicrosoftMangleContextImpl() or similar? Sure. I also described the issue with codeview that motivates why we want unique names. I think I sent this out hastily at the end of the day yesterday, and I didn't write enough comments to answer all of your questions. Hopefully with these changes it makes more sense. https://reviews.llvm.org/D50877 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340079: [MS] Mangle a hash of the main file path into
anonymous namespaces (authored by rnk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50877?vs=161310&id=161323#toc
Repository:
rC Clang
https://reviews.llvm.org/D50877
Files:
lib/AST/MicrosoftMangle.cpp
test/CodeGenCXX/cfi-cross-dso.cpp
test/CodeGenCXX/cfi-icall.cpp
test/CodeGenCXX/debug-info-thunk.cpp
test/CodeGenCXX/dllexport.cpp
test/CodeGenCXX/mangle-ms.cpp
test/CodeGenCXX/microsoft-abi-structors.cpp
test/CodeGenCXX/microsoft-abi-throw.cpp
test/CodeGenCXX/microsoft-abi-thunks.cpp
test/CodeGenCXX/microsoft-abi-vftables.cpp
test/CodeGenCXX/microsoft-abi-virtual-member-pointers.cpp
test/CodeGenCXX/msabi-swiftcall-cc.cpp
test/CodeGenCXX/pragma-init_seg.cpp
test/CodeGenCXX/type-metadata.cpp
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -29,6 +29,7 @@
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/JamCRC.h"
+#include "llvm/Support/xxhash.h"
#include "llvm/Support/MD5.h"
#include "llvm/Support/MathExtras.h"
@@ -127,10 +128,10 @@
llvm::DenseMap LambdaIds;
llvm::DenseMap SEHFilterIds;
llvm::DenseMap SEHFinallyIds;
+ SmallString<16> AnonymousNamespaceHash;
public:
- MicrosoftMangleContextImpl(ASTContext &Context, DiagnosticsEngine &Diags)
- : MicrosoftMangleContext(Context, Diags) {}
+ MicrosoftMangleContextImpl(ASTContext &Context, DiagnosticsEngine &Diags);
bool shouldMangleCXXName(const NamedDecl *D) override;
bool shouldMangleStringLiteral(const StringLiteral *SL) override;
void mangleCXXName(const NamedDecl *D, raw_ostream &Out) override;
@@ -238,6 +239,12 @@
return Result.first->second;
}
+ /// Return a character sequence that is (somewhat) unique to the TU suitable
+ /// for mangling anonymous namespaces.
+ StringRef getAnonymousNamespaceHash() const {
+return AnonymousNamespaceHash;
+ }
+
private:
void mangleInitFiniStub(const VarDecl *D, char CharCode, raw_ostream &Out);
};
@@ -371,6 +378,34 @@
};
}
+MicrosoftMangleContextImpl::MicrosoftMangleContextImpl(ASTContext &Context,
+ DiagnosticsEngine &Diags)
+: MicrosoftMangleContext(Context, Diags) {
+ // To mangle anonymous namespaces, hash the path to the main source file. The
+ // path should be whatever (probably relative) path was passed on the command
+ // line. The goal is for the compiler to produce the same output regardless of
+ // working directory, so use the uncanonicalized relative path.
+ //
+ // It's important to make the mangled names unique because, when CodeView
+ // debug info is in use, the debugger uses mangled type names to distinguish
+ // between otherwise identically named types in anonymous namespaces.
+ //
+ // These symbols are always internal, so there is no need for the hash to
+ // match what MSVC produces. For the same reason, clang is free to change the
+ // hash at any time without breaking compatibility with old versions of clang.
+ // The generated names are intended to look similar to what MSVC generates,
+ // which are something like "?A0x01234567@".
+ SourceManager &SM = Context.getSourceManager();
+ if (const FileEntry *FE = SM.getFileEntryForID(SM.getMainFileID())) {
+// Truncate the hash so we get 8 characters of hexadecimal.
+uint32_t TruncatedHash = uint32_t(xxHash64(FE->getName()));
+AnonymousNamespaceHash = llvm::utohexstr(TruncatedHash);
+ } else {
+// If we don't have a path to the main file, we'll just use 0.
+AnonymousNamespaceHash = "0";
+ }
+}
+
bool MicrosoftMangleContextImpl::shouldMangleCXXName(const NamedDecl *D) {
if (const FunctionDecl *FD = dyn_cast(D)) {
LanguageLinkage L = FD->getLanguageLinkage();
@@ -785,7 +820,7 @@
if (const NamespaceDecl *NS = dyn_cast(ND)) {
if (NS->isAnonymousNamespace()) {
- Out << "?A@";
+ Out << "?A0x" << Context.getAnonymousNamespaceHash() << '@';
break;
}
}
Index: test/CodeGenCXX/debug-info-thunk.cpp
===
--- test/CodeGenCXX/debug-info-thunk.cpp
+++ test/CodeGenCXX/debug-info-thunk.cpp
@@ -86,7 +86,7 @@
};
}
void C::c() {}
-// CHECK-DAG: DISubprogram{{.*}}linkageName: "?f@C@?A@Test4@@W7EAAXXZ"{{.*}} flags: {{.*}}DIFlagThunk
+// CHECK-DAG: DISubprogram{{.*}}linkageName: "?f@C@?A0x{{[^@]*}}@Test4@@W7EAAXXZ"{{.*}} flags: {{.*}}DIFlagThunk
void C::f() {}
// Force C::f to be used.
Index: test/CodeGenCXX/msabi-swiftcall-cc.cpp
===
--- test/CodeGenCXX/msabi-swiftcall-cc.cpp
+++ test/CodeGenCXX/msabi-swiftcall-cc.cpp
@@ -12,8 +12,8 @@
na
[PATCH] D50924: [CodeGen] add rotate builtins
rnk added a comment. Do you mind updating the _rotl* and _rotr* intrinsics to use the same codegen? They're right above in the switch. https://reviews.llvm.org/D50924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50805: Don't warn on returning the address of a label from a statement expression
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340101: Don't warn on returning the address of a label
from a statement expression (authored by rnk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50805?vs=161148&id=161344#toc
Repository:
rC Clang
https://reviews.llvm.org/D50805
Files:
lib/Sema/SemaInit.cpp
test/Sema/statements.c
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6924,6 +6924,10 @@
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
} else if (isa(L)) {
+// Don't warn when returning a label from a statement expression.
+// Leaving the scope doesn't end its lifetime.
+if (LK == LK_StmtExprResult)
+ return false;
Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
Index: test/Sema/statements.c
===
--- test/Sema/statements.c
+++ test/Sema/statements.c
@@ -34,6 +34,15 @@
return &&bar; // expected-warning {{returning address of label, which is
local}}
}
+// PR38569: Don't warn when returning a label from a statement expression.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
+}
+
// PR6034
void test11(int bit) {
switch (bit)
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6924,6 +6924,10 @@
} else if (isa(L)) {
Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
} else if (isa(L)) {
+// Don't warn when returning a label from a statement expression.
+// Leaving the scope doesn't end its lifetime.
+if (LK == LK_StmtExprResult)
+ return false;
Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else {
Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
Index: test/Sema/statements.c
===
--- test/Sema/statements.c
+++ test/Sema/statements.c
@@ -34,6 +34,15 @@
return &&bar; // expected-warning {{returning address of label, which is local}}
}
+// PR38569: Don't warn when returning a label from a statement expression.
+void test10_logpc(void*);
+void test10a() {
+ test10_logpc(({
+my_pc:
+ &&my_pc;
+ }));
+}
+
// PR6034
void test11(int bit) {
switch (bit)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50924: [CodeGen] add rotate builtins
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Thanks, looks good! https://reviews.llvm.org/D50924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40925: Add option -fkeep-static-consts
rnk added inline comments. Comment at: include/clang/Basic/LangOptions.def:311 +BENIGN_LANGOPT(KeepStaticConsts , 1, 0, "keep static const variables even if unused") + Let's make this a CodeGenOption, since only CodeGen needs to look at it. https://reviews.llvm.org/D40925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40925: Add option -fkeep-static-consts
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm! https://reviews.llvm.org/D40925 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.
rnk added a comment. Remind me what the approximate size overhead of this is? I expect it is negligible, as most symbols are not address taken. Repository: rL LLVM https://reviews.llvm.org/D51049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51026: [CodeGen] Implicitly set stackrealign on the main function, if custom stack alignment is used
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/CodeGen/CodeGenFunction.cpp:989
+CGM.getCodeGenOpts().StackAlignment)
+ Fn->addFnAttr("stackrealign");
+
mstorsjo wrote:
> erichkeane wrote:
> > Is there not an attribute name for this already in LLVM? I guess I'm
> > otherwise fine with this, but would want @craig.topper or @rnk to confirm
> > that we're Ok just sending this string as an attribute.
> I don't see one in llvm/include/llvm/IR/Attributes.td at least, and all other
> occurrances in clang just use the plain string.
Yep, we use the string stackrealign attribute.
Repository:
rC Clang
https://reviews.llvm.org/D51026
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.
rnk added inline comments.
Comment at: lib/CodeGen/CGBlocks.cpp:1276
+InitVar->setSection(".CRT$XCLa");
+CGM.addUsedGlobal(InitVar);
+ }
mstorsjo wrote:
> rjmccall wrote:
> > DHowett-MSFT wrote:
> > > rjmccall wrote:
> > > > Is the priority system not good enough?
> > > My reading of the LLVM language reference leads me to believe it’s only
> > > ordered per-module. If that’s the case, the benefit of emitting into
> > > `XC*` is that it provides guaranteed order over all linker input.
> > >
> > > `llvm.global_ctors` excerpt:
> > >
> > > > The functions referenced by this array will be called in ascending
> > > > order of priority (i.e. lowest first) when the module is loaded.
> > >
> > > Now if the priority system _is_ guaranteed over all linker input, will
> > > that guarantee hold for mixed Clang and CL objects?
> > Init priorities on ELF are preserved in the section name like
> > `.init_array.200`, and the sections get sorted by that in the image — it's
> > really a very similar trick to how it works with `.CRT$XC`. Of the major
> > formats, I think it's just Mach-O that doesn't have any built-in
> > prioritization mechanism across object files. But I don't know if LLVM
> > actually tries to translate init priorities over into `.CRT$XC` suffices
> > when targeting PE/COFF, and arguably that's good: init priorities as
> > presented in LLVM right now are pretty specific to the ELF mechanism.
> > Long-term, maybe `llvm.global_ctors` should be generalized so that on ELF
> > targets it takes an integer priority, on PE/COFF targets it takes a string,
> > and on Mach-O it doesn't take anything at all; but I won't hold up this
> > patch for that.
> >
> > On the other hand, I tend to agree that maybe the best solution is for the
> > backend to just take care of this and automatically create a global
> > initializer to install non-function (or maybe non-function &
> > `unnamed_addr`) `dllimport`ed symbols in global data.
> > But I don't know if LLVM actually tries to translate init priorities over
> > into .CRT$XC suffices when targeting PE/COFF, and arguably that's good:
> > init priorities as presented in LLVM right now are pretty specific to the
> > ELF mechanism. Long-term, maybe llvm.global_ctors should be generalized so
> > that on ELF targets it takes an integer priority, on PE/COFF targets it
> > takes a string, and on Mach-O it doesn't take anything at all; but I won't
> > hold up this patch for that.
>
> FWIW, for the MinGW targets, the same ELF-like priority system is used (or I
> think it's actually an older mechanism previously used on ELF); constructors
> are emitted in `.ctors`, and those with a priority go into sections named
> `.ctors.01234` where the number is 65535 minus the init priority specified.
> By making these zero padded, they get ordered right by the normal
> alphabetical sorting. MinGW CRT startup routines then have extra code to run
> constructors in the right order from the `.ctors` section, in addition to the
> normal `.CRT$XC` ones.
I filed a bug to turn the integers into strings on COFF:
https://bugs.llvm.org/show_bug.cgi?id=38552
Repository:
rC Clang
https://reviews.llvm.org/D50144
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46320: Remove \brief commands from doxygen comments.
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm! (Back from a week long vacation) https://reviews.llvm.org/D46320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46332: [X86] Only enable the __ud2 and __int2c builtins if intrin.h has been included.
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D46332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46300: [Clang] Implement function attribute no_stack_protector.
rnk added inline comments.
Comment at: include/clang/Basic/Attr.td:1494
+def NoStackProtector : InheritableAttr {
+ let Spellings = [GCC<"no_stack_protector">];
+ let Subjects = SubjectList<[Function]>;
aaron.ballman wrote:
> manojgupta wrote:
> > aaron.ballman wrote:
> > > This is not a GCC attribute, so this should use the Clang spelling.
> > >
> > > However, why spell the attribute this way rather than use the GCC
> > > spelling (`optimize("no-stack-protector")`?
> > Thanks, I have changed it to use Clang spelling.
> >
> > Regarding __attribute__((optimize("..."))), it is a generic facility in GCC
> > that works for many optimizer flags.
> > Clang currently does not support this syntax currently instead preferring
> > its own version for some options e.g. -O0.
> > e.g.
> > ```
> > __attribute__((optimize("O0"))) // clang version is
> > __attribute__((optnone))
> > ```
> > If we want to support the GCC syntax, future expectation would be support
> > more flags under this syntax. Is that the path we want to take (I do not
> > know the history related to previous syntax decisions but better GCC
> > compatibility will be a nice thing to have)
> The history of `optnone` predates my involvement with Clang and I've not been
> able to find the original review thread (I did find the one where I gave my
> LGTM on the original patch, but that was a resubmission after the original
> design was signed off).
>
> I'm not keen on attributes that have the same semantics but differ in
> spelling from attributes supported by GCC unless there's a good reason to
> deviate. Given that we've already deviated, I'd like to understand why better
> -- I don't want to push you to implement something we've already decided was
> a poor design, but I also don't want to accept code if we can instead use
> syntax that is compatible with GCC.
I do not think we want to pursue supporting generic optimization flags with
`__attribute__((optimize("...")))`.
Part of the reason we only support `optnone` is that LLVM's pass pipeline is
global to the entire module. It's not trivial to enable optimizations for a
single function, but it is much easier to have optimization passes skip
functions marked `optnone`.
Repository:
rC Clang
https://reviews.llvm.org/D46300
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode
rnk reopened this revision. rnk added a comment. This revision is now accepted and ready to land. Please don't do this, this is actually really problematic, since `#line` directives lose information about what's a system header. That means the result of -E usually won't compile, since Windows headers are typically full of warnings and default-error warnings. Repository: rL LLVM https://reviews.llvm.org/D46520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk updated this revision to Diff 146025.
rnk added a comment.
- don't expose CCEK, use a bool
https://reviews.llvm.org/D43320
Files:
clang/include/clang/AST/Expr.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/dllimport-constexpr.cpp
clang/test/SemaCXX/dllimport-memptr.cpp
Index: clang/test/SemaCXX/dllimport-memptr.cpp
===
--- clang/test/SemaCXX/dllimport-memptr.cpp
+++ clang/test/SemaCXX/dllimport-memptr.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++17 %s
// expected-no-diagnostics
Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++14 %s -verify -fms-extensions -triple x86_64-windows-msvc
+// RUN: %clang_cc1 -std=c++17 %s -verify -fms-extensions -triple x86_64-windows-msvc
+
+__declspec(dllimport) void imported_func();
+__declspec(dllimport) int imported_int;
+struct Foo {
+ void __declspec(dllimport) imported_method();
+};
+
+// Instantiation is OK.
+template struct TemplateFnPtr {
+ static void getit() { FP(); }
+};
+template struct TemplateFnRef {
+ static void getit() { FP(); }
+};
+void instantiate1() {
+ TemplateFnPtr<&imported_func>::getit();
+ TemplateFnRef::getit();
+}
+
+// Check variable template instantiation.
+template struct TemplateIntPtr {
+ static int getit() { return *GI; }
+};
+template struct TemplateIntRef {
+ static int getit() { return GI; }
+};
+int instantiate2() {
+ int r = 0;
+ r += TemplateIntPtr<&imported_int>::getit();
+ r += TemplateIntRef::getit();
+ return r;
+}
+
+// Member pointer instantiation.
+template struct TemplateMemPtr { };
+TemplateMemPtr<&Foo::imported_method> instantiate_mp;
+
+// constexpr initialization doesn't work for dllimport things.
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (*constexpr_import_func)() = &imported_func;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr int *constexpr_import_int = &imported_int;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (Foo::*constexpr_memptr)() = &Foo::imported_method;
+
+// We make dynamic initializers for 'const' globals, but not constexpr ones.
+void (*const const_import_func)() = &imported_func;
+int *const const_import_int = &imported_int;
+void (Foo::*const const_memptr)() = &Foo::imported_method;
+
+// Check that using a non-type template parameter for constexpr global
+// initialization is correctly diagnosed during template instantiation.
+template struct StaticConstexpr {
+ // expected-error@+1{{must be initialized by a constant expression}}
+ static constexpr void (*g_fp)() = FP;
+};
+void instantiate3() {
+ // expected-note@+1 {{requested here}}
+ StaticConstexpr::g_fp();
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5408,9 +5408,8 @@
Expr::EvalResult Eval;
Eval.Diag = &Notes;
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsCoreConstExpr(
+ Eval, T, CCE == Sema::CCEK_TemplateArg, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
// The expression can't be folded, so we can't keep it at this position in
// the AST.
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -704,6 +704,10 @@
/// a constant expression.
EM_PotentialConstantExpression,
+ /// Evaluate as a C++17 non-type template argument, which is a core
+ /// constant expression with a special case for dllimport declarations.
+ EM_TemplateArgument,
+
/// Fold the expression to a constant. Stop if we hit a side-effect that
/// we can't model.
EM_ConstantFold,
@@ -793,6 +797,14 @@
return false;
}
+/// Return true if this declaration is dllimport and we cannot treat the
+/// address as a constant expression. Generally, we do not want to constant
+/// fold dllimport declarations unless they are used in a non-type template
+/// parameter.
+bool isNonConstDllImportDecl(const Decl *D) {
+ return EvalMode != EM_TemplateArgument && D->hasAttr();
+}
+
CallStackFrame *getCallFrame(unsigned CallIndex) {
assert(CallIndex && "no call index in getCallFrame");
// We will eventually hit BottomFrame, which has Index 1, so Frame can't
@@ -845,6 +857,7 @@
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk added inline comments. Comment at: clang/include/clang/AST/Expr.h:670-672 + /// Evaluate an expression that is required to be a core constant expression. + bool EvaluateAsCoreConstExpr(EvalResult &Result, QualType ParamType, + CCEKind CCE, const ASTContext &Ctx) const; rsmith wrote: > Seems strange to pass a converted constant expression kind to an 'evaluate as > core constant expression' function. And it seems like we don't need the kind > here, just an "evaluating for emission w/relocations" vs "evaluating for > mangling" enum. > > Also, we need to evaluate non-type template arguments as constant > expressions, not just as core constant expressions, which the implementation > does, but the name and comment here don't reflect. (The difference is that > you can't result in a pointer/reference to a temporary or automatic / thread > storage duration entity.) So... what are these things? Converted constant expressions? What are we evaluating them as? I guess they're not rvalues or lvalues. https://reviews.llvm.org/D43320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk updated this revision to Diff 146039.
rnk added a comment.
- getting closer
https://reviews.llvm.org/D43320
Files:
clang/include/clang/AST/Expr.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/dllimport-constexpr.cpp
clang/test/SemaCXX/dllimport-memptr.cpp
Index: clang/test/SemaCXX/dllimport-memptr.cpp
===
--- clang/test/SemaCXX/dllimport-memptr.cpp
+++ clang/test/SemaCXX/dllimport-memptr.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++17 %s
// expected-no-diagnostics
Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++14 %s -verify -fms-extensions -triple x86_64-windows-msvc
+// RUN: %clang_cc1 -std=c++17 %s -verify -fms-extensions -triple x86_64-windows-msvc
+
+__declspec(dllimport) void imported_func();
+__declspec(dllimport) int imported_int;
+struct Foo {
+ void __declspec(dllimport) imported_method();
+};
+
+// Instantiation is OK.
+template struct TemplateFnPtr {
+ static void getit() { FP(); }
+};
+template struct TemplateFnRef {
+ static void getit() { FP(); }
+};
+void instantiate1() {
+ TemplateFnPtr<&imported_func>::getit();
+ TemplateFnRef::getit();
+}
+
+// Check variable template instantiation.
+template struct TemplateIntPtr {
+ static int getit() { return *GI; }
+};
+template struct TemplateIntRef {
+ static int getit() { return GI; }
+};
+int instantiate2() {
+ int r = 0;
+ r += TemplateIntPtr<&imported_int>::getit();
+ r += TemplateIntRef::getit();
+ return r;
+}
+
+// Member pointer instantiation.
+template struct TemplateMemPtr { };
+TemplateMemPtr<&Foo::imported_method> instantiate_mp;
+
+// constexpr initialization doesn't work for dllimport things.
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (*constexpr_import_func)() = &imported_func;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr int *constexpr_import_int = &imported_int;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (Foo::*constexpr_memptr)() = &Foo::imported_method;
+
+// We make dynamic initializers for 'const' globals, but not constexpr ones.
+void (*const const_import_func)() = &imported_func;
+int *const const_import_int = &imported_int;
+void (Foo::*const const_memptr)() = &Foo::imported_method;
+
+// Check that using a non-type template parameter for constexpr global
+// initialization is correctly diagnosed during template instantiation.
+template struct StaticConstexpr {
+ // expected-error@+1{{must be initialized by a constant expression}}
+ static constexpr void (*g_fp)() = FP;
+};
+void instantiate3() {
+ // expected-note@+1 {{requested here}}
+ StaticConstexpr::g_fp();
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5407,10 +5407,11 @@
SmallVector Notes;
Expr::EvalResult Eval;
Eval.Diag = &Notes;
+ Expr::ConstExprUsage Usage = CCE == Sema::CCEK_TemplateArg
+ ? Expr::EvaluateForMangling
+ : Expr::EvaluateForCodeGen;
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsConstantExpr(Eval, Usage, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
// The expression can't be folded, so we can't keep it at this position in
// the AST.
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -1720,7 +1720,8 @@
/// value for an address or reference constant expression. Return true if we
/// can fold this expression, whether or not it's a constant expression.
static bool CheckLValueConstantExpression(EvalInfo &Info, SourceLocation Loc,
- QualType Type, const LValue &LVal) {
+ QualType Type, const LValue &LVal,
+ Expr::ConstExprUsage Usage) {
bool IsReferenceType = Type->isReferenceType();
APValue::LValueBase Base = LVal.getLValueBase();
@@ -1753,7 +1754,7 @@
return false;
// A dllimport variable never acts like a constant.
- if (Var->hasAttr())
+ if (Usage == Expr::EvaluateForCodeGen && Var->hasAttr())
return false;
}
if (const auto *FD = dyn_cast(VD)) {
@@ -1767,7 +1768,8 @@
// The C language has
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk added inline comments.
Comment at: clang/include/clang/AST/Expr.h:662
+ /// Indicates how the constant expression will be used.
+ enum ConstExprUsage { EvaluateForCodeGen, EvaluateForMangling };
+
I expect we could come up with a better name, but is this closer to what you
had in mind?
https://reviews.llvm.org/D43320
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
rnk added a comment.
Is it possible to fix this in assignInheritanceModel instead? I'd imagine we'd
get the most recent decl. If that's not the issue, maybe you're fixing the bug
in the right spot, but we need to find out where other class template
attributes are moved from instantiation to real declaration.
Comment at: lib/Sema/SemaTemplate.cpp:7079
+void Sema::CheckCXXRecordDecl(CXXRecordDecl *Record, CXXRecordDecl *Prev)
+{
This seems like an overly-generic name for what it does, which currently is
specific to class template instantiations.
Repository:
rC Clang
https://reviews.llvm.org/D46664
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332018: Allow dllimport non-type template arguments in C++17
(authored by rnk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43320?vs=146039&id=146178#toc
Repository:
rC Clang
https://reviews.llvm.org/D43320
Files:
include/clang/AST/Expr.h
lib/AST/ExprConstant.cpp
lib/Sema/SemaOverload.cpp
test/SemaCXX/dllimport-constexpr.cpp
test/SemaCXX/dllimport-memptr.cpp
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -1720,7 +1720,8 @@
/// value for an address or reference constant expression. Return true if we
/// can fold this expression, whether or not it's a constant expression.
static bool CheckLValueConstantExpression(EvalInfo &Info, SourceLocation Loc,
- QualType Type, const LValue &LVal) {
+ QualType Type, const LValue &LVal,
+ Expr::ConstExprUsage Usage) {
bool IsReferenceType = Type->isReferenceType();
APValue::LValueBase Base = LVal.getLValueBase();
@@ -1753,7 +1754,7 @@
return false;
// A dllimport variable never acts like a constant.
- if (Var->hasAttr())
+ if (Usage == Expr::EvaluateForCodeGen && Var->hasAttr())
return false;
}
if (const auto *FD = dyn_cast(VD)) {
@@ -1767,7 +1768,8 @@
// The C language has no notion of ODR; furthermore, it has no notion of
// dynamic initialization. This means that we are permitted to
// perform initialization with the address of the thunk.
- if (Info.getLangOpts().CPlusPlus && FD->hasAttr())
+ if (Info.getLangOpts().CPlusPlus && Usage == Expr::EvaluateForCodeGen &&
+ FD->hasAttr())
return false;
}
}
@@ -1800,12 +1802,14 @@
static bool CheckMemberPointerConstantExpression(EvalInfo &Info,
SourceLocation Loc,
QualType Type,
- const APValue &Value) {
+ const APValue &Value,
+ Expr::ConstExprUsage Usage) {
const ValueDecl *Member = Value.getMemberPointerDecl();
const auto *FD = dyn_cast_or_null(Member);
if (!FD)
return true;
- return FD->isVirtual() || !FD->hasAttr();
+ return Usage == Expr::EvaluateForMangling || FD->isVirtual() ||
+ !FD->hasAttr();
}
/// Check that this core constant expression is of literal type, and if not,
@@ -1843,8 +1847,10 @@
/// Check that this core constant expression value is a valid value for a
/// constant expression. If not, report an appropriate diagnostic. Does not
/// check that the expression is of literal type.
-static bool CheckConstantExpression(EvalInfo &Info, SourceLocation DiagLoc,
-QualType Type, const APValue &Value) {
+static bool
+CheckConstantExpression(EvalInfo &Info, SourceLocation DiagLoc, QualType Type,
+const APValue &Value,
+Expr::ConstExprUsage Usage = Expr::EvaluateForCodeGen) {
if (Value.isUninit()) {
Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized)
<< true << Type;
@@ -1863,48 +1869,49 @@
QualType EltTy = Type->castAsArrayTypeUnsafe()->getElementType();
for (unsigned I = 0, N = Value.getArrayInitializedElts(); I != N; ++I) {
if (!CheckConstantExpression(Info, DiagLoc, EltTy,
- Value.getArrayInitializedElt(I)))
+ Value.getArrayInitializedElt(I), Usage))
return false;
}
if (!Value.hasArrayFiller())
return true;
-return CheckConstantExpression(Info, DiagLoc, EltTy,
- Value.getArrayFiller());
+return CheckConstantExpression(Info, DiagLoc, EltTy, Value.getArrayFiller(),
+ Usage);
}
if (Value.isUnion() && Value.getUnionField()) {
return CheckConstantExpression(Info, DiagLoc,
Value.getUnionField()->getType(),
- Value.getUnionValue());
+ Value.getUnionValue(), Usage);
}
if (Value.isStruct()) {
RecordDecl *RD = Type->castAs()->getDecl();
if (const CXXRecordDecl *CD = dyn_cast(RD)) {
unsigned BaseIndex = 0;
- for (CXXRecordDecl::base_class_const_iterator I = CD->bases_begin(),
- End = CD->bases_end(); I != End; ++I, ++BaseIndex) {
-if (!CheckConstantExpression(Info, DiagLoc, I->getType(),
- Value.getStructBase(BaseIndex)))
+ for (const CXXBaseSpecifier &BS : CD->bases()) {
+if (!Check
[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode
rnk added a comment. In https://reviews.llvm.org/D46520#1092681, @mstorsjo wrote: > Reverted in SVN r331858. Thanks! When one attempts to generate pre-processed source with clang and compile it with MSVC, you usually have to remove clang's internal intrinsic headers from the include search path, otherwise you'll end up with references to builtins that don't exist in MSVC. Adding an extra -fuse-line-directives flag is just one more thing that you have to remember for that corner-case usage. Repository: rL LLVM https://reviews.llvm.org/D46520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk added a comment. Thanks for the guidance! Comment at: clang/lib/AST/ExprConstant.cpp:1871-1902 if (!CheckConstantExpression(Info, DiagLoc, EltTy, Value.getArrayInitializedElt(I))) return false; } if (!Value.hasArrayFiller()) return true; return CheckConstantExpression(Info, DiagLoc, EltTy, rsmith wrote: > `Usage` should be passed into these recursive calls to > `CheckConstantExpression`, although that's NFC for now, until/unless we start > allowing class types as template parameters. Makes sense, done. Repository: rC Clang https://reviews.llvm.org/D43320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46656: [Builtins] Improve the IR emitted for MSVC compatible rotr/rotl builtins to match what the middle and backends understand
rnk added a comment. Thanks! Repository: rL LLVM https://reviews.llvm.org/D46656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46929: Fix a mangling failure on clang-cl C++17
rnk added a comment. I searched around, and I noticed that `VTableContext::getMethodVTableIndex` has the same implied contract that the caller must always provide a canonical declaration or things will break. It looks like it has three callers, and they all do this. We should probably sink the canonicalization into this helper as well and clean up the now-superfluous canonicalizations at the call site. Repository: rC Clang https://reviews.llvm.org/D46929 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers
rnk added a comment. @hans @thakis, do you remember how these flags are supposed to work? I've forgotten anything I ever knew about them... https://reviews.llvm.org/D46652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46929: Fix a mangling failure on clang-cl C++17
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Shall I go ahead and commit this for you? Comment at: lib/AST/VTableBuilder.cpp:2507 const MethodInfo &MI = I.second; + assert(MD == MD->getCanonicalDecl()); + Thanks for that! Repository: rC Clang https://reviews.llvm.org/D46929 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46929: Fix a mangling failure on clang-cl C++17
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332639: Fix a mangling failure on clang-cl C++17 (authored
by rnk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46929?vs=147244&id=147359#toc
Repository:
rC Clang
https://reviews.llvm.org/D46929
Files:
lib/AST/VTableBuilder.cpp
lib/CodeGen/CGCXX.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -825,7 +825,6 @@
llvm::Constant *ItaniumCXXABI::BuildMemberPointer(const CXXMethodDecl *MD,
CharUnits ThisAdjustment) {
assert(MD->isInstance() && "Member function must not be static!");
- MD = MD->getCanonicalDecl();
CodeGenTypes &Types = CGM.getTypes();
@@ -1640,7 +1639,6 @@
Address This,
llvm::Type *Ty,
SourceLocation Loc) {
- GD = GD.getCanonicalDecl();
Ty = Ty->getPointerTo()->getPointerTo();
auto *MethodDecl = cast(GD.getDecl());
llvm::Value *VTable = CGF.GetVTablePtr(This, Ty, MethodDecl->getParent());
@@ -1674,7 +1672,7 @@
VFunc = VFuncLoad;
}
- CGCallee Callee(MethodDecl, VFunc);
+ CGCallee Callee(MethodDecl->getCanonicalDecl(), VFunc);
return Callee;
}
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -228,7 +228,6 @@
const CXXRecordDecl *
getThisArgumentTypeForMethod(const CXXMethodDecl *MD) override {
-MD = MD->getCanonicalDecl();
if (MD->isVirtual() && !isa(MD)) {
MethodVFTableLocation ML =
CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD);
@@ -1320,23 +1319,21 @@
CharUnits
MicrosoftCXXABI::getVirtualFunctionPrologueThisAdjustment(GlobalDecl GD) {
- GD = GD.getCanonicalDecl();
const CXXMethodDecl *MD = cast(GD.getDecl());
- GlobalDecl LookupGD = GD;
if (const CXXDestructorDecl *DD = dyn_cast(MD)) {
// Complete destructors take a pointer to the complete object as a
// parameter, thus don't need this adjustment.
if (GD.getDtorType() == Dtor_Complete)
return CharUnits();
// There's no Dtor_Base in vftable but it shares the this adjustment with
// the deleting one, so look it up instead.
-LookupGD = GlobalDecl(DD, Dtor_Deleting);
+GD = GlobalDecl(DD, Dtor_Deleting);
}
MethodVFTableLocation ML =
- CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
+ CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
CharUnits Adjustment = ML.VFPtrOffset;
// Normal virtual instance methods need to adjust from the vfptr that first
@@ -1370,7 +1367,6 @@
return CGF.Builder.CreateConstByteGEP(This, Adjustment);
}
- GD = GD.getCanonicalDecl();
const CXXMethodDecl *MD = cast(GD.getDecl());
GlobalDecl LookupGD = GD;
@@ -1839,7 +1835,6 @@
Address This,
llvm::Type *Ty,
SourceLocation Loc) {
- GD = GD.getCanonicalDecl();
CGBuilderTy &Builder = CGF.Builder;
Ty = Ty->getPointerTo()->getPointerTo();
@@ -1878,7 +1873,7 @@
VFunc = Builder.CreateAlignedLoad(VFuncPtr, CGF.getPointerAlign());
}
- CGCallee Callee(MethodDecl, VFunc);
+ CGCallee Callee(MethodDecl->getCanonicalDecl(), VFunc);
return Callee;
}
@@ -2737,7 +2732,6 @@
MicrosoftCXXABI::EmitMemberFunctionPointer(const CXXMethodDecl *MD) {
assert(MD->isInstance() && "Member function must not be static!");
- MD = MD->getCanonicalDecl();
CharUnits NonVirtualBaseAdjustment = CharUnits::Zero();
const CXXRecordDecl *RD = MD->getParent()->getMostRecentDecl();
CodeGenTypes &Types = CGM.getTypes();
Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -267,7 +267,6 @@
const CXXRecordDecl *RD) {
assert(!CGF.CGM.getTarget().getCXXABI().isMicrosoft() &&
"No kext in Microsoft ABI");
- GD = GD.getCanonicalDecl();
CodeGenModule &CGM = CGF.CGM;
llvm::Value *VTable = CGM.getCXXABI().getAddrOfVTable(RD, CharUnits());
Ty = Ty->getPointerTo()->getPointerTo();
@@ -283,7 +282,7 @@
CGF.Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfnkxt");
llvm::Value *VFunc =
CGF.Builder.CreateAlignedLoad(VFuncPtr, CGF.PointerAlignInBytes);
- CGCallee Callee(GD.getDecl(), VFunc);
+ CGCallee Callee(GD.getDecl()->getCanonicalDecl(), VFunc);
return Callee;
}
Index: lib/AST/VTab
[PATCH] D46910: [Support] Avoid normalization in sys::getDefaultTargetTriple
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D46910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47142: [x86] invpcid intrinsic
rnk added a comment. Why do we need a feature flag for this in the first place? The MSVC model for most "instruction" intrinsics is that the exact instruction is emitted regardless of the feature enabled. The target attribute seems like it would get in the way of that. Comment at: lib/Headers/intrin.h:196 + */ void __cdecl _invpcid(unsigned int, void *); +#endif craig.topper wrote: > @rnk, what's the right thing to do here? What problems does this redeclaration cause? Repository: rC Clang https://reviews.llvm.org/D47142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47142: [x86] invpcid intrinsic
rnk added inline comments. Comment at: lib/Headers/intrin.h:196 + */ void __cdecl _invpcid(unsigned int, void *); +#endif GBuella wrote: > GBuella wrote: > > rnk wrote: > > > craig.topper wrote: > > > > @rnk, what's the right thing to do here? > > > What problems does this redeclaration cause? > > Now that I think about it, none. > Also, I think this could be added as TARGET_HEADER_BUILTIN..."intrin.h", > ALL_MS_LANGUAGES into BuiltinsX86.def > And removed from here. > Right? It could be, but then you'd need to implement it as a builtin instead of having invpcidintrin.h, right? Repository: rC Clang https://reviews.llvm.org/D47142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47182: [X86] Move all Intel defined intrinsic includes into immintrin.h
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D47182#1107900, @craig.topper wrote: > Eventually this was determined to not be very scalable to remember which > header file contained what intrinsics and you have to change it with each > generation to get the latest.. So immintrin.h was created to just include > everything. Speaking of things that aren't scalable... immintrin.h is like the new windows.h. :( The reorganization looks good to me, though. Repository: rC Clang https://reviews.llvm.org/D47182 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47174: [X86] Move 128-bit f16c intrinsics to __emmintrin_f16c.h include from emmintrin.h. Move 256-bit f16c intrinsics back to f16cintrin.h
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D47174 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47357: [Driver] Rename DefaultTargetTriple to TargetTriple
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D47357 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types
rnk added a comment.
I think the approach makes sense.
Comment at: lib/CodeGen/CGObjCMac.cpp:7457-7460
CGObjCNonFragileABIMac::GetEHType(QualType T) {
// There's a particular fixed type info for 'id'.
if (T->isObjCIdType() || T->isObjCQualifiedIdType()) {
+if (CGM.getTriple().isWindowsMSVCEnvironment())
@rjmccall how should this be organized in the long run? At this point, the
naming seems totally wrong. Is the non-fragile ABI sort of the canonical way
forward for Obj-C, i.e. it's what a new platform would want to use to best stay
in sync with the future of obj-c?
Repository:
rC Clang
https://reviews.llvm.org/D47233
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.
rnk added a comment.
Should we do this in exactly the places we would lock in an inheritance model
in the MS ABI, i.e. when the member pointer type is required to be complete? I
think we could take this code:
bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
TypeDiagnoser *Diagnoser) {
// FIXME: Add this assertion to make sure we always get instantiation
points.
// assert(!Loc.isInvalid() && "Invalid location in RequireCompleteType");
// FIXME: Add this assertion to help us flush out problems with
// checking for dependent types and type-dependent expressions.
//
// assert(!T->isDependentType() &&
// "Can't ask whether a dependent type is complete");
// We lock in the inheritance model once somebody has asked us to ensure
// that a pointer-to-member type is complete.
if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
if (const MemberPointerType *MPTy = T->getAs()) {
if (!MPTy->getClass()->isDependentType()) {
(void)isCompleteType(Loc, QualType(MPTy->getClass(), 0));
assignInheritanceModel(*this, MPTy->getMostRecentCXXRecordDecl());
}
}
}
And rewrite the side-effecting isCompleteType call to use RequireCompleteType
with the warning diagnostic. I guess we would change the `isMicrosoft` check to
alternatively check if the diagnostic is enabled. It still has the problem that
enabling a warning can cause template instantiation errors, but oh well.
https://reviews.llvm.org/D47503
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38226: [XRay][Driver] Do not link in XRay runtime in shared libs
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good https://reviews.llvm.org/D38226 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38158: [Sema] Null check in BuildDeclarationNameExpr
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rL LLVM https://reviews.llvm.org/D38158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38303: [Sema] Correct IUnknown to support Unknwnbase.h Header.
rnk added inline comments.
Comment at: lib/AST/DeclCXX.cpp:1497-1505
+ bool IsInNamespace = false;
+ const auto *DeclContext = getDeclContext();
+ while (!DeclContext->isTranslationUnit()) {
+if (DeclContext->isNamespace()) {
+ IsInNamespace = true;
+ break;
+}
This would be nicer factored out as a static helper because it would let you
early return and avoid the variable and break. You could just fold the call
into the condition in the `if` below.
https://reviews.llvm.org/D38303
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38303: [Sema] Correct IUnknown to support Unknwnbase.h Header.
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/AST/DeclCXX.cpp:1473
+static bool IsDeclContextInNamespace(const DeclContext *DC) {
+ while (!DC->isTranslationUnit()) {
Lower case `isDeclContext...` is more inline with our style
https://reviews.llvm.org/D38303
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37466: D37461: fixups for existing InlineAsm tests + adding new ones
rnk added inline comments. Comment at: test/CodeGen/ms-inline-asm.cpp:37-38 - int lvar = 10; - __asm mov eax, offset Foo::ptr - __asm mov eax, offset Foo::Bar::ptr -// CHECK-LABEL: define void @_Z2t2v() coby wrote: > rnk wrote: > > These don't seem tested anywhere now > I've tested them against msvc, and they are seem to be unsupported there as > well, so I don't see any value in keeping this one around :\ Got it. Can we make a negative test for offset with namespace separators then? Repository: rL LLVM https://reviews.llvm.org/D37466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38290: Add a ld64.lld alias for the MACHO LLD target
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D38290#885503, @ruiu wrote: > This patch virtually sets `ld64` the linker command name for macOS. I'd be a > bit reluctant doing that, because `ld64` sounds like a too generic name to > indicate "a linker for macOS". But that's probably okay because we don't have > a better name. It's not a good name, but I do believe it is recognizable as the Darwin linker. It's also consistent with the names we invent on other platforms, which are essentially "native linker binary name plus lld". > Can you get an LGTM from someone who owns the clang driver? lgtm Thinking *LONG LONG* term, what I want to see happen is: 1. Annotate the exported C++ API of libLLVM.so to ensure that this does not regress clang compilation time (avoids PLT indirection for calls internal to LLVM) 2. Build with -fvisibility=hidden, so MachO and ELF builds of LLVM work like COFF 3. Make building LLVM.dll/libLLVM.dylib/libLLVM.so the default 4. Stop busy-boxing the LLD ports, and have separate executables for each: ld64.lld, lld-link.exe, and ld.lld This will save disk space and potentially runtime if the loader keeps the LLVM shared object in memory, rather than loading a second copy for every link step in the buidl. Repository: rL LLVM https://reviews.llvm.org/D38290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38123: [driver] [cl] Add/fix c++17/c++latest
rnk accepted this revision. rnk added a comment. Sorry for the delay, thank you, this looks good! @martell, do you mind landing this? https://reviews.llvm.org/D38123 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver
rnk added inline comments.
Comment at: lib/Basic/TargetInfo.cpp:29
+namespace {
+TargetInfo::IntType GetDefaultWCharType(const llvm::Triple &T) {
+ const llvm::Triple::ArchType Arch = T.getArch();
How is this better than what we had before? It's totally inconsistent with our
existing strategy for figuring out type widths and sizes. Our current approach
can be extended for new targets, this requires modifying shared TargetInfo
code. This refactoring *should* be NFC anyway, so if we did want to do it, we'd
want to split it out.
https://reviews.llvm.org/D37891
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver
rnk added inline comments.
Comment at: lib/Basic/TargetInfo.cpp:29
+namespace {
+TargetInfo::IntType GetDefaultWCharType(const llvm::Triple &T) {
+ const llvm::Triple::ArchType Arch = T.getArch();
compnerd wrote:
> rnk wrote:
> > How is this better than what we had before? It's totally inconsistent with
> > our existing strategy for figuring out type widths and sizes. Our current
> > approach can be extended for new targets, this requires modifying shared
> > TargetInfo code. This refactoring *should* be NFC anyway, so if we did want
> > to do it, we'd want to split it out.
> The previous thing was split across and was fairly difficult to identify what
> was going on. I tend to think that this makes it easier to identify what is
> going on. However, if you have a strong opinion on this, Im willing to
> switch it back (though, possibly retain some of the adjustments to make it
> easier to follow).
I do, I want to see the minimal functional change. It'll make it easier to spot
awkward places where we duplicate logic.
https://reviews.llvm.org/D37891
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017
rnk created this revision.
This raises our default past 1900, which controls whether char16_t is a
builtin type or not.
Implements PR34243
https://reviews.llvm.org/D38646
Files:
clang/lib/Driver/ToolChains/MSVC.cpp
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1266,9 +1266,8 @@
if (MSVT.empty() &&
Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=18.00 is default.
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-MSVT = VersionTuple(18);
+// -fms-compatibility-version=19.10 is default, aka 2017
+MSVT = VersionTuple(19, 10);
}
return MSVT;
}
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1266,9 +1266,8 @@
if (MSVT.empty() &&
Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=18.00 is default.
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-MSVT = VersionTuple(18);
+// -fms-compatibility-version=19.10 is default, aka 2017
+MSVT = VersionTuple(19, 10);
}
return MSVT;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017
rnk updated this revision to Diff 118079.
rnk added a comment.
- go to 1911, 1910 was the preview
- add release notes
https://reviews.llvm.org/D38646
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Driver/ToolChains/MSVC.cpp
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1266,9 +1266,8 @@
if (MSVT.empty() &&
Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=18.00 is default.
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-MSVT = VersionTuple(18);
+// -fms-compatibility-version=19.11 is default, aka 2017
+MSVT = VersionTuple(19, 11);
}
return MSVT;
}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -89,6 +89,11 @@
- Bitrig OS was merged back into OpenBSD, so Bitrig support has been
removed from Clang/LLVM.
+- The default value of _MSC_VER was raised from 1800 to 1911, making it
+ compatible with the Visual Studio 2015 and 2017 C++ standard library headers.
+ Users should generally expect this to be regularly raised to match the most
+ recently released version of the Visual C++ compiler.
+
New Compiler Flags
--
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1266,9 +1266,8 @@
if (MSVT.empty() &&
Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=18.00 is default.
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-MSVT = VersionTuple(18);
+// -fms-compatibility-version=19.11 is default, aka 2017
+MSVT = VersionTuple(19, 11);
}
return MSVT;
}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -89,6 +89,11 @@
- Bitrig OS was merged back into OpenBSD, so Bitrig support has been
removed from Clang/LLVM.
+- The default value of _MSC_VER was raised from 1800 to 1911, making it
+ compatible with the Visual Studio 2015 and 2017 C++ standard library headers.
+ Users should generally expect this to be regularly raised to match the most
+ recently released version of the Visual C++ compiler.
+
New Compiler Flags
--
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315107: [MS] Raise the default value of _MSC_VER to 1911,
which is VS 2017 (authored by rnk).
Changed prior to commit:
https://reviews.llvm.org/D38646?vs=118079&id=118080#toc
Repository:
rL LLVM
https://reviews.llvm.org/D38646
Files:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
Index: cfe/trunk/docs/ReleaseNotes.rst
===
--- cfe/trunk/docs/ReleaseNotes.rst
+++ cfe/trunk/docs/ReleaseNotes.rst
@@ -89,6 +89,11 @@
- Bitrig OS was merged back into OpenBSD, so Bitrig support has been
removed from Clang/LLVM.
+- The default value of _MSC_VER was raised from 1800 to 1911, making it
+ compatible with the Visual Studio 2015 and 2017 C++ standard library headers.
+ Users should generally expect this to be regularly raised to match the most
+ recently released version of the Visual C++ compiler.
+
New Compiler Flags
--
Index: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
@@ -1266,9 +1266,8 @@
if (MSVT.empty() &&
Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=18.00 is default.
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-MSVT = VersionTuple(18);
+// -fms-compatibility-version=19.11 is default, aka 2017
+MSVT = VersionTuple(19, 11);
}
return MSVT;
}
Index: cfe/trunk/docs/ReleaseNotes.rst
===
--- cfe/trunk/docs/ReleaseNotes.rst
+++ cfe/trunk/docs/ReleaseNotes.rst
@@ -89,6 +89,11 @@
- Bitrig OS was merged back into OpenBSD, so Bitrig support has been
removed from Clang/LLVM.
+- The default value of _MSC_VER was raised from 1800 to 1911, making it
+ compatible with the Visual Studio 2015 and 2017 C++ standard library headers.
+ Users should generally expect this to be regularly raised to match the most
+ recently released version of the Visual C++ compiler.
+
New Compiler Flags
--
Index: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
@@ -1266,9 +1266,8 @@
if (MSVT.empty() &&
Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=18.00 is default.
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-MSVT = VersionTuple(18);
+// -fms-compatibility-version=19.11 is default, aka 2017
+MSVT = VersionTuple(19, 11);
}
return MSVT;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Looks good with nits
Comment at: lib/Basic/Targets/AArch64.cpp:47-51
+ bool IsNetBSD = getTriple().getOS() == llvm::Triple::NetBSD;
+ bool IsOpenBSD = getTriple().getOS() == llvm::Triple::OpenBSD;
+ if (!getTriple().isOSDarwin() && !IsNetBSD && !IsOpenBSD)
+WCharType = UnsignedInt;
+
I felt like this was clearer the way it was before, and we're already checking
for the BSDs above.
Comment at: lib/Basic/Targets/AArch64.cpp:160-161
- Builder.defineMacro("__ARM_SIZEOF_WCHAR_T", Opts.ShortWChar ? "2" : "4");
+ Builder.defineMacro("__ARM_SIZEOF_WCHAR_T",
+ llvm::utostr(Opts.WCharSize ? Opts.WCharSize : 4));
This is correct because we compute macros after we apply the flag override,
right?
Repository:
rL LLVM
https://reviews.llvm.org/D37891
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows
rnk added inline comments.
Comment at: src/AddressSpace.hpp:388-389
+ found_obj = true;
+ } else if (!strncmp((const char *)pish->Name, ".eh_frame",
+ IMAGE_SIZEOF_SHORT_NAME)) {
+info.dwarf_section = begin;
".eh_frame" is 9 characters, right? I thought mingw linkers took sections with
long names and moved them to an extended symbol table. Does that not apply to
.eh_frame?
https://reviews.llvm.org/D38679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017
rnk added a comment. In https://reviews.llvm.org/D38646#892246, @STL_MSFT wrote: > FYI: 1910 was the value for VS 2017 RTM. 1911 is the value for VS 2017 15.3, > the first toolset update. 1912 will be the value for VS 2017 15.5, the second > toolset update. Yep. The initial draft of this patch had the wrong commit message, but everything is fixed in the committed version. I had 1910 installed on my machine locally, and spent this morning untangling that. Apparently now you have to install updates through the VS "Tools -> Extesions & Updates -> mumble" menu. Downloading and running the VS update 3 installer isn't enough. Repository: rL LLVM https://reviews.llvm.org/D38646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows
rnk added inline comments.
Comment at: src/AddressSpace.hpp:388-389
+ found_obj = true;
+ } else if (!strncmp((const char *)pish->Name, ".eh_frame",
+ IMAGE_SIZEOF_SHORT_NAME)) {
+info.dwarf_section = begin;
mstorsjo wrote:
> rnk wrote:
> > ".eh_frame" is 9 characters, right? I thought mingw linkers took sections
> > with long names and moved them to an extended symbol table. Does that not
> > apply to .eh_frame?
> Yes, they do, so this actually only matches `.eh_fram`. I didn't yet look at
> how to navigate the IMAGE_*_HEADERS structs to find the coresponding full
> long name.
Can you add a FIXME here? No need to read the long-form symbol table yet, I
just want to document that we will need that for compatibility with ld.bfd.
https://reviews.llvm.org/D38679
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size
rnk added inline comments. Comment at: src/libunwind.cpp:188 + co->getInfo(&info); + pint_t orgArgSize = (pint_t)info.gp; + uint64_t orgFuncStart = info.start_ip; I think it makes sense to have this here: the contract is that if the personality sets the IP when the cursor pointed to a PC with a non-zero arg size, we should adjust SP for the personality. However, it's not clear to me that we don't need the same adjustment when stepping across frames that use arg size without a frame pointer. https://reviews.llvm.org/D38680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size
rnk accepted this revision. rnk added a comment. lgtm Comment at: src/libunwind.cpp:188 + co->getInfo(&info); + pint_t orgArgSize = (pint_t)info.gp; + uint64_t orgFuncStart = info.start_ip; mstorsjo wrote: > rnk wrote: > > I think it makes sense to have this here: the contract is that if the > > personality sets the IP when the cursor pointed to a PC with a non-zero arg > > size, we should adjust SP for the personality. > > > > However, it's not clear to me that we don't need the same adjustment when > > stepping across frames that use arg size without a frame pointer. > When stepping across frames, the gnu args size is already included in, as > part of the CFA offset, so with the current code, it steps too far. OK, yes, now I see the code that does that. :) https://reviews.llvm.org/D38680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38700: [Sema][Crash] Correctly handle an non-dependent noexcept expr in function template
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good, thanks! https://reviews.llvm.org/D38700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls
rnk added a comment.
In https://reviews.llvm.org/D39079#904050, @lebedev.ri wrote:
> No tests?
+1, there should be an -emit-llvm test in clang/test/CodeGen/.
Comment at: lib/CodeGen/CGCall.cpp:1859
+if (auto *Fn = dyn_cast(TargetDecl)) {
+ if (!Fn->isDefined() && !AttrOnCallSite) {
+FuncAttrs.addAttribute(llvm::Attribute::NonLazyBind);
Remind me what happens when the definition is within the current DSO after
linking. I seem to recall that the call through memory is 6 bytes and the
direct pcrel call is 5 bytes, and the linker is required to know to rewrite the
indirect call to a nop. Is that accurate?
https://reviews.llvm.org/D39079
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38819: [libunwind] Add support for dwarf unwinding on windows on x86_64
rnk added inline comments. Comment at: src/UnwindRegistersRestore.S:98 + # skip fs + # skip gs + movq 56(%rcx), %rsp # cut back rsp to new location mstorsjo wrote: > mstorsjo wrote: > > compnerd wrote: > > > Doesn't Win64 ABI require some of the MMX registers be saved/restored too? > > Right, yes, xmm6-xmm15 should be backed up and restored. I'll try to amend > > this with such a change. > Actually, such a change doesn't necessarily make much sense on its own. > > As long as the dwarf encoding itself doesn't describe how to restore those > registers (and on unix platforms you don't need to, so it probably isn't even > specified), you'd just end up backing up the xmm registers on entry when > throwing the exception, and restoring the exactly same ones again - it only > guards against changes within libcxxabi/libunwind and the unwinding machinery > itself, not against changes further down in the call stack between the > thrower and catcher of the exception. > > So with that, I guess this patch is futile unless planning to extend the > x86_64 dwarf handling in llvm to include those registers as well - and that's > a little out of scope of what I intended to do here... If we have XMM values in the register context, we might as well reload them here. I assume libunwind will switch away from DWARF and over to UNWIND_INFO opcodes in the near future, and that will give us an accurate register context. https://reviews.llvm.org/D38819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D39079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39206: [libunwind] Add missing checks for register number
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D39206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39210: Add default calling convention support for regcall.
rnk added inline comments.
Comment at: lib/Sema/SemaType.cpp:3269-3273
+ bool IsMain = false;
+ if (D.getIdentifier() && D.getIdentifier()->isStr("main") &&
+ S.CurContext->getRedeclContext()->isTranslationUnit() &&
+ !S.getLangOpts().Freestanding)
+IsMain = true;
I highly doubt this is correct. I have a feeling there are all kinds of ways
you can get this to fire on things that aren't a function declaration. It's
also inefficient to check the identifier string every time we make a function
type. Please find somewhere else to add this. I'd suggest adjusting the
function type in CheckMain, or some time before then, whereever we make main
implicitly extern "C".
https://reviews.llvm.org/D39210
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39210: Add default calling convention support for regcall.
rnk added inline comments.
Comment at: lib/Sema/SemaType.cpp:3269-3273
+ bool IsMain = false;
+ if (D.getIdentifier() && D.getIdentifier()->isStr("main") &&
+ S.CurContext->getRedeclContext()->isTranslationUnit() &&
+ !S.getLangOpts().Freestanding)
+IsMain = true;
erichkeane wrote:
> rnk wrote:
> > I highly doubt this is correct. I have a feeling there are all kinds of
> > ways you can get this to fire on things that aren't a function declaration.
> > It's also inefficient to check the identifier string every time we make a
> > function type. Please find somewhere else to add this. I'd suggest
> > adjusting the function type in CheckMain, or some time before then,
> > whereever we make main implicitly extern "C".
> I believe the logic here was pulled from FunctionDecl's "isMain" function:
> https://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html#aa2b31caf653741632b16cce1ae2061cc
>
> As this is so early in the process (at Declarator), I didn't see a good place
> to recommend extracting this, besides pulling it into the Declarator.
>
> CheckMain is also run at the Decl stage, isn't it? Is your suggestion be to
> simply let this go through with the wrong calling-convention here, then
> revert it it in "CheckMain"?
Yep. You can look at how we use FunctionTypeUnwrapper and
ASTContext::adjustFunctionType in various places to fix up function types that
were built with the wrong calling convention without losing type source info.
https://reviews.llvm.org/D39210
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39064: implement __has_unique_object_representations
rnk added inline comments.
Comment at: lib/AST/Type.cpp:2226
+Context.getFieldOffset(*Record->field_begin()));
+for (const auto *Field : Record->fields()) {
+ if (!Field->getType().hasUniqueObjectRepresentations(Context))
What about base classes? I think that's where the padding detection is going to
get wacky. =/
https://reviews.llvm.org/D39064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39127: Fix template parameter default args missed if redecled
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D39127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39127: Fix template parameter default args missed if redecled
rnk added a comment. I think you forgot to svn add the test case Repository: rL LLVM https://reviews.llvm.org/D39127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls
rnk added a comment. In https://reviews.llvm.org/D39079#905372, @joerg wrote: > Why again is this a good idea? It saves the direct jump to the PLT, reducing icache pressure, which is a major cost in some workloads. > This is an even worse hack than -Bsymbolic, Personally, I would like to build LLVM with -Bsymbolic so that we can build LLVM as a DSO and load it from clang without regressing startup time, so I don't see what's so terrible about -Bsymbolic, especially for C++ programs. > the latter at least is visible in ELF header without code inspection. This is > breaking core premises of ELF. What are you talking about? Anyway, LLVM already has an attribute, nonlazybind, and this just provides a flag to apply it to all declarations. It gives the user access to the GOTPCREL relocations that we, and loaders, already support. https://reviews.llvm.org/D39079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls
rnk added a comment. In https://reviews.llvm.org/D39079#905395, @joerg wrote: > Let me phrase it differently. What is this patch (and the matching backend > PR) supposed to achieve? There are effectively two ways to get rid of PLT > entries: > (1) Bind references locally. This is effectively what -Bsymbolic does and > what is breaking the ELF interposition rules. > (2) Do an indirect call via the GOT. Requires knowing what an external > symbol is, making it non-attractive for anything but LTO, since it will > create performance issues for all non-local accesses (i.e. anything private). This patch does 2. According to @tmsriram, clever linkers can turn the indirect call back into a nop+call_pcrel32. If this isn't universal, the user must know what their linker supports. I don't see how it causes performance issues for non-local calls, since the PLT will do a jump through the GOT anyway. https://reviews.llvm.org/D39079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39224: Moved QualTypeNames.h from Tooling to AST.
rnk added a comment. Can you remind me why `NamedDecl::printQualifiedName` is not appropriate for your needs? https://reviews.llvm.org/D39224 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls
rnk added a comment. In https://reviews.llvm.org/D39079#905454, @joerg wrote: > It also increases the pressure on the branch predictor, so it is not really > black and white. I don't understand this objection. I'm assuming that the PLT stub is an indirect jump through the PLTGOT, not a hotpatched stub that jumps directly to the definition chosen by the loader. This is the ELF model that I'm familiar with, especially since calls to code more than 2GB away generally need to be indirect anyway. > Qt5 tries that. Requires further hacks as the main binary must be compiled as > fully position independent code to not run into fun latter. Fun with copy > relocations is only part of it. I'm not sure I understand, but this patch isn't introducing copy relocations, to be clear. > The loader doesn't see GOTPCREL anymore. It also requires a linker that > disassembles instructions, because it can't distinguish between a normal > pointer load and a call, to be able to optimize it. Well, yes. The user needs to know that they have an x86-encoding-aware linker, or using this flag is probably going to slow their code down. From my perspective, this is a performance tuning flag, so that's reasonable. https://reviews.llvm.org/D39079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39064: implement __has_unique_object_representations
rnk added inline comments.
Comment at: lib/AST/Type.cpp:2212-2213
+for (const auto Base : ClassDecl->bases()) {
+ if (Base.isVirtual())
+return false;
+
OK, I guess vbases don't have a unique representation. :) What about vtable
slots? Should we check isDynamic?
https://reviews.llvm.org/D39064
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39064: implement __has_unique_object_representations
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! https://reviews.llvm.org/D39064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Excellent! Repository: rL LLVM https://reviews.llvm.org/D51049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. It makes sense to me to do this all in one go and defer the clang `__attribute__` until later. Comment at: llvm/include/llvm/IR/Attributes.td:249 def : MergeRule<"adjustNullPointerValidAttr">; + I can't tell if this is just vim fixing the lack of a proper trailing newline, but revert any whitespace change if possible. Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:1169 + case Attribute::SpeculativeLoadHardening: +return 1ULL << 60; case Attribute::Dereferenceable: These appear to repeat LLVMBitcodes.h, unless I am mistaken. Weird. Anyway, fixing that is out of scope. Repository: rL LLVM https://reviews.llvm.org/D51157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
