[PATCH] D24812: Lit C++11 Compatibility Patch #11

[Paul Robinson via Phabricator via cfe-commits]

probinson added a reviewer: rjmccall.
probinson added a comment.

+rjmccall as CodeGen owner.


https://reviews.llvm.org/D24812



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

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

These are all Objective-C++ tests, and AFAIK we don't intend to change the 
default Objective-C++ dialect when we finally do change the default C++ dialect.
So I think these tests do not need to be modified.


https://reviews.llvm.org/D29739



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

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D29739#673971, @rjmccall wrote:

> In https://reviews.llvm.org/D29739#673933, @tigerleapgorge wrote:
>
> > Hi John,
> >
> > Here is the most recent discussion I can find on cfe-dev.
> >  “I'm guessing that Objective-C/C++ is kind of passe, so nobody is really 
> > interested in modernizing it”
> >  http://lists.llvm.org/pipermail/cfe-dev/2016-December/051844.html
> >
> > As far as I am aware, there appears to be no strong reason to bump or not 
> > to bump ObjC++.
>
>
> It certainly simplifies the message to say that we've changed the default C++ 
> dialect to C++11 across the board.  That should apply to ObjC++ as well.  I 
> would not describe ObjC++ as passé; it's a very important language for Apple 
> developers.


Nice to know, although nobody piped up on the earlier cited discussion.

Sony is invested in making the lit tests C++11 clean so that we can upstream a 
change to make it the default C++ dialect for PS4.  That will ensure that any 
new C++ tests are C++11 clean.  This is one step in the direction of making 
C++11 (or even something newer) the default dialect for everybody.
However we are not an Objective-C++ vendor.  We are neutral about changing the 
default dialect there; if you want to change the default dialect for 
Objective-C++, that's fine with us, but I don't think we can invest in learning 
enough about Objective-C++ to do the right thing with the existing 
Objective-C++ tests.  In particular I don't know whether forcing 98 on these 
tests is the "right" solution; all we know is that it made the tests pass, 
which is not particularly surprising.  I really think Apple would need to step 
up here if the default Objective-C++ dialect is going to change.

> It is likely that the Rewriter generates C++98-only code.  I believe the 
> Rewriter is no longer being actively maintained; I'm not sure we're ready to 
> propose removing it yet, but if there are specific problems with those tests, 
> I think it makes some sense to just pass -std=gnu++98 for them.
> 
> John.

I am pretty sure we already modified the Rewriter tests to explicitly compile 
the rewritten C++ as -98 code.  It's just the straight Objective-C++ tests 
where Charles has stepped back.

--paulr


https://reviews.llvm.org/D29739



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

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D29739#674297, @rjmccall wrote:

> In https://reviews.llvm.org/D29739#674288, @probinson wrote:
>
> > I really think Apple would need to step up here if the default 
> > Objective-C++ dialect is going to change.
>
>
> I don't mind stepping up and doing this work.  I just thought you'd already 
> done it.  This patch updates some tests; is that enough, or are there further 
> changes required in order to change the default ObjC++ dialect?
>
> John.


Charles tells me this is the last group of Objective-C++ tests that failed, so 
maybe there isn't anything else to do after all.  I had thought there were 
more.  He'll go ahead with this set.
We have another couple dozen C++ tests to straighten out, then we'll be ready 
to have the dev-list discussion about upgrading the default for 5.0, and 
whether it's the narrow case (just C++ and just for PS4, which was our original 
plan) or the big bang (both languages and for everybody).


https://reviews.llvm.org/D29739



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#675687, @chandlerc wrote:

> In https://reviews.llvm.org/D28404#675616, @mehdi_amini wrote:
>
> > We're still waiting for @rsmith to comment whether it'd be better to `have 
> > a LangOpts flag that basically means "pragma clang optimize off is always 
> > in effect."` and `Have Sema pretend the pragma is in effect at all times, 
> > at -O0`.
>
>
> FWIW, I have no real opinion about this side of it, I see it more as a detail 
> of how Clang wants to implement this kind of thing.


That was my suggestion as it seemed like this patch is essentially replicating 
the attribute-conflict detection logic that's in place for attributes specified 
in the source.  And we do like to say DRY.
But I won't insist; the patch can proceed as far as I'm concerned.


https://reviews.llvm.org/D28404



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

[PATCH] D28620: Guard __gnuc_va_list typedef

[Paul Robinson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL292819: Guard __gnuc_va_list typedef. (authored by 
probinson).

Changed prior to commit:
  https://reviews.llvm.org/D28620?vs=84151=85430#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28620

Files:
  cfe/trunk/lib/Headers/stdarg.h
  cfe/trunk/test/Headers/stdarg-gnuc_va_list.c


Index: cfe/trunk/test/Headers/stdarg-gnuc_va_list.c
===
--- cfe/trunk/test/Headers/stdarg-gnuc_va_list.c
+++ cfe/trunk/test/Headers/stdarg-gnuc_va_list.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wsystem-headers -std=c99 %s
+// expected-no-diagnostics
+
+// Check that no warnings are emitted from stdarg.h if __gnuc_va_list has
+// previously been defined in another header file.
+typedef __builtin_va_list __va_list;
+typedef __va_list __gnuc_va_list;
+#define __GNUC_VA_LIST
+
+#include 
Index: cfe/trunk/lib/Headers/stdarg.h
===
--- cfe/trunk/lib/Headers/stdarg.h
+++ cfe/trunk/lib/Headers/stdarg.h
@@ -43,10 +43,9 @@
 #define va_copy(dest, src)  __builtin_va_copy(dest, src)
 #endif
 
-/* Hack required to make standard headers work, at least on Ubuntu */
 #ifndef __GNUC_VA_LIST
 #define __GNUC_VA_LIST 1
-#endif
 typedef __builtin_va_list __gnuc_va_list;
+#endif
 
 #endif /* __STDARG_H */


Index: cfe/trunk/test/Headers/stdarg-gnuc_va_list.c
===
--- cfe/trunk/test/Headers/stdarg-gnuc_va_list.c
+++ cfe/trunk/test/Headers/stdarg-gnuc_va_list.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wsystem-headers -std=c99 %s
+// expected-no-diagnostics
+
+// Check that no warnings are emitted from stdarg.h if __gnuc_va_list has
+// previously been defined in another header file.
+typedef __builtin_va_list __va_list;
+typedef __va_list __gnuc_va_list;
+#define __GNUC_VA_LIST
+
+#include 
Index: cfe/trunk/lib/Headers/stdarg.h
===
--- cfe/trunk/lib/Headers/stdarg.h
+++ cfe/trunk/lib/Headers/stdarg.h
@@ -43,10 +43,9 @@
 #define va_copy(dest, src)  __builtin_va_copy(dest, src)
 #endif
 
-/* Hack required to make standard headers work, at least on Ubuntu */
 #ifndef __GNUC_VA_LIST
 #define __GNUC_VA_LIST 1
-#endif
 typedef __builtin_va_list __gnuc_va_list;
+#endif
 
 #endif /* __STDARG_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27597: [DebugInfo] Restore test case for long double constants.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

As dblaikie said in email, probably better to make this X86-specific; if 
long-double varies by OS you can put in a specific triple.

FTR we don't rely on Perl being available everywhere, anything that does this 
kind of scripty stuff uses Python.




Comment at: test/CodeGen/debug-info-static-const-fp.c:1
-// RUN: %clang -emit-llvm -O0 -S -g %s -o - | FileCheck %s
+// RUN: %clang -emit-llvm -O0 -S -g %s -o %t
+// RUN: perl <%t >%t-check-prefix -n -e  \

Sorry didn't notice this before, %clang is for driver tests only; use 
%clang_cc1 here.


https://reviews.llvm.org/D27597



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

[PATCH] D27641: DebugInfo: Added support for Checksum debug info feature (Clang part)

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:350
+  std::string Checksum;
+  SM.getChecksumMD5(SM.getFileID(Loc), Checksum);
+

rnk wrote:
> We should only do this if `CGM.getCodeGenOpts().EmitCodeView`, or we will 
> regress compile time on other platforms without any added functionality.
Eventually we'll want it for DWARF 5 also, but for now conditioning on CodeView 
is fine.


https://reviews.llvm.org/D27641



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

[PATCH] D27597: [DebugInfo] Restore test case for long double constants.

[Paul Robinson via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D27597#621618, @dgross wrote:

> So would a Python equivalent of the Perl be acceptable?  I think this is an 
> academic question -- better to explicitly test multiple triples.


There are a small number of tests that run Python scripts, but they are 
generally doing something really unusual (I admit I haven't gone to look, but 
that's my recollection).  If you can get proper coverage without Python, and it 
doesn't require standing on your head, people generally prefer that.  In this 
case, multiple RUNs with different triples will do the job, so that's 
preferable.

LGTM.


https://reviews.llvm.org/D27597



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

[PATCH] D27794: Make some diagnostic tests C++11 clean

[Paul Robinson via Phabricator via cfe-commits]

probinson added a reviewer: ABataev.
probinson added a comment.

+abataev for OpenMP.


https://reviews.llvm.org/D27794



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

[PATCH] D27794: Make some diagnostic tests C++11 clean

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added a reviewer: rsmith.
probinson added subscribers: cfe-commits, tigerleapgorge.

Another half-dozen test revisions in the ongoing campaign to make things ready 
for C++11 as Clangs's default dialect.

Most of these are straightforward, but I am not entirely sure about a couple of 
things:

- In fixit.cpp, the place that now gets 'expected unqualified-id' seems funny, 
but maybe that's just the nature of things
- In copy-assignment.cpp, I am bemused by the whole thing but especially 
'passing argument to parameter here'


https://reviews.llvm.org/D27794

Files:
  test/FixIt/fixit.cpp
  test/OpenMP/teams_distribute_collapse_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_collapse_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_collapse_messages.cpp
  test/Parser/backtrack-off-by-one.cpp
  test/SemaCXX/copy-assignment.cpp

Index: test/SemaCXX/copy-assignment.cpp
===
--- test/SemaCXX/copy-assignment.cpp
+++ test/SemaCXX/copy-assignment.cpp
@@ -1,12 +1,22 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+
+#if __cplusplus >= 201103L
+// expected-note@+3 2 {{candidate constructor}}
+// expected-note@+2 {{passing argument to parameter here}}
+#endif
 struct A {
 };
 
 struct ConvertibleToA {
   operator A();
 };
 
 struct ConvertibleToConstA {
+#if __cplusplus >= 201103L
+// expected-note@+2 {{candidate function}}
+#endif
   operator const A();
 };
 
@@ -69,6 +79,9 @@
   na = a;
   na = constA;
   na = convertibleToA;
+#if __cplusplus >= 201103L
+// expected-error@+2 {{no viable conversion}}
+#endif
   na = convertibleToConstA;
   na += a; // expected-error{{no viable overloaded '+='}}
 
Index: test/Parser/backtrack-off-by-one.cpp
===
--- test/Parser/backtrack-off-by-one.cpp
+++ test/Parser/backtrack-off-by-one.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify %s -std=c++98
+// RUN: %clang_cc1 -verify %s -std=c++11
 
 // PR25946
 // We had an off-by-one error in an assertion when annotating A below.  Our
@@ -10,8 +12,10 @@
 
 // expected-error@+1 {{expected '{' after base class list}}
 template  class B : T // not ',' or '{'
-// expected-error@+3 {{C++ requires a type specifier for all declarations}}
-// expected-error@+2 {{expected ';' after top level declarator}}
+#if __cplusplus < 201103L
+// expected-error@+4 {{expected ';' after top level declarator}}
+#endif
+// expected-error@+2 {{C++ requires a type specifier for all declarations}}
 // expected-error@+1 {{expected ';' after class}}
 A {
 };
Index: test/OpenMP/teams_distribute_parallel_for_simd_collapse_messages.cpp
===
--- test/OpenMP/teams_distribute_parallel_for_simd_collapse_messages.cpp
+++ test/OpenMP/teams_distribute_parallel_for_simd_collapse_messages.cpp
@@ -1,8 +1,13 @@
 // RUN: %clang_cc1 -verify -fopenmp %s
+// RUN: %clang_cc1 -verify -fopenmp %s -std=c++98
+// RUN: %clang_cc1 -verify -fopenmp %s -std=c++11
 
 void foo() {
 }
 
+#if __cplusplus >= 201103L
+// expected-note@+2 4 {{declared here}}
+#endif
 bool foobool(int argc) {
   return argc;
 }
@@ -50,6 +55,9 @@
   for (int i = ST; i < N; i++)
 argv[0][i] = argv[0][i] - argv[0][i-ST]; // expected-error 2 {{expected 2 for loops after '#pragma omp teams distribute parallel for simd', but found only 1}}
 
+#if __cplusplus >= 201103L
+// expected-note@+6 2 {{non-constexpr function 'foobool' cannot be used}}
+#endif
 // expected-error@+4 2 {{directive '#pragma omp teams distribute parallel for simd' cannot contain more than one 'collapse' clause}}
 // expected-error@+3 2 {{argument to 'collapse' clause must be a strictly positive integer value}}
 // expected-error@+2 2 {{expression is not an integral constant expression}}
@@ -62,7 +70,11 @@
   for (int i = ST; i < N; i++)
 argv[0][i] = argv[0][i] - argv[0][i-ST];
 
-// expected-error@+2 2 {{expression is not an integral constant expression}}
+#if __cplusplus >= 201103L
+// expected-error@+5 2 {{integral constant expression must have integral or unscoped enumeration type}}
+#else
+// expected-error@+3 2 {{expression is not an integral constant expression}}
+#endif
 #pragma omp target
 #pragma omp teams distribute parallel for simd collapse (argv[1]=2) // expected-error {{expected ')'}} expected-note {{to match this '('}}
   for (int i = ST; i < N; i++)
@@ -110,11 +122,17 @@
   for (int i = 4; i < 12; i++)
 argv[0][i] = argv[0][i] - argv[0][i-4]; // expected-error {{expected 4 for loops after '#pragma omp teams distribute parallel for simd', but found only 1}}
 
+#if __cplusplus >= 201103L
+// expected-note@+3 {{non-constexpr function 'foobool' cannot be used}}
+#endif
 #pragma omp target
 

[PATCH] D27549: [DebugInfo] Add support for __fp16, float, and double constants.

[Paul Robinson via Phabricator via cfe-commits]

probinson added subscribers: cfe-commits, probinson.
probinson added a comment.

Hi David!
As this is a Clang patch, you should subscribe cfe-commits rather than 
llvm-commits.  I've done that for you.
See also inline comments.




Comment at: lib/CodeGen/CGDebugInfo.cpp:3765
+InitExpr =
+
DBuilder.createConstantValueExpression(Init.getFloat().bitcastToAPInt().getZExtValue());
   GV.reset(DBuilder.createGlobalVariable(

This line exceeds 80 columns.  clang-format is your friend.



Comment at: test/CodeGen/debug-info-static-const-fp.c:19
+// CHECK: !4 = distinct !DIGlobalVariable(name: "hVal", scope: !0, file: !1, 
line: 6, type: !5, isLocal: true, isDefinition: true, expr: !7)
+// CHECK: !7 = !DIExpression(DW_OP_constu, 16502, DW_OP_stack_value)
+// CHECK: !8 = distinct !DIGlobalVariable(name: "fVal", scope: !0, file: !1, 
line: 8, type: !9, isLocal: true, isDefinition: true, expr: !11)

Checking for the entire exact line for the variable is a bit fragile.  Really 
what you want to do is associate the variable with the correct expression, and 
ignore all the irrelevant details.  Something like this:
```
// CHECK: DIGlobalVariable(name: "hVal", {{.*}} expr: ![[HEXPR:{{[0-9]+}}]]
// CHECK: ![[HEXPR] = ! DIExpression(DW_OP_constu, 16502, DW_OP_stack_value)
```
And similar pairs for the other variables.  I'd use a different FileCheck 
variable for each case.



https://reviews.llvm.org/D27549



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

[PATCH] D27549: [DebugInfo] Add support for __fp16, float, and double constants.

[Paul Robinson via Phabricator via cfe-commits]

probinson accepted this revision.
probinson added a reviewer: probinson.
probinson added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: lib/CodeGen/CGDebugInfo.cpp:3765
+InitExpr =
+
DBuilder.createConstantValueExpression(Init.getFloat().bitcastToAPInt().getZExtValue());
   GV.reset(DBuilder.createGlobalVariable(

dgross wrote:
> probinson wrote:
> > This line exceeds 80 columns.  clang-format is your friend.
> Done.  Is clang-format mentioned in any of the documentation that describes 
> the development process?  I might have missed it (not that that's an excuse 
> for a needless violation of the coding guidelines).
I would have thought the coding guidelines would mention clang-format, but I'm 
not finding it there (or in the stuff about submitting a patch).


https://reviews.llvm.org/D27549



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I guess I'm getting irritated because people are trying to tell me what optnone 
means. I know what it means; I spent probably a whole year pushing to get it 
adopted.

Optnone means:  When you are running optimizations, try not to optimize this 
part, if you can.

That's it.  That's *all*.  It has never meant anything else.  Telling me 
different means you misunderstand, and trying to persuade me that *I* 
misunderstand is going to be a waste of time and effort.

I fully understand that this is not the definition of optnone that you *want*.  
Please feel free to propose a redefinition.  But don't go telling me that the 
thing you *want* is what the thing already *is* and that any difference is a 
bug.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Basically, I don't see why having clang always emit a real .o at -O0 would be a 
problem.
I haven't gotten through the other-CFI documentation yet though.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640182, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote:
>
> > Also, that's not practicable: what if I have an LTO static library for 
> > which I don't have the source, now if I build my own file with -O0 -flto I 
> > can't link anymore.
>
>
> Also: LTO is required for some features likes CFI. There are users who wants 
> CFI+O0 during development (possibly for debugging a subcomponent of the app).


Sorry, you lost me.  CFI is part of DWARF and we do DWARF perfectly well 
without LTO (and at O0).


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote:

> I don't follow: IMO if I generate a module with optnone and pipe it to `opt 
> -O3` I expect no function IR to be touched. If it is not the case it is a bug.


Your opinion and expectation are not supported by the IR spec.  Optnone skips 
"most" optimization passes.  It is not practical (or was not, at the time) to 
make the -O3 pipeline behave exactly the same as the -O0 pipeline, and also not 
actually necessary to support the purpose for which 'optnone' was invented.

If you have a goal of making 'optnone' functions use the actual -O0 pipeline, 
while non-optnone functions use the optimizing pipeline, more power to you and 
you will need to take up that particular design challenge with Chandler first.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640362, @probinson wrote:

> In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote:
>
> > I don't follow: IMO if I generate a module with optnone and pipe it to `opt 
> > -O3` I expect no function IR to be touched. If it is not the case it is a 
> > bug.
>
>
> Your opinion and expectation are not supported by the IR spec.  Optnone skips 
> "most" optimization passes.  It is not practical (or was not, at the time) to 
> make the -O3 pipeline behave exactly the same as the -O0 pipeline, and also 
> not actually necessary to support the purpose for which 'optnone' was 
> invented.
>
> If you have a goal of making 'optnone' functions use the actual -O0 pipeline, 
> while non-optnone functions use the optimizing pipeline, more power to you 
> and you will need to take up that particular design challenge with Chandler 
> first.


Oh, maybe you are thinking of eliminating the -O0 pipeline?  Because if -O0 
implies optnone then it's kinda-sorta the same thing as the optimization 
pipeline operating on nothing but optnone functions?  I'd think that would make 
-O0 compilations slow down, which would not be a feature.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640588, @mehdi_amini wrote:

> Actually, as mentioned before, I could be fine with making `O0` incompatible 
> with LTO, however security features like CFI (or other sort of whole-program 
> analyses/instrumentations) requires LTO.


Well, "requires LTO" is overstating the case, AFAICT from the link you gave me. 
 Doesn't depend on //optimization// at all.  It depends on some interprocedural 
analyses given some particular scope/visibility boundary, which it is 
convenient to define as a set of linked bitcode modules, that by some happy 
chance is the same set of linked bitcode modules that LTO will operate on.

If it's important to support combining a bitcode version of my-application with 
your-bitcode-library for this CFI or whatever, and you also want to let me have 
my-application be unoptimized while your-bitcode-library gets optimized, NOW we 
have a use-case.  (Maybe that's what you had in mind earlier, but for some 
reason I wasn't able to extract that out of any prior comments.  No matter.)

I'm now thinking along the lines of a `-foptimize-off` flag (bikesheds welcome) 
which would set the default for the pragma to 'off'.  How is that different 
than what you wanted for `-O0`?  It is defined in terms of an existing pragma, 
which is WAY easier to explain and WAY easier to implement.  And, it still lets 
us say that `-c -O0 -flto` is a mistake, if that seems like a useful thing to 
say.

Does that seem reasonable?  Fit your understanding of the needs?


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640170, @probinson wrote:

> In my experience, modifying source


Note that the source modification consists of adding `#pragma clang optimize 
off` to the top of the file.  It is not a complicated thing.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28404#640284, @probinson wrote:
>
> > In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote:
> >
> > > In https://reviews.llvm.org/D28404#640170, @probinson wrote:
> > >
> > > > In my experience, modifying source is by far simpler than hacking a 
> > > > build system to make a special case for compiler options for one module 
> > > > in an application.  (If you have a way to build Clang with everything 
> > > > done LTO except one module built with -O0, on Linux with ninja, I would 
> > > > be very curious to hear how you do that.)
> > >
> > >
> > > Static library, separated projects, etc.
> > >  We have tons of users...
> >
> >
> > Still waiting.
>
>
> Waiting for what?
>  We have use-cases, I gave you a few (vendor static libraries are one). 
> Again, if you think it is wrong to support O0 and LTO, then please elaborate.


Your original use-case described debugging a module in an application.  You 
claimed it was simpler to change the build options for a module than change the 
source, which I am still waiting to hear how/why that is simpler.

Your subsequent use cases are about entire sub-projects, which is entirely 
different and orthogonal to where you started.  Please elaborate on the 
original use case.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote:

> You just wrote above that " mixing -O0 and LTO " is wrong, *if* I were to 
> agree with you at some point, then I'd make it a hard error.


Yes, I was not clear that I meant that `-O0 -flto` on the same clang command 
line just seems nonsensical.  "Optimize my program without optimizing it" 
forsooth.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#639887, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28404#639874, @probinson wrote:
>
> > Over the weekend I had a thought:  Why is -O0 so special here?  That is, 
> > after going to all this trouble to propagate -O0 to LTO, how does this 
> > generalize to propagating -O1 or any other specific -O option?  (Maybe this 
> > question would be better dealt with on the dev list...)
>
>
> O0 is "special" like Os and Oz because we have an attribute for it and passes 
> "know" how to handle this attribute.
>  I guess no-one cares enough about 
> https://reviews.llvm.org/owners/package/1//https://reviews.llvm.org/owners/package/2//O3
>  to find a solution for these (in the context of LTO, I don't really care 
> about 
> https://reviews.llvm.org/owners/package/1//https://reviews.llvm.org/owners/package/2/).
>  It is likely that Og would need a special treatment at some point, maybe 
> with a new attribute as well, to inhibit optimization that can't preserve 
> debug info properly.


"I don't care" doesn't seem like much of a principle.

Optnone does not equal -O0.  It is a debugging aid for the programmer, because 
debugging optimized code sucks.  If you have an LTO-built application and want 
to de-optimize parts of it to aid with debugging, then you can use the pragma, 
as originally intended.  I don't think `-c -O0 -flto` should get this 
not-entirely-O0-like behavior.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28404#640170, @probinson wrote:
>
> > In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote:
> >
> > > In https://reviews.llvm.org/D28404#640046, @probinson wrote:
> > >
> > > > "I don't care" doesn't seem like much of a principle.
> > >
> > >
> > > Long version is: "There is no use-case, no users, so I don't have much 
> > > motivation to push it forward for the only sake of completeness". Does it 
> > > sound enough of a principle like that?
> >
> >
> > No.  You still need to have adequate justification for your use case, which 
> > I think you do not.
>
>
> I don't follow your logic. 
>  IIUC, you asked about "why not supporting `O1/O2/O3`" ; how is *not 
> supporting* these because their not useful / don't have use-case related to 
> "supporting `O0` is useful"?


Upfront, it seemed peculiar to handle only one optimization level.  After more 
thought, the whole idea of mixing -O0 and LTO seems wrong.  Sorry, should have 
signaled that I had changed my mind about it.

 Optnone does not equal -O0.  It is a debugging aid for the programmer, 
 because debugging optimized code sucks.  If you have an LTO-built 
 application and want to de-optimize parts of it to aid with debugging, 
 then you can use the pragma, as originally intended.
>>> 
>>> Having to modifying the source isn't friendly. Not being able to honor -O0 
>>> during LTO is not user-friendly.
>> 
>> IMO, '-O0' and '-flto' are conflicting options and therefore not deserving 
>> of special support.
> 
> You're advocating for *rejecting* O0 built module at link-time? We'd still 
> need to detect this though. Status-quo isn't acceptable.
>  Also, that's not practicable: what if I have an LTO static library for which 
> I don't have the source, now if I build my own file with -O0 -flto I can't 
> link anymore.

No, I'm saying they are conflicting options on the same Clang command line.
As long as your linker can handle foo.o and bar.bc on the same command line, 
not a problem.  (If your linker can't handle that, fix the linker first.)

>> In my experience, modifying source is by far simpler than hacking a build 
>> system to make a special case for compiler options for one module in an 
>> application.  (If you have a way to build Clang with everything done LTO 
>> except one module built with -O0, on Linux with ninja, I would be very 
>> curious to hear how you do that.)
> 
> Static library, separated projects, etc.
>  We have tons of users...

Still waiting.  Your up-front use case was about de-optimizing a module to 
assist debugging it within an LTO-built application, not building entire 
projects one way versus another.  If that is not actually your use case, you 
need to start over with the correct description.

   I don't think `-c -O0` should get this not-entirely-O0-like behavior.
>>> 
>>> What is "not-entirely"? And why do you think that?
>> 
>> "Not entirely" means that running the -O0 pipeline, and running an 
>> optimization pipeline but asking some subset of passes to turn themselves 
>> off, does not get you the same result.  And I think that because I'm the one 
>> who put 'optnone' upstream in the first place.  The case that particularly 
>> sticks in my memory is the register allocator, but I believe there are 
>> passes at every stage that do not turn themselves off for optnone.
> 
> That's orthogonal: you're saying we are not handling it correctly yet, I'm 
> just moving toward *fixing* all these.

It's not orthogonal; that's exactly how 'optnone' behaves today.  If you have 
proposed a redesign of how to mix optnone and non-optnone functions in the same 
compilation unit, in some way other than what's done today, I am not aware of 
it; can you point to your proposal?


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28404#640284, @probinson wrote:
>
> > Upfront, it seemed peculiar to handle only one optimization level.  After 
> > more thought, the whole idea of mixing -O0 and LTO seems wrong.  Sorry, 
> > should have signaled that I had changed my mind about it.
>
>
> You just haven't articulated 1) why it is wrong and 2) what should we do 
> about it.


"Optimize without optimizing" really?  Does not sound confused to you?  
Persuade me why it makes sense.

If it doesn't make sense, then yes making the `-O0 -flto` combination an error 
would be the right path.

Unless you are taking the position that `-flto` doesn't mean "use LTO" and 
instead means something else, like "emit bitcode" in which case you should be 
advocating to change the name of the option to say what it means.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28404#640046, @probinson wrote:
>
> > "I don't care" doesn't seem like much of a principle.
>
>
> Long version is: "There is no use-case, no users, so I don't have much 
> motivation to push it forward for the only sake of completeness". Does it 
> sound enough of a principle like that?


No.  You still need to have adequate justification for your use case, which I 
think you do not.

>> Optnone does not equal -O0.  It is a debugging aid for the programmer, 
>> because debugging optimized code sucks.  If you have an LTO-built 
>> application and want to de-optimize parts of it to aid with debugging, then 
>> you can use the pragma, as originally intended.
> 
> Having to modifying the source isn't friendly. Not being able to honor -O0 
> during LTO is not user-friendly.

IMO, '-O0' and '-flto' are conflicting options and therefore not deserving of 
special support.

In my experience, modifying source is by far simpler than hacking a build 
system to make a special case for compiler options for one module in an 
application.  (If you have a way to build Clang with everything done LTO except 
one module built with -O0, on Linux with ninja, I would be very curious to hear 
how you do that.)  But if your build system makes that easy, you can just as 
easily remove `-flto` as add `-O0` and thus get the result you want without 
trying to pass conflicting options to the compiler.  Or spending time 
implementing this patch.

>>   I don't think `-c -O0` should get this not-entirely-O0-like behavior.
> 
> What is "not-entirely"? And why do you think that?

"Not entirely" means that running the -O0 pipeline, and running an optimization 
pipeline but asking some subset of passes to turn themselves off, does not get 
you the same result.  And I think that because I'm the one who put 'optnone' 
upstream in the first place.  The case that particularly sticks in my memory is 
the register allocator, but I believe there are passes at every stage that do 
not turn themselves off for optnone.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#640682, @mehdi_amini wrote:

> > I'm now thinking along the lines of a `-foptimize-off` flag (bikesheds 
> > welcome) which would set the default for the pragma to 'off'.  How is that 
> > different than what you wanted for `-O0`?  It is defined in terms of an 
> > existing pragma, which is WAY easier to explain and WAY easier to 
> > implement.  And, it still lets us say that `-c -O0 -flto` is a mistake, if 
> > that seems like a useful thing to say.
>
> Well -O0 being actually "disable optimization", I found "way easier" to 
> handle everything the same way (pragma, command line, etc.). I kind of find 
> it confusing for the user to differentiate `-O0` from `-foptimize=off`. What 
> is supposed to change between the two?


There is a pedantic difference, rooted in the still-true factoid that O0 != 
optnone.
If we redefine LTO as "Link Time Operation" (rather than Optimization; see my 
reply to Duncan)  then `-O0 -flto` is no longer an oxymoron, but using the 
attribute to imply the optimization level is still not good fidelity to what 
the user asked for.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

@rsmith could you say whether it seems reasonable to have a LangOpts flag that 
basically means "`pragma clang optimize off` is always in effect."  I think it 
would make the other optnone-related logic simpler.  It would not be the only 
sort-of-codegen-related flag in LangOpts (e.g. the PIC/PIE stuff).

In https://reviews.llvm.org/D28404#641538, @probinson wrote:

> There is another way to make use of the attribute, which I think will be more 
> robust:
>
> Have Sema pretend the pragma is in effect at all times, at -O0.  Then all the 
> existing conflict detection/resolution logic Just Works, and there's no need 
> to spend 4 lines of code hoping to replicate the correct conditions in 
> CodeGenModule.
>
> Because Sema does not have a handle on CodeGenOptions and therefore does not 
> a-priori know the optimization level, probably the right thing to do is move 
> the flag to LangOpts and set it under the correct conditions in 
> CompilerInvocation.  It wouldn't be the first codegen-like option in LangOpts.



https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#641078, @chandlerc wrote:

> For me, the arguments you're raising against -O0 and -flto don't hold up on 
> closer inspection:
>
> - O0 != optnone: correct. But this is only visible in LTO. And in LTO, Os != 
> optsize, and Oz != minsize. But we use optsize and minsize to communicate 
> between the compilation and the LTO step to the best of our ability the 
> intent of the programmer. It appears we can use optnone exactly the same way 
> here.


If the design decision is that relevant optimization controls are propagated 
into bitcode as function attributes, I grumble but concede it will do something 
similar to what was requested.

It does bother me that we keep finding things that LTO needs to know but which 
it does not know because it runs in a separate phase of the workflow.  I hope 
it is not a serious problem to ask "is there a more sensible way to fix this?"  
Maybe I'm not so good at expressing that so it comes out as a question rather 
than an objection, but that's basically what it is.

This design decision leaves -O1/-Og needing yet another attribute, when we get 
around to that, but I suppose Og would not have the 
interaction-with-other-attributes problems that optnone has.

> - optnone isn't *really* no optimizations: clearly this is true, but then 
> neither is -O0. We run the always inliner, a couple of other passes, and we 
> run several parts of the code generators optimizer. I understand why optnone 
> deficiencies (ie, too many optimizations) might be frustrating, but having 
> *more users* seems likely to make this *better*.

We have picked all the low-hanging fruit there, and probably some 
medium-hanging fruit.  Mehdi did have the misunderstanding that optnone == -O0 
and that I think was worth correcting.

> - There is no use case for -O0 + -flto:

The email thread has an exchange between Duncan and me, where I accept the use 
case.

> But all of this seems like an attempt to argue "you are wrong to have your 
> use case". I personally find that an unproductive line of discussion.

Not saying it was *wrong* just the description did not convey adequate 
justification.  Listing a few project types does not constitute a use case.  We 
did get to one, eventually, and it even involved differences in optimization 
levels.

> For example, you might ask: could we find some other way to solve the problem 
> you are trying to solve here?

There is another way to make use of the attribute, which I think will be more 
robust:

Have Sema pretend the pragma is in effect at all times, at -O0.  Then all the 
existing conflict detection/resolution logic Just Works, and there's no need to 
spend 4 lines of code hoping to replicate the correct conditions in 
CodeGenModule.

Because Sema does not have a handle on CodeGenOptions and therefore does not 
a-priori know the optimization level, probably the right thing to do is move 
the flag to LangOpts and set it under the correct conditions in 
CompilerInvocation.  It wouldn't be the first codegen-like option in LangOpts.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#641557, @mehdi_amini wrote:

> As I stand right now, there hasn't been any correction. 
>  I still consider the fact that `optnone` wouldn't produce the "same" result 
> (modulo corner cases around `merging global variables` for instance) as O0 a 
> bug that need to be fixed.


Why?  That's not the purpose of optnone.  You've already admitted there are 
some differences.  Why are other differences important?


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#641606, @mehdi_amini wrote:

> If we want to support `-O0 -flto` and `optnone` it the way to convey this to 
> the optimizer, I don't see the alternative.


optsize != -Os (according to Chandler)
minsize != -Oz (according to Chandler)
optnone != -O0 (according to both me and Chandler)

optnone is not "the way to convey (-O0) to the optimizer."  Please get that 
misunderstanding out of your head.  Clang handles -O0 by creating a short, 
minimalist pipeline, and running everything through it.  Clang handles -O2 by 
creating a fuller optimization pipeline, and functions with 'optnone' skip many 
of the passes in the pipeline.

These are architecturally different processes, you are not going to be able to 
make 'optnone' behave exactly like -O0 without major redesign of how the 
pipelines work.


https://reviews.llvm.org/D28404



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

[PATCH] D28503: Documentation for the newly added x86 intrinsics.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: emmintrin.h:1607
+///
+/// This intrinsic corresponds to the  VMOVSD / MOVSD  instruction.
+///

should this be VMOVQ/MOVQ instead?


Repository:
  rL LLVM

https://reviews.llvm.org/D28503



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

[PATCH] D28620: Guard __gnuc_va_list typedef

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added a reviewer: yaron.keren.
probinson added a subscriber: cfe-commits.

https://reviews.llvm.org/D28620

Files:
  lib/Headers/stdarg.h
  test/Headers/stdarg-gnuc_va_list.c


Index: test/Headers/stdarg-gnuc_va_list.c
===
--- test/Headers/stdarg-gnuc_va_list.c
+++ test/Headers/stdarg-gnuc_va_list.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wsystem-headers -std=c99 %s
+// expected-no-diagnostics
+
+// Check that no warnings are emitted from stdarg.h if __gnuc_va_list has
+// previously been defined in another header file.
+typedef __builtin_va_list __va_list;
+typedef __va_list __gnuc_va_list;
+#define __GNUC_VA_LIST
+
+#include 
Index: lib/Headers/stdarg.h
===
--- lib/Headers/stdarg.h
+++ lib/Headers/stdarg.h
@@ -46,7 +46,7 @@
 /* Hack required to make standard headers work, at least on Ubuntu */
 #ifndef __GNUC_VA_LIST
 #define __GNUC_VA_LIST 1
-#endif
 typedef __builtin_va_list __gnuc_va_list;
+#endif
 
 #endif /* __STDARG_H */


Index: test/Headers/stdarg-gnuc_va_list.c
===
--- test/Headers/stdarg-gnuc_va_list.c
+++ test/Headers/stdarg-gnuc_va_list.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wsystem-headers -std=c99 %s
+// expected-no-diagnostics
+
+// Check that no warnings are emitted from stdarg.h if __gnuc_va_list has
+// previously been defined in another header file.
+typedef __builtin_va_list __va_list;
+typedef __va_list __gnuc_va_list;
+#define __GNUC_VA_LIST
+
+#include 
Index: lib/Headers/stdarg.h
===
--- lib/Headers/stdarg.h
+++ lib/Headers/stdarg.h
@@ -46,7 +46,7 @@
 /* Hack required to make standard headers work, at least on Ubuntu */
 #ifndef __GNUC_VA_LIST
 #define __GNUC_VA_LIST 1
-#endif
 typedef __builtin_va_list __gnuc_va_list;
+#endif
 
 #endif /* __STDARG_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Maybe instead, pass a flag to enable setting optnone on everything when the 
driver sees `-O0 -flto`?  The patch as-is obviously has a massive testing cost, 
and it's easy to imagine people being tripped up by this in the future.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28404#638217, @probinson wrote:
>
> > The patch as-is obviously has a massive testing cost, and it's easy to 
> > imagine people being tripped up by this in the future.
>
>
> Can you clarify what massive testing cost you're referring to?


Well, you just had to modify around 50 tests, and I'd expect some future tests 
to have to deal with it too.  Maybe "massive" is overstating it but it seemed 
like an unusually large number.

I don't know that just slapping the option on all these tests is really the 
most appropriate fix, either, in some cases.  I'll look at it more.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D28404#638350, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28404#638299, @probinson wrote:
>
> > In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote:
> >
> > > In https://reviews.llvm.org/D28404#638217, @probinson wrote:
> > >
> > > > The patch as-is obviously has a massive testing cost, and it's easy to 
> > > > imagine people being tripped up by this in the future.
> > >
> > >
> > > Can you clarify what massive testing cost you're referring to?
> >
> >
> > Well, you just had to modify around 50 tests, and I'd expect some future 
> > tests to have to deal with it too.  Maybe "massive" is overstating it but 
> > it seemed like an unusually large number.
>
>
> There are two things:
>
> - tests are modified: when adding a new option, it does not seems unusual to 
> me


50 seems rather more than usual, but whatever.  Granted it's not hundreds.

> - what impact on future testing. I still don't see any of this future 
> "testing cost" you're referring to right now.

Maybe I worry too much.

I am getting a slightly different set of test failures than you did though.  I 
get these failures:
CodeGen/aarch64-neon-extract.c
CodeGen/aarch64-poly128.c
CodeGen/arm-neon-shifts.c
CodeGen/arm64-crc32.c

And I don't get these failures:
CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
CodeGenCXX/apple-kext-no-staticinit-section.cpp
CodeGenCXX/debug-info-global-ctor-dtor.cpp




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:900
+// OptimizeNone implies noinline; we should not be inlining such
+// functions.
 B.addAttribute(llvm::Attribute::NoInline);

I'd set ShouldAddOptNone = false here, as it's already explicit.



Comment at: clang/test/CodeGen/aarch64-neon-2velem.c:1
-// RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon 
-emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon 
-disable-O0-optnone -disable-O0-optnone -emit-llvm -o - %s | opt -S -mem2reg | 
FileCheck %s
 

Option specified twice.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:910-912
 // OptimizeNone wins over OptimizeForSize and MinSize.
 F->removeFnAttr(llvm::Attribute::OptimizeForSize);
 F->removeFnAttr(llvm::Attribute::MinSize);

mehdi_amini wrote:
> chandlerc wrote:
> > Is this still at all correct? Why? it seems pretty confusing especially in 
> > conjunction with the code below.
> > 
> > 
> > I think this may force you to either:
> > a) stop early-marking of -Os and -Oz flags with these attributes (early: 
> > prior to calling this routine) and handling all of the -O flag synthesized 
> > attributes here, or
> > b) set optnone for -O0 wher ewe set optsize for -Os and friends, and then 
> > remove it where necessary here.
> > 
> > I don't have any strong opinion about a vs. b.
> I believe it is still correct: during Os/Oz we reach this point and figure 
> that there is `__attribute__((optnone))` in the *source* (not `-O0`), we 
> remove the attributes, nothing changes. Did I miss something?
> 
Hmmm the Os/Oz attributes are added in CGCall.cpp, and are guarded with a check 
on the presence of the Optnone source attribute, so if the Optnone source 
attribute is present we should never see these.  And Os/Oz set 
OptimizationLevel to 2, which is not zero, so we won't come through here for 
ShouldAddOptNone reasons either.
Therefore these 'remove' calls should be no-ops and could be removed.  (For 
paranoia you could turn them into asserts, and do some experimenting to see 
whether I'm confused about how this all fits together.)


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:962
+  ShouldAddOptNone &= !D->hasAttr();
+  ShouldAddOptNone &= !D->hasAttr();
+  ShouldAddOptNone &= !F->hasFnAttribute(llvm::Attribute::AlwaysInline);

chandlerc wrote:
> why is optnone incompatible with *cold*
Because cold implies OptimizeForSize (just above this).  I take no position on 
whether that is reasonable.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:896
+  !CodeGenOpts.DisableO0ImplyOptNone && CodeGenOpts.OptimizationLevel == 0;
+  // We can't add optnone in the following cases, it won't pass the verifier
+  ShouldAddOptNone &= !D->hasAttr();

Period at the end of a comment.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:900
+  ShouldAddOptNone &= !D->hasAttr();
+  if (ShouldAddOptNone) {
+B.addAttribute(llvm::Attribute::OptimizeNone);

This block is redundant now?  The same things are added in the next if block.


https://reviews.llvm.org/D28404



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Over the weekend I had a thought:  Why is -O0 so special here?  That is, after 
going to all this trouble to propagate -O0 to LTO, how does this generalize to 
propagating -O1 or any other specific -O option?  (Maybe this question would be 
better dealt with on the dev list...)


https://reviews.llvm.org/D28404



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

[PATCH] D27994: Make two vtable tests tolerate C++11

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added a reviewer: rjmccall.
probinson added a subscriber: cfe-commits.

In C++11, we don't emit vtables as eagerly as we do for C++03, so fiddle the 
tests to emit them when the test expects them.

In the C++11 test cleanup project, we're commonly making the tests run in both 
dialects and sometimes with no dialect specified (as Clang's default will 
presumably advance to C++14/17 at some point).  I didn't do that for 
vtable-layout.cpp because it runs FileCheck 46 times, and replicating that 
really seemed like too much.
If it also seems like too much for vtable-linkage.cpp, the easy thing is to 
force it to C++03.


https://reviews.llvm.org/D27994

Files:
  test/CodeGenCXX/vtable-layout.cpp
  test/CodeGenCXX/vtable-linkage.cpp


Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o %t
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -std=c++03 -o %t.03
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -std=c++11 -o %t.11
 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -disable-llvm-optzns -O3 
-emit-llvm -o %t.opt
 // RUN: FileCheck %s < %t
+// RUN: FileCheck %s < %t.03
+// RUN: FileCheck %s < %t.11
 // RUN: FileCheck --check-prefix=CHECK-OPT %s < %t.opt
 
 namespace {
@@ -33,6 +37,11 @@
 
 static struct : D { } e;
 
+// Force 'e' to be constructed and therefore have a vtable defined.
+void use_e() {
+  e.f();
+}
+
 // The destructor is the key function.
 template
 struct E {
Index: test/CodeGenCXX/vtable-layout.cpp
===
--- test/CodeGenCXX/vtable-layout.cpp
+++ test/CodeGenCXX/vtable-layout.cpp
@@ -1919,6 +1919,8 @@
 virtual int i(int);
 virtual int i();
   };
+  // Force C's vtable to be generated.
+  int C::f() { return 1; }
 
   class D : C {};
 


Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o %t
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -std=c++03 -o %t.03
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -std=c++11 -o %t.11
 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -disable-llvm-optzns -O3 -emit-llvm -o %t.opt
 // RUN: FileCheck %s < %t
+// RUN: FileCheck %s < %t.03
+// RUN: FileCheck %s < %t.11
 // RUN: FileCheck --check-prefix=CHECK-OPT %s < %t.opt
 
 namespace {
@@ -33,6 +37,11 @@
 
 static struct : D { } e;
 
+// Force 'e' to be constructed and therefore have a vtable defined.
+void use_e() {
+  e.f();
+}
+
 // The destructor is the key function.
 template
 struct E {
Index: test/CodeGenCXX/vtable-layout.cpp
===
--- test/CodeGenCXX/vtable-layout.cpp
+++ test/CodeGenCXX/vtable-layout.cpp
@@ -1919,6 +1919,8 @@
 virtual int i(int);
 virtual int i();
   };
+  // Force C's vtable to be generated.
+  int C::f() { return 1; }
 
   class D : C {};
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27936: C++11 test cleanup: nonthrowing destructors

[Paul Robinson via Phabricator via cfe-commits]

probinson added a reviewer: rjmccall.
probinson added a comment.

+rjmccall as IR Gen owner.


https://reviews.llvm.org/D27936



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

[PATCH] D27936: C++11 test cleanup: nonthrowing destructors

[Paul Robinson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290207: C++11 test cleanup: nonthrowing destructors 
(authored by probinson).

Changed prior to commit:
  https://reviews.llvm.org/D27936?vs=81982=82157#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27936

Files:
  cfe/trunk/test/CodeGenCXX/destructors.cpp
  cfe/trunk/test/CodeGenCXX/nrvo.cpp
  cfe/trunk/test/CodeGenCXX/partial-destruction.cpp

Index: cfe/trunk/test/CodeGenCXX/partial-destruction.cpp
===
--- cfe/trunk/test/CodeGenCXX/partial-destruction.cpp
+++ cfe/trunk/test/CodeGenCXX/partial-destruction.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - -fcxx-exceptions -fexceptions | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - -fcxx-exceptions -fexceptions -std=c++03 | FileCheck %s -check-prefixes=CHECK,CHECKv03
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - -fcxx-exceptions -fexceptions -std=c++11 | FileCheck %s -check-prefixes=CHECK,CHECKv11
 
 // Test IR generation for partial destruction of aggregates.
 
@@ -45,7 +46,8 @@
   // CHECK-NEXT: br label
   // CHECK:  [[ED_AFTER:%.*]] = phi [[A]]* [ [[ED_END]], {{%.*}} ], [ [[ED_CUR:%.*]], {{%.*}} ]
   // CHECK-NEXT: [[ED_CUR]] = getelementptr inbounds [[A]], [[A]]* [[ED_AFTER]], i64 -1
-  // CHECK-NEXT: invoke void @_ZN5test01AD1Ev([[A]]* [[ED_CUR]])
+  // CHECKv03-NEXT: invoke void @_ZN5test01AD1Ev([[A]]* [[ED_CUR]])
+  // CHECKv11-NEXT: call   void @_ZN5test01AD1Ev([[A]]* [[ED_CUR]])
   // CHECK:  [[T0:%.*]] = icmp eq [[A]]* [[ED_CUR]], [[ED_BEGIN]]
   // CHECK-NEXT: br i1 [[T0]],
   // CHECK:  ret void
@@ -58,7 +60,8 @@
   // CHECK-NEXT: br i1 [[T0]],
   // CHECK:  [[E_AFTER:%.*]] = phi [[A]]* [ [[PARTIAL_END]], {{%.*}} ], [ [[E_CUR:%.*]], {{%.*}} ]
   // CHECK-NEXT: [[E_CUR]] = getelementptr inbounds [[A]], [[A]]* [[E_AFTER]], i64 -1
-  // CHECK-NEXT: invoke void @_ZN5test01AD1Ev([[A]]* [[E_CUR]])
+  // CHECKv03-NEXT: invoke void @_ZN5test01AD1Ev([[A]]* [[E_CUR]])
+  // CHECKv11-NEXT: call   void @_ZN5test01AD1Ev([[A]]* [[E_CUR]])
   // CHECK:  [[T0:%.*]] = icmp eq [[A]]* [[E_CUR]], [[E_BEGIN]]
   // CHECK-NEXT: br i1 [[T0]],
 
@@ -73,20 +76,21 @@
   // FIXME: There's some really bad block ordering here which causes
   // the partial destroy for the primary normal destructor to fall
   // within the primary EH destructor.
-  // CHECK:  landingpad { i8*, i32 }
-  // CHECK-NEXT:   cleanup
-  // CHECK:  [[T0:%.*]] = icmp eq [[A]]* [[ED_BEGIN]], [[ED_CUR]]
-  // CHECK-NEXT: br i1 [[T0]]
-  // CHECK:  [[EDD_AFTER:%.*]] = phi [[A]]* [ [[ED_CUR]], {{%.*}} ], [ [[EDD_CUR:%.*]], {{%.*}} ]
-  // CHECK-NEXT: [[EDD_CUR]] = getelementptr inbounds [[A]], [[A]]* [[EDD_AFTER]], i64 -1
-  // CHECK-NEXT: invoke void @_ZN5test01AD1Ev([[A]]* [[EDD_CUR]])
-  // CHECK:  [[T0:%.*]] = icmp eq [[A]]* [[EDD_CUR]], [[ED_BEGIN]]
-  // CHECK-NEXT: br i1 [[T0]]
+  // CHECKv03:  landingpad { i8*, i32 }
+  // CHECKv03-NEXT:   cleanup
+  // CHECKv03:  [[T0:%.*]] = icmp eq [[A]]* [[ED_BEGIN]], [[ED_CUR]]
+  // CHECKv03-NEXT: br i1 [[T0]]
+  // CHECKv03:  [[EDD_AFTER:%.*]] = phi [[A]]* [ [[ED_CUR]], {{%.*}} ], [ [[EDD_CUR:%.*]], {{%.*}} ]
+  // CHECKv03-NEXT: [[EDD_CUR]] = getelementptr inbounds [[A]], [[A]]* [[EDD_AFTER]], i64 -1
+  // CHECKv03-NEXT: invoke void @_ZN5test01AD1Ev([[A]]* [[EDD_CUR]])
+  // CHECKv03:  [[T0:%.*]] = icmp eq [[A]]* [[EDD_CUR]], [[ED_BEGIN]]
+  // CHECKv03-NEXT: br i1 [[T0]]
 
   // Back to the primary EH destructor.
   // CHECK:  [[E_AFTER:%.*]] = phi [[A]]* [ [[E_END]], {{%.*}} ], [ [[E_CUR:%.*]], {{%.*}} ]
   // CHECK-NEXT: [[E_CUR]] = getelementptr inbounds [[A]], [[A]]* [[E_AFTER]], i64 -1
-  // CHECK-NEXT: invoke void @_ZN5test01AD1Ev([[A]]* [[E_CUR]])
+  // CHECKv03-NEXT: invoke void @_ZN5test01AD1Ev([[A]]* [[E_CUR]])
+  // CHECKv11-NEXT: call   void @_ZN5test01AD1Ev([[A]]* [[E_CUR]])
   // CHECK:  [[T0:%.*]] = icmp eq [[A]]* [[E_CUR]], [[E0]]
   // CHECK-NEXT: br i1 [[T0]],
 
@@ -120,8 +124,10 @@
   // CHECK-NEXT:   cleanup
   // CHECK:  landingpad { i8*, i32 }
   // CHECK-NEXT:   cleanup
-  // CHECK:  invoke void @_ZN5test11AD1Ev([[A]]* [[Y]])
-  // CHECK:  invoke void @_ZN5test11AD1Ev([[A]]* [[X]])
+  // CHECKv03:  invoke void @_ZN5test11AD1Ev([[A]]* [[Y]])
+  // CHECKv03:  invoke void @_ZN5test11AD1Ev([[A]]* [[X]])
+  // CHECKv11:  call   void @_ZN5test11AD1Ev([[A]]* [[Y]])
+  // CHECKv11:  call   void @_ZN5test11AD1Ev([[A]]* [[X]])
 }
 
 namespace test2 {
@@ -153,7 +159,8 @@
 // CHECK-NEXT: br i1 [[EMPTY]],
 // CHECK:  [[PAST:%.*]] = phi [[A]]* [ [[CUR]], {{%.*}} ], [ [[DEL:%.*]], {{%.*}} ]
 // CHECK-NEXT: [[DEL]] = getelementptr inbounds [[A]], [[A]]* [[PAST]], i64 -1
-// CHECK-NEXT: invoke void @_ZN5test21AD1Ev([[A]]* [[DEL]])
+// CHECKv03-NEXT: invoke void 

[PATCH] D27994: Make two vtable tests tolerate C++11

[Paul Robinson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290205: Make two vtable tests tolerate C++11. (authored by 
probinson).

Changed prior to commit:
  https://reviews.llvm.org/D27994?vs=82126=82156#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27994

Files:
  cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
  cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp


Index: cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
===
--- cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
+++ cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
@@ -1919,6 +1919,8 @@
 virtual int i(int);
 virtual int i();
   };
+  // Force C's vtable to be generated.
+  int C::f() { return 1; }
 
   class D : C {};
 
Index: cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
===
--- cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
+++ cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o %t
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -std=c++03 -o %t.03
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -std=c++11 -o %t.11
 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -disable-llvm-optzns -O3 
-emit-llvm -o %t.opt
 // RUN: FileCheck %s < %t
+// RUN: FileCheck %s < %t.03
+// RUN: FileCheck %s < %t.11
 // RUN: FileCheck --check-prefix=CHECK-OPT %s < %t.opt
 
 namespace {
@@ -33,6 +37,11 @@
 
 static struct : D { } e;
 
+// Force 'e' to be constructed and therefore have a vtable defined.
+void use_e() {
+  e.f();
+}
+
 // The destructor is the key function.
 template
 struct E {


Index: cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
===
--- cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
+++ cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
@@ -1919,6 +1919,8 @@
 virtual int i(int);
 virtual int i();
   };
+  // Force C's vtable to be generated.
+  int C::f() { return 1; }
 
   class D : C {};
 
Index: cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
===
--- cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
+++ cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o %t
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -std=c++03 -o %t.03
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -std=c++11 -o %t.11
 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -disable-llvm-optzns -O3 -emit-llvm -o %t.opt
 // RUN: FileCheck %s < %t
+// RUN: FileCheck %s < %t.03
+// RUN: FileCheck %s < %t.11
 // RUN: FileCheck --check-prefix=CHECK-OPT %s < %t.opt
 
 namespace {
@@ -33,6 +37,11 @@
 
 static struct : D { } e;
 
+// Force 'e' to be constructed and therefore have a vtable defined.
+void use_e() {
+  e.f();
+}
+
 // The destructor is the key function.
 template
 struct E {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27956: Make CodeGenCXX/stack-reuse-miscompile.cpp tolerate C++11

[Paul Robinson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290208: Make a test use a specific C++ dialect (authored by 
probinson).

Changed prior to commit:
  https://reviews.llvm.org/D27956?vs=82020=82160#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27956

Files:
  cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp


Index: cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp
===
--- cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -S -target armv7l-unknown-linux-gnueabihf -emit-llvm -O1 -mllvm 
-disable-llvm-optzns -S %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple armv7l-unknown-linux-gnueabihf -emit-llvm -O1 
-disable-llvm-optzns -std=c++03 %s -o - | FileCheck %s
 
 // This test should not to generate llvm.lifetime.start/llvm.lifetime.end for
 // f function because all temporary objects in this function are used for the


Index: cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp
===
--- cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -S -target armv7l-unknown-linux-gnueabihf -emit-llvm -O1 -mllvm -disable-llvm-optzns -S %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple armv7l-unknown-linux-gnueabihf -emit-llvm -O1 -disable-llvm-optzns -std=c++03 %s -o - | FileCheck %s
 
 // This test should not to generate llvm.lifetime.start/llvm.lifetime.end for
 // f function because all temporary objects in this function are used for the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27794: Make some diagnostic tests C++11 clean

[Paul Robinson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290262: Make some diagnostic tests C++11 clean. (authored by 
probinson).

Changed prior to commit:
  https://reviews.llvm.org/D27794?vs=81977=82248#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27794

Files:
  cfe/trunk/test/FixIt/fixit.cpp
  cfe/trunk/test/Parser/backtrack-off-by-one.cpp
  cfe/trunk/test/SemaCXX/copy-assignment.cpp

Index: cfe/trunk/test/FixIt/fixit.cpp
===
--- cfe/trunk/test/FixIt/fixit.cpp
+++ cfe/trunk/test/FixIt/fixit.cpp
@@ -1,8 +1,12 @@
-// RUN: %clang_cc1 -pedantic -Wall -Wno-comment -verify -fcxx-exceptions -x c++ %s
+// RUN: %clang_cc1 -pedantic -Wall -Wno-comment -verify -fcxx-exceptions -x c++ -std=c++98 %s
+// RUN: cp %s %t-98
+// RUN: not %clang_cc1 -pedantic -Wall -Wno-comment -fcxx-exceptions -fixit -x c++ -std=c++98 %t-98
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -Wno-comment -fcxx-exceptions -x c++ -std=c++98 %t-98
 // RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -x c++ -std=c++11 %s 2>&1 | FileCheck %s
-// RUN: cp %s %t
-// RUN: not %clang_cc1 -pedantic -Wall -Wno-comment -fcxx-exceptions -fixit -x c++ %t
-// RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -Wno-comment -fcxx-exceptions -x c++ %t
+// RUN: %clang_cc1 -pedantic -Wall -Wno-comment -verify -fcxx-exceptions -x c++ -std=c++11 %s
+// RUN: cp %s %t-11
+// RUN: not %clang_cc1 -pedantic -Wall -Wno-comment -fcxx-exceptions -fixit -x c++ -std=c++11 %t-11
+// RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror -Wno-comment -fcxx-exceptions -x c++ -std=c++11 %t-11
 
 /* This is a test of the various code modification hints that are
provided as part of warning or extension diagnostics. All of the
@@ -21,7 +25,11 @@
 
 template struct CT { template struct Inner; }; // expected-note{{previous use is here}}
 
+// FIXME: In C++11 this gets 'expected unqualified-id' which fixit can't fix.
+// Probably parses as `CT<10> > 2 > ct;` rather than `CT<(10 >> 2)> ct;`.
+#if __cplusplus < 201103L
 CT<10 >> 2> ct; // expected-warning{{require parentheses}}
+#endif
 
 class C3 {
 public:
@@ -41,7 +49,11 @@
 };
 
 class B : public A {
+#if __cplusplus >= 201103L
+  A::foo; // expected-error{{ISO C++11 does not allow access declarations}}
+#else
   A::foo; // expected-warning{{access declarations are deprecated}}
+#endif
 };
 
 void f() throw(); // expected-note{{previous}}
@@ -285,8 +297,10 @@
 void (*p)() = ;
 (void)(==p); // expected-error {{use '> ='}}
 (void)(>=p); // expected-error {{use '> >'}}
+#if __cplusplus < 201103L
 (void)(>=p); // expected-error {{use '> >'}}
 (Shr)>>=p; // expected-error {{use '> >'}}
+#endif
 
 // FIXME: We correct this to ' > >= p;' not ' >>= p;'
 //(Shr)>>=p;
Index: cfe/trunk/test/Parser/backtrack-off-by-one.cpp
===
--- cfe/trunk/test/Parser/backtrack-off-by-one.cpp
+++ cfe/trunk/test/Parser/backtrack-off-by-one.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify %s -std=c++98
+// RUN: %clang_cc1 -verify %s -std=c++11
 
 // PR25946
 // We had an off-by-one error in an assertion when annotating A below.  Our
@@ -10,8 +12,10 @@
 
 // expected-error@+1 {{expected '{' after base class list}}
 template  class B : T // not ',' or '{'
-// expected-error@+3 {{C++ requires a type specifier for all declarations}}
-// expected-error@+2 {{expected ';' after top level declarator}}
+#if __cplusplus < 201103L
+// expected-error@+4 {{expected ';' after top level declarator}}
+#endif
+// expected-error@+2 {{C++ requires a type specifier for all declarations}}
 // expected-error@+1 {{expected ';' after class}}
 A {
 };
Index: cfe/trunk/test/SemaCXX/copy-assignment.cpp
===
--- cfe/trunk/test/SemaCXX/copy-assignment.cpp
+++ cfe/trunk/test/SemaCXX/copy-assignment.cpp
@@ -1,12 +1,22 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+
+#if __cplusplus >= 201103L
+// expected-note@+3 2 {{candidate constructor}}
+// expected-note@+2 {{passing argument to parameter here}}
+#endif
 struct A {
 };
 
 struct ConvertibleToA {
   operator A();
 };
 
 struct ConvertibleToConstA {
+#if __cplusplus >= 201103L
+// expected-note@+2 {{candidate function}}
+#endif
   operator const A();
 };
 
@@ -69,6 +79,9 @@
   na = a;
   na = constA;
   na = convertibleToA;
+#if __cplusplus >= 201103L
+// expected-error@+2 {{no viable conversion}}
+#endif
   na = convertibleToConstA;
   na += a; // expected-error{{no viable overloaded '+='}}
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits