[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

2019-10-08 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

You may be iinterested to l


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682



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


[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

2019-10-08 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

In D67536#1699221 , @ilya-biryukov 
wrote:

> I have actually seen clients that just make the text gray and it looks pretty 
> nice (ReSharper for Visual Studio and IntelliJ IDEA definitely do that).


I don't think there is a conflict between this style, and using line 
highlightings: if the style being applied is a foreground color (gray text) as 
opposed to a background color, then it will look the same regardless of whether 
you apply it as a line style, or as a character range style.

> It also lets them consistently highlight part of the line (e.g. dead 
> expressions or statements can be marked in gray even if they are on the same 
> line).

Highlighting part of a line is not applicable to inactive preprocessor branches 
in C++.

If we later introduce highlightings for dead expressions or statements, we can 
of course use character ranges and not lines for them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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


[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

2019-10-08 Thread Nathan Ridge via Phabricator via cfe-commits
nridge planned changes to this revision.
nridge marked an inline comment as done.
nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:152
+  // Don't bother computing the offset for the end of the line, just 
use
+  // zero. The client will treat this highlighting kind specially, and
+  // highlight the entire line visually (i.e. not just to where the 
text

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > nridge wrote:
> > > > hokein wrote:
> > > > > This seems too couple with VSCode client, I would prefer to calculate 
> > > > > the range of the line and return to the client.
> > > > > 
> > > > > Is there any big differences in VSCode between highlighting with the 
> > > > > `isWholeLine` and highlighting with the range of the line? 
> > > > I took some screenshots to illustrate to difference.
> > > > 
> > > > Highlighting only to the end of the line of text:
> > > > 
> > > > {F10158508}
> > > > 
> > > > Highlighting the whole line:
> > > > 
> > > > {F10158515}
> > > > 
> > > > I think the first one looks pretty bad, and is inconsistent with 
> > > > existing practice.
> > > > 
> > > > Note also that the suggestion is not to special-case the VSCode client 
> > > > specifically; it's to special-case one particular highlighting, which 
> > > > any client can implement.
> > > > 
> > > > If this special-casing is really unpalatable, we could instead try this 
> > > > suggestion by @sammccall:
> > > > 
> > > > > Failing that, I'd suggest encoding a list of line-styles on 
> > > > > SemanticHighlightingInformation, that should be combined with any 
> > > > > tokens on that line.
> > > > 
> > > > I guess one consideration when evaluating these options is, do we 
> > > > expect to use that "list of line-styles" for anything else in the 
> > > > future? I can't think of anything at the moment, but perhaps there are 
> > > > other uses for it.
> > > > 
> > > > If not, we could do something slightly simpler, and add a single 
> > > > `isInactive` flag to `SemanticHighlightingInformation`.
> > > Three approaches seem feasible here:
> > > 1. clients that know about the specific scope can extend it to the whole 
> > > line. 
> > > 2. [0,0) or so indicates "highlight the whole line"
> > > 3. use a dedicated property for line styles (vs token styles)
> > > 
> > > 3 is clearly better than 2 I think, it's more explicit. I don't have a 
> > > strong opinion of 1 vs 3, but if going with 1 I think it's a good idea to 
> > > measure the line as Haojian says, so we at least get a basic version of 
> > > the feature if the client doesn't know about line styles.
> > > 
> > > > I guess one consideration when evaluating these options is, do we 
> > > > expect to use that "list of line-styles" for anything else in the 
> > > > future? I can't think of anything at the moment
> > > Preprocessor directives maybe? (Though these are easy enough for clients 
> > > to highlight with regex)
> > I can't say whether highlighting the line is better than highlighting the 
> > range of the line text, but below is the how the inactive TS code is 
> > highlighted in VSCode (only the range of text), I personally prefer this 
> > style.
> > 
> > {F10189885}
> I think that's an argument for making sure clients clearly distinguish 
> between regular tokens and marking lines: overlapping tokens don't compose 
> well, but we can easily say lines and token styles should compose.
> 
> (That particular style is not for me, but it doesn't matter)
I find this a convincing argument for using line styles, thanks.

Especially since, at some point in the future, I would like us to be able to 
produce regular token highlightings for inactive code, much like in that TS 
code screenshot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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


[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-10-08 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/docs/UsersManual.rst:1330
+   and ``fast``.
+   Details:
+

rjmccall wrote:
> "provided by other, single-purpose floating point options."
I don't know why you keep including "clang" as a modifier here; this is the 
clang documentation, and all of these options are clang options no matter where 
they might have been borrowed from.



Comment at: clang/docs/UsersManual.rst:1341
+   has been selected, then the compiler will issue a diagnostic warning
+   that the override has occurred.
+

mibintc wrote:
> rjmccall wrote:
> > That's not typical driver behavior; why this choice?
> The rationale for the warnings is that the floating point options are 
> sufficiently complicated that it makes sense to warn the uses that one of the 
> later options supplied on the command line is undoing a choice made earlier.  
> It's not obvious that e.g. the setting for fassociative-math is also 
> controlled by  -fp-model=strict
Okay.  Well, it's a new option, so new behavior is alright, but if you're 
worried about the collisions having arbitrary effects that you'll have to 
maintain compatibility with, you should consider making it an error instead, 
because a warning still means it's permitted.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

2019-10-08 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc created this revision.
poelmanc added reviewers: jonathanmeier, alexfh.
poelmanc added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre, ormris.
Herald added a project: clang.

Clang-tidy can ignore most formatting concerns as formatting is clang-format's 
job. However, clang-format treats blank lines as intentional separators, so 
clang-tidy fixes that accidentally insert blank lines can prevent clang-format 
from properly fixing formatting. For example, applying 
readability-redundant-member-init to:

  struct F10 {
F10() :
  f(),
  g()
{}
S f, g;
  };

results in:

  struct F10 {
F10()
  
  
{}
S f, g;
  };

which may get converted to:

  struct F10 {
F10()
  
  
  = default;
S f, g;
  };

The newly blank lines prevent clang-format from fixing this to the desired:

  struct F10 {
F10() = default;
S f, g;
  };

A few individual fixes take steps to reduce some newly blank lines, e.g. 
RedundantControlFlowCheck.cpp calls 
`Lexer::findLocationAfterToken(.../*SkipTrailingWhitespaceAndNewLine=*/true )` 
to chomp trailing newlines after redundant `continue;` or `break;` statements. 
But this approach is insufficient when multiple rules combine to generate a 
blank line and forces more work onto each fix writer.

This patch adds a function `removeNewlyBlankLinesFromReplacements` to 
ClangTidy.cpp that looks for sequences of Replacements that will result in 
making a previously non-blank line blank. When found a Replacement is added 
that removes the entire line, including newlines.

The patch adds the above and other tests to 
readability-redundant-member-init.cpp, and adds tests to 
readability-redundant-control-flow.cpp and 
readability-redundant-declaration.cpp to demonstrate its effectiveness in 
avoiding newly blank lines anywhere.

Two previously private/static helper functions are exposed to assist in the 
implementation: AST/CommentLexer's `skipNewline` and AST/CommentParser's 
`isWhitespace(StringRef)`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D68682

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-control-flow.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
  clang-tools-extra/test/clang-tidy/readability-redundant-member-init.cpp
  clang/include/clang/AST/CommentLexer.h
  clang/include/clang/AST/CommentParser.h
  clang/lib/AST/CommentLexer.cpp
  clang/lib/AST/CommentParser.cpp

Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -16,7 +16,8 @@
 
 namespace clang {
 
-static inline bool isWhitespace(llvm::StringRef S) {
+// Consider moving this useful function to a more general utility location.
+bool isWhitespace(llvm::StringRef S) {
   for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) {
 if (!isWhitespace(*I))
   return false;
Index: clang/lib/AST/CommentLexer.cpp
===
--- clang/lib/AST/CommentLexer.cpp
+++ clang/lib/AST/CommentLexer.cpp
@@ -16,6 +16,23 @@
 #include "llvm/Support/ErrorHandling.h"
 
 namespace clang {
+
+// Consider moving this useful function to a more general utility location.
+const char *skipNewline(const char *BufferPtr, const char *BufferEnd) {
+  if (BufferPtr == BufferEnd)
+return BufferPtr;
+
+  if (*BufferPtr == '\n')
+BufferPtr++;
+  else {
+assert(*BufferPtr == '\r');
+BufferPtr++;
+if (BufferPtr != BufferEnd && *BufferPtr == '\n')
+  BufferPtr++;
+  }
+  return BufferPtr;
+}
+
 namespace comments {
 
 void Token::dump(const Lexer , const SourceManager ) const {
@@ -131,21 +148,6 @@
   return BufferEnd;
 }
 
-const char *skipNewline(const char *BufferPtr, const char *BufferEnd) {
-  if (BufferPtr == BufferEnd)
-return BufferPtr;
-
-  if (*BufferPtr == '\n')
-BufferPtr++;
-  else {
-assert(*BufferPtr == '\r');
-BufferPtr++;
-if (BufferPtr != BufferEnd && *BufferPtr == '\n')
-  BufferPtr++;
-  }
-  return BufferPtr;
-}
-
 const char *skipNamedCharacterReference(const char *BufferPtr,
 const char *BufferEnd) {
   for ( ; BufferPtr != BufferEnd; ++BufferPtr) {
Index: clang/include/clang/AST/CommentParser.h
===
--- clang/include/clang/AST/CommentParser.h
+++ clang/include/clang/AST/CommentParser.h
@@ -22,6 +22,10 @@
 namespace clang {
 class SourceManager;
 
+/// Returns true if and only if S consists entirely of whitespace.
+/// Consider moving this useful function to a more general utility location.
+bool isWhitespace(llvm::StringRef S);
+
 namespace comments {
 class CommandTraits;
 
Index: clang/include/clang/AST/CommentLexer.h

[PATCH] D68578: [HIP] Fix device stub name

2019-10-08 Thread Tony Tye via Phabricator via cfe-commits
t-tye added a comment.

In D68578#1700652 , @tra wrote:

> In D68578#1698864 , @t-tye wrote:
>
> > From a source language point of view, the device function comprises the 
> > code that is launched as a grid. We need this fact to be present in the 
> > symbols used. Only the device function should have a symbol name matching 
> > the mangled name of the device function.
>
>
> What do you have in mind when you use 'symbol name' here? Is that a symbol as 
> seen by linker? If that's the case, do host and device share this name space 
> on AMD GPUs? In case of CUDA, linker symbols are per-target (i.e. host and 
> each GPU have their own spaces), so they never clash, but the kernel names 
> must have identical mangled name on host and all devices, so the host can 
> refer to the device-side kernel when it needs to launch it.


We want to support a heterogeneous gdb debugger for a single source programming 
language. We would like to follow the same conventions used by compilers that 
implement other languages supported by gdb. The debugger can use symbols to 
find functions. It supports unmangling them and using the unmangled name to 
indicate the source language function it corresponds to. We would like this to 
remain true. The stub is not the kernel function, it is a helper function that 
will launch the kernel. In many ways it is acting like other trampolines. 
Therefore, it should be named as a internal helper function. The debugger can 
chose what it wants to do with it, but it does not want to be confused into 
thinking it actually IS the kernel function. If the user sets a breakpoint in 
the code of the kernel function then that breakpoint should be hit by every 
instance of the kernel that is created by the dispatch. It should not be hit by 
the code that is initiatig the dispatch. If that is what the user wanted they 
would set a breakpoint at the statement that performs the dispatch launch.

Whether the kernel is present in the CPU or GPU code is s separate concept. If 
it is present in both, then both would have the same symbol as they are both 
implementing the kernel. The debugger would set a breakpoint in both as from a 
language execution model poit of view if either piece of code executes it 
corresponds to the same source language kernel.

> 
> 
>> It the device function has both a host and device implementation then both 
>> can have the source language function name for the symbol since both 
>> actually implement the device function. If the user asks to set a breakpoint 
>> in the device function then the debugger would set in both implementations 
>> so the user is notified when the source program executes the device 
>> function, regardless of which implementation is invoked. This is similar to 
>> the debugger setting a breakpoint in a function that is inlined into 
>> multiple places: the debugger sets breeakpoints in all the inlined places so 
>> the user can tstill think of the program debugging in terms of the source 
>> language semantics.
> 
> OK. This sounds like `__host__`/`__device__` function overloads and what 
> you're saying does make sense for that.

Right. Well its not really and overload, not a request to have instances of the 
kernel avaiable for either the CPU or GPU to execute. They are the same 
function, not different overloads.

> 
> 
>> In contrast, the stub is effectively part of the implementation of actually 
>> launching the device function. It should have a distinct name.
> 
> I'm not sure how the requirement of distinct name follows from the fact that 
> the stub is the host-side part of the device-side kernel? To me it looks like 
> an argument for them to have the same name so it's clear that they are both 
> part of the same function as written in the source.
> 
> The don't have to be different. CUDA (and HIP) does not allow overloading of 
> kernels, so the stub and the kernel can have identical names as in the 
> example of `__host__` and `__device__` overloads you've described above, only 
> now it's `__host__` stub + `__global__` kernel itself, instead of two 
> user-implemented functions. Debugger, of course, will need to know about that 
> to pick the stub or kernel as the breakpoint location, but that appears 
> doable.

As mentioned, the stub is not the host side part of the device side kernel. The 
stub is a means to launch the kernel. That launching could happen on the device 
(device enqueue), or on the host. The kernel itself could execute on the device 
or the host. There is the act of launching the kernel (the function call 
statement if you will), and the kernel instances that come into existence (the 
threads created to execute the body of the kernel according to the launch 
bounds presented at the launch statement).

The user may want to set a breakpoint at the launch statement, or in the body 
of the kernel. The language execution model treats those 

r374135 - [c++20] P1152R4: warn on any simple-assignment to a volatile lvalue

2019-10-08 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Tue Oct  8 19:04:54 2019
New Revision: 374135

URL: http://llvm.org/viewvc/llvm-project?rev=374135=rev
Log:
[c++20] P1152R4: warn on any simple-assignment to a volatile lvalue
whose value is not ignored.

We don't warn on all the cases that are deprecated: specifically, we
choose to not warn for now if there are parentheses around the
assignment but its value is not actually used. This seems like a more
defensible rule, particularly for cases like sizeof(v = a), where the
parens are part of the operand rather than the sizeof syntax.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/deprecated.cpp
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=374135=374134=374135=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct  8 19:04:54 
2019
@@ -6654,6 +6654,9 @@ def err_increment_decrement_enum : Error
 def warn_deprecated_increment_decrement_volatile : Warning<
   "%select{decrement|increment}0 of object of volatile-qualified type %1 "
   "is deprecated">, InGroup;
+def warn_deprecated_simple_assign_volatile : Warning<
+  "use of result of assignment to object of volatile-qualified type %0 "
+  "is deprecated">, InGroup;
 def warn_deprecated_compound_assign_volatile : Warning<
   "compound assignment to object of volatile-qualified type %0 is deprecated">,
   InGroup;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=374135=374134=374135=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Oct  8 19:04:54 2019
@@ -1062,6 +1062,11 @@ public:
 
 llvm::SmallPtrSet PossibleDerefs;
 
+/// Expressions appearing as the LHS of a volatile assignment in this
+/// context. We produce a warning for these when popping the context if
+/// they are not discarded-value expressions nor unevaluated operands.
+SmallVector VolatileAssignmentLHSs;
+
 /// \brief Describes whether we are in an expression constext which we have
 /// to handle differently.
 enum ExpressionKind {
@@ -4248,6 +4253,9 @@ public:
   ExprResult TransformToPotentiallyEvaluated(Expr *E);
   ExprResult HandleExprEvaluationContextForTypeof(Expr *E);
 
+  ExprResult CheckUnevaluatedOperand(Expr *E);
+  void CheckUnusedVolatileAssignment(Expr *E);
+
   ExprResult ActOnConstantExpression(ExprResult Res);
 
   // Functions for marking a declaration referenced.  These functions also

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=374135=374134=374135=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Oct  8 19:04:54 2019
@@ -3801,6 +3801,16 @@ bool Sema::CheckUnaryExprOrTypeTraitOper
   QualType ExprTy = E->getType();
   assert(!ExprTy->isReferenceType());
 
+  bool IsUnevaluatedOperand =
+  (ExprKind == UETT_SizeOf || ExprKind == UETT_AlignOf ||
+   ExprKind == UETT_PreferredAlignOf);
+  if (IsUnevaluatedOperand) {
+ExprResult Result = CheckUnevaluatedOperand(E);
+if (Result.isInvalid())
+  return true;
+E = Result.get();
+  }
+
   if (ExprKind == UETT_VecStep)
 return CheckVecStepTraitOperandType(*this, ExprTy, E->getExprLoc(),
 E->getSourceRange());
@@ -3838,9 +3848,8 @@ bool Sema::CheckUnaryExprOrTypeTraitOper
 
   // The operand for sizeof and alignof is in an unevaluated expression 
context,
   // so side effects could result in unintended consequences.
-  if ((ExprKind == UETT_SizeOf || ExprKind == UETT_AlignOf ||
-   ExprKind == UETT_PreferredAlignOf) &&
-  !inTemplateInstantiation() && E->HasSideEffects(Context, false))
+  if (IsUnevaluatedOperand && !inTemplateInstantiation() &&
+  E->HasSideEffects(Context, false))
 Diag(E->getExprLoc(), diag::warn_side_effects_unevaluated_context);
 
   if (CheckObjCTraitOperandConstraints(*this, ExprTy, E->getExprLoc(),
@@ -3939,8 +3948,6 @@ bool Sema::CheckUnaryExprOrTypeTraitOper
 }
 
 static bool CheckAlignOfExpr(Sema , Expr *E, UnaryExprOrTypeTrait ExprKind) {
-  E = E->IgnoreParens();
-
   // Cannot know anything else if the expression is dependent.
   if (E->isTypeDependent())
 return false;
@@ -3952,9 +3959,10 @@ static bool CheckAlignOfExpr(Sema , Ex
   }
 
   ValueDecl *D = nullptr;
-  if (DeclRefExpr *DRE = 

[PATCH] D68681: [libc++][test] Miscellaneous MSVC cleanups

2019-10-08 Thread Casey Carter via Phabricator via cfe-commits
CaseyCarter created this revision.
CaseyCarter added reviewers: mclow.lists, EricWF, ldionne.
Herald added a subscriber: dexonsmith.

- Silence unused-local-typedef warnings: 
`map.cons/assign_initializer_list.pass.cpp` (and the `set.cons` variant) uses a 
local typedef only within `LIBCPP_ASSERT`s, so clang diagnoses it as unused 
when testing non-libc++.

- Add missing include: `c.math/abs.pass.cpp` uses `std::numeric_limits` but 
failed to `#include `.

- Don't test non-type: A "recent" change to 
`meta.trans.other/underlying_type.pass.cpp` unconditionally tests the type `F` 
which is conditionally defined.

- Silence truncation warnings: A few deduction guide tests use 
`unordered_meow>` which triggers sirens and claxons (as 
it should). `midpoint.float.pass` uses `float meow = 3.14;`, which triggers 
bogus truncation warnings.


https://reviews.llvm.org/D68681

Files:
  test/std/containers/associative/map/map.cons/assign_initializer_list.pass.cpp
  test/std/containers/associative/set/set.cons/assign_initializer_list.pass.cpp
  test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp
  test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.pass.cpp
  test/std/containers/unord/unord.set/unord.set.cnstr/deduct.pass.cpp
  test/std/numerics/c.math/abs.pass.cpp
  test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp
  test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

Index: test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp
===
--- test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp
+++ test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp
@@ -42,7 +42,7 @@
 ASSERT_SAME_TYPE(Expected, typename std::underlying_type::type);
 #if TEST_STD_VER > 11
 ASSERT_SAME_TYPE(Expected, typename std::underlying_type_t);
-#endif  
+#endif
 }
 
 enum E { V = INT_MIN };
@@ -79,7 +79,9 @@
 //  SFINAE-able underlying_type
 #if TEST_STD_VER > 17
 static_assert( has_type_member::value, "");
+#ifdef TEST_UNSIGNED_UNDERLYING_TYPE
 static_assert( has_type_member::value, "");
+#endif // TEST_UNSIGNED_UNDERLYING_TYPE
 static_assert( has_type_member::value, "");
 
 static_assert(!has_type_member::value, "");
Index: test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp
===
--- test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp
+++ test/std/numerics/numeric.ops/numeric.ops.midpoint/midpoint.float.pass.cpp
@@ -41,7 +41,7 @@
 
 constexpr T maxV = std::numeric_limits::max();
 constexpr T minV = std::numeric_limits::min();
-
+
 //  Things that can be compared exactly
 static_assert((std::midpoint(T(0), T(0))   == T(0)),   "");
 static_assert((std::midpoint(T(2), T(4))   == T(3)),   "");
@@ -58,7 +58,7 @@
 assert((fptest_close_pct(std::midpoint(T(0.1),  T(0.4)),  T(0.25), pct)));
 
 assert((fptest_close_pct(std::midpoint(T(11.2345), T(14.5432)), T(12.5),  pct)));
-
+
 //  From e to pi
 assert((fptest_close_pct(std::midpoint(T(2.71828182845904523536028747135266249775724709369995),
   T(3.14159265358979323846264338327950288419716939937510)),
@@ -86,7 +86,7 @@
 //  TODO
 
 //  Check two values "close to each other"
-T d1 = 3.14;
+T d1 = T(3.14);
 T d0 = std::nextafter(d1, T(2));
 T d2 = std::nextafter(d1, T(5));
 assert(d0 < d1);  // sanity checking
Index: test/std/numerics/c.math/abs.pass.cpp
===
--- test/std/numerics/c.math/abs.pass.cpp
+++ test/std/numerics/c.math/abs.pass.cpp
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "test_macros.h"
@@ -75,4 +76,3 @@
 
 return 0;
 }
-
Index: test/std/containers/unord/unord.set/unord.set.cnstr/deduct.pass.cpp
===
--- test/std/containers/unord/unord.set/unord.set.cnstr/deduct.pass.cpp
+++ test/std/containers/unord/unord.set/unord.set.cnstr/deduct.pass.cpp
@@ -56,6 +56,11 @@
 
 #include "test_allocator.h"
 
+#ifdef TEST_COMPILER_C1XX
+#pragma warning(disable: 4242) // '%s': conversion from '%s' to '%s', possible loss of data
+#pragma warning(disable: 4244) // '%s': conversion from '%s' to '%s', possible loss of data
+#endif // TEST_COMPILER_C1XX
+
 int main(int, char**)
 {
 const int expected_s[] = {1, 2, 3, INT_MAX};
Index: test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.pass.cpp
===
--- test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.pass.cpp
+++ test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.pass.cpp
@@ -56,6 +56,11 @@
 
 #include "test_allocator.h"
 
+#ifdef 

[PATCH] D68362: [libunwind][RISCV] Add 64-bit RISC-V support

2019-10-08 Thread Mitchell Horne via Phabricator via cfe-commits
mhorne marked 5 inline comments as done.
mhorne added a comment.

Thanks for the review!




Comment at: libunwind/src/Registers.hpp:3585
+
+inline uint64_t Registers_riscv::getRegister(int regNum) const {
+  if (regNum == UNW_REG_IP)

lenary wrote:
> Do you want to include an override in this function for `regNum == 
> UNW_RISCV_X0` to always return zero?
> 
> The reason I ask is because I worry that 
> `Registers_riscv::Registers_riscv(const void *registers)` could initialise a 
> non-zero value into `_registers.__x[0]`, which could lead to really confusing 
> bugs.
> 
> It might also make sense to include `assert(validRegister(regNum));` in both 
> this function and `setRegister` as you've done with the float registers.
The assert is not necessary, since `_LIBUNWIND_ABORT` is called in the case of 
an invalid register number.



Comment at: libunwind/src/UnwindCursor.hpp:999
+#if defined (_LIBUNWIND_TARGET_RISCV)
+  int stepWithCompactEncoding(Registers_riscv &) {
+return UNW_EINVAL;

lenary wrote:
> Nit: This should be formatted like the version for `Registers_sparc` above.
`Registers_sparc` is the outlier in this case, I've formatted it as all the 
other architectures use. If you'd like I could fix the SPARC formatting in this 
patch.


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

https://reviews.llvm.org/D68362



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


[PATCH] D61365: [libcxx] [test] Suppress float->int narrowing warning in vector range-construction test.

2019-10-08 Thread Casey Carter via Phabricator via cfe-commits
CaseyCarter added inline comments.



Comment at: 
test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:159
 float array[3] = {0.0f, 1.0f, 2.0f};
+#pragma warning(suppress: 4244) // narrowing float to int
 std::vector v(array, array + 3);

CaseyCarter wrote:
> BillyONeal wrote:
> > CaseyCarter wrote:
> > > This will blow up non-MSVC-ish when running the test suite with `-Wall -W 
> > > -Werror` (which is typical). I suggest wrapping in `#ifdef _MSC_VER`.
> > Why didn't it blow up on Contest then? clang-cl is happy with it?
> clang-cl is the "ish" in my "MSVC-ish" (MSVC and compilers emulating MSVC). 
> GCC and clang-without-`-fms-compatibility` when compiling with `-Wall` warn 
> about unrecognized pragmas, just as does cl in default mode 
> (https://godbolt.org/z/Chue0L).
Correction: Wrap with `#ifdef TEST_COMPILER_C1XX` instead of `#ifdef _MSC_VER`.


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

https://reviews.llvm.org/D61365



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


[PATCH] D68362: [libunwind][RISCV] Add 64-bit RISC-V support

2019-10-08 Thread Mitchell Horne via Phabricator via cfe-commits
mhorne updated this revision to Diff 223971.
mhorne added a comment.

Address lenary's comments.

Add a check for __riscv_float_abi_double in getFloat/setFloat.


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

https://reviews.llvm.org/D68362

Files:
  libunwind/include/__libunwind_config.h
  libunwind/include/libunwind.h
  libunwind/src/Registers.hpp
  libunwind/src/UnwindCursor.hpp
  libunwind/src/UnwindRegistersRestore.S
  libunwind/src/UnwindRegistersSave.S
  libunwind/src/config.h
  libunwind/src/libunwind.cpp

Index: libunwind/src/libunwind.cpp
===
--- libunwind/src/libunwind.cpp
+++ libunwind/src/libunwind.cpp
@@ -58,6 +58,8 @@
 # warning The MIPS architecture is not supported with this ABI and environment!
 #elif defined(__sparc__)
 # define REGISTER_KIND Registers_sparc
+#elif defined(__riscv) && __riscv_xlen == 64
+# define REGISTER_KIND Registers_riscv
 #else
 # error Architecture not supported
 #endif
Index: libunwind/src/config.h
===
--- libunwind/src/config.h
+++ libunwind/src/config.h
@@ -103,7 +103,8 @@
 defined(__ppc__) || defined(__ppc64__) || defined(__powerpc64__) ||\
 (!defined(__APPLE__) && defined(__arm__)) ||   \
 (defined(__arm64__) || defined(__aarch64__)) ||\
-defined(__mips__)
+defined(__mips__) ||   \
+defined(__riscv)
 #if !defined(_LIBUNWIND_BUILD_SJLJ_APIS)
 #define _LIBUNWIND_BUILD_ZERO_COST_APIS
 #endif
Index: libunwind/src/UnwindRegistersSave.S
===
--- libunwind/src/UnwindRegistersSave.S
+++ libunwind/src/UnwindRegistersSave.S
@@ -974,6 +974,87 @@
   std %i6, [%o0 + 120]
   jmp %o7
clr %o0   // return UNW_ESUCCESS
+
+#elif defined(__riscv) && __riscv_xlen == 64
+
+#
+# extern int __unw_getcontext(unw_context_t* thread_state)
+#
+# On entry:
+#  thread_state pointer is in a0
+#
+DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext)
+  // x0 is zero
+  sdx1, (8 * 1)(a0)
+  sdx2, (8 * 2)(a0)
+  sdx3, (8 * 3)(a0)
+  sdx4, (8 * 4)(a0)
+  sdx5, (8 * 5)(a0)
+  sdx6, (8 * 6)(a0)
+  sdx7, (8 * 7)(a0)
+  sdx8, (8 * 8)(a0)
+  sdx9, (8 * 9)(a0)
+  sdx10, (8 * 10)(a0)
+  sdx11, (8 * 11)(a0)
+  sdx12, (8 * 12)(a0)
+  sdx13, (8 * 13)(a0)
+  sdx14, (8 * 14)(a0)
+  sdx15, (8 * 15)(a0)
+  sdx16, (8 * 16)(a0)
+  sdx17, (8 * 17)(a0)
+  sdx18, (8 * 18)(a0)
+  sdx19, (8 * 19)(a0)
+  sdx20, (8 * 20)(a0)
+  sdx21, (8 * 21)(a0)
+  sdx22, (8 * 22)(a0)
+  sdx23, (8 * 23)(a0)
+  sdx24, (8 * 24)(a0)
+  sdx25, (8 * 25)(a0)
+  sdx26, (8 * 26)(a0)
+  sdx27, (8 * 27)(a0)
+  sdx28, (8 * 28)(a0)
+  sdx29, (8 * 29)(a0)
+  sdx30, (8 * 30)(a0)
+  sdx31, (8 * 31)(a0)
+
+#ifdef __riscv_float_abi_double
+  fsdf0, (8 * 32 + 8 * 0)(a0)
+  fsdf1, (8 * 32 + 8 * 1)(a0)
+  fsdf2, (8 * 32 + 8 * 2)(a0)
+  fsdf3, (8 * 32 + 8 * 3)(a0)
+  fsdf4, (8 * 32 + 8 * 4)(a0)
+  fsdf5, (8 * 32 + 8 * 5)(a0)
+  fsdf6, (8 * 32 + 8 * 6)(a0)
+  fsdf7, (8 * 32 + 8 * 7)(a0)
+  fsdf8, (8 * 32 + 8 * 8)(a0)
+  fsdf9, (8 * 32 + 8 * 9)(a0)
+  fsdf10, (8 * 32 + 8 * 10)(a0)
+  fsdf11, (8 * 32 + 8 * 11)(a0)
+  fsdf12, (8 * 32 + 8 * 12)(a0)
+  fsdf13, (8 * 32 + 8 * 13)(a0)
+  fsdf14, (8 * 32 + 8 * 14)(a0)
+  fsdf15, (8 * 32 + 8 * 15)(a0)
+  fsdf16, (8 * 32 + 8 * 16)(a0)
+  fsdf17, (8 * 32 + 8 * 17)(a0)
+  fsdf18, (8 * 32 + 8 * 18)(a0)
+  fsdf19, (8 * 32 + 8 * 19)(a0)
+  fsdf20, (8 * 32 + 8 * 20)(a0)
+  fsdf21, (8 * 32 + 8 * 21)(a0)
+  fsdf22, (8 * 32 + 8 * 22)(a0)
+  fsdf23, (8 * 32 + 8 * 23)(a0)
+  fsdf24, (8 * 32 + 8 * 24)(a0)
+  fsdf25, (8 * 32 + 8 * 25)(a0)
+  fsdf26, (8 * 32 + 8 * 26)(a0)
+  fsdf27, (8 * 32 + 8 * 27)(a0)
+  fsdf28, (8 * 32 + 8 * 28)(a0)
+  fsdf29, (8 * 32 + 8 * 29)(a0)
+  fsdf30, (8 * 32 + 8 * 30)(a0)
+  fsdf31, (8 * 32 + 8 * 31)(a0)
+
+#endif
+
+  li a0, 0  // return UNW_ESUCCESS
+  ret   // jump to ra
 #endif
 
   WEAK_ALIAS(__unw_getcontext, unw_getcontext)
Index: libunwind/src/UnwindRegistersRestore.S
===
--- libunwind/src/UnwindRegistersRestore.S
+++ libunwind/src/UnwindRegistersRestore.S
@@ -1029,6 +1029,87 @@
   jmp %o7
nop
 
+#elif defined(__riscv) && __riscv_xlen == 64
+
+//
+// void libunwind::Registers_riscv::jumpto()
+//
+// On entry:
+//  thread_state pointer is in a0
+//
+  .p2align 2
+DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind15Registers_riscv6jumptoEv)
+#ifdef __riscv_float_abi_double
+  fldf0, (8 * 32 + 8 * 0)(a0)
+  fldf1, (8 * 32 + 8 * 1)(a0)
+  fldf2, (8 * 32 + 8 * 2)(a0)
+  fldf3, (8 * 32 + 8 * 3)(a0)
+  fld

r374133 - [c++20] Implement most of P1152R4.

2019-10-08 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Tue Oct  8 17:49:40 2019
New Revision: 374133

URL: http://llvm.org/viewvc/llvm-project?rev=374133=rev
Log:
[c++20] Implement most of P1152R4.

Diagnose some now-deprecated uses of volatile types:
 * as function parameter types and return types
 * as the type of a structured binding declaration
 * as the type of the lvalue operand of an increment / decrement /
   compound assignment operator

This does not implement a check for the deprecation of simple
assignments whose results are used; that check requires somewhat
more complexity and will be addressed separately.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaCXX/deprecated.cpp
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=374133=374132=374133=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Oct  8 17:49:40 2019
@@ -140,6 +140,7 @@ def DeprecatedImplementations :DiagGroup
 def DeprecatedIncrementBool : DiagGroup<"deprecated-increment-bool">;
 def DeprecatedRegister : DiagGroup<"deprecated-register">;
 def DeprecatedThisCapture : DiagGroup<"deprecated-this-capture">;
+def DeprecatedVolatile : DiagGroup<"deprecated-volatile">;
 def DeprecatedWritableStr : DiagGroup<"deprecated-writable-strings",
   [CXX11CompatDeprecatedWritableStr]>;
 // FIXME: Why is DeprecatedImplementations not in this group?
@@ -150,6 +151,7 @@ def Deprecated : DiagGroup<"deprecated",
   DeprecatedIncrementBool,
   DeprecatedRegister,
   DeprecatedThisCapture,
+  DeprecatedVolatile,
   DeprecatedWritableStr]>,
  DiagCategory<"Deprecations">;
 

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=374133=374132=374133=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct  8 17:49:40 
2019
@@ -6650,6 +6650,23 @@ def ext_increment_bool : ExtWarn<
   DefaultError, InGroup;
 def err_increment_decrement_enum : Error<
   "cannot %select{decrement|increment}0 expression of enum type %1">;
+
+def warn_deprecated_increment_decrement_volatile : Warning<
+  "%select{decrement|increment}0 of object of volatile-qualified type %1 "
+  "is deprecated">, InGroup;
+def warn_deprecated_compound_assign_volatile : Warning<
+  "compound assignment to object of volatile-qualified type %0 is deprecated">,
+  InGroup;
+def warn_deprecated_volatile_return : Warning<
+  "volatile-qualified return type %0 is deprecated">,
+  InGroup;
+def warn_deprecated_volatile_param : Warning<
+  "volatile-qualified parameter type %0 is deprecated">,
+  InGroup;
+def warn_deprecated_volatile_structured_binding : Warning<
+  "volatile qualifier in structured binding declaration is deprecated">,
+  InGroup;
+
 def err_catch_incomplete_ptr : Error<
   "cannot catch pointer to incomplete type %0">;
 def err_catch_incomplete_ref : Error<

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=374133=374132=374133=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Oct  8 17:49:40 2019
@@ -775,6 +775,13 @@ Sema::ActOnDecompositionDeclarator(Scope
   return nullptr;
   }
 
+  // C++2a [dcl.struct.bind]p1:
+  //   A cv that includes volatile is deprecated
+  if ((DS.getTypeQualifiers() & DeclSpec::TQ_volatile) &&
+  getLangOpts().CPlusPlus2a)
+Diag(DS.getVolatileSpecLoc(),
+ diag::warn_deprecated_volatile_structured_binding);
+
   TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
   QualType R = TInfo->getType();
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=374133=374132=374133=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Oct  8 17:49:40 2019
@@ -11938,6 +11938,21 @@ QualType Sema::CheckAssignmentOperands(E
 
   CheckForNullPointerDereference(*this, LHSExpr);
 
+  if 

[PATCH] D68055: Add -fgnuc-version= to control __GNUC__ and other GCC macros

2019-10-08 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

In D68055#1688747 , @programmerjake 
wrote:

> Shouldn't `__GNUG__` match `__GNUC__`?


+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68055



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


[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

2019-10-08 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Looks fine to me with some doc improvements.




Comment at: clang/docs/ReleaseNotes.rst:64-66
+  non-zero offset to ``nullptr`` (or making non-``nullptr`` a ``nullptr``,
+  by subtracting pointer's integral value from the pointer itself; in C, also,
+  applying *any* (even zero) offset to ``nullptr``) is undefined behaviour.

The parenthetical here makes this awkward to read. Also, I don't think we need 
to say which LLVM revision this happened in. How about:

"In both C and C++ (C17 6.5.6p8, C++ [expr.add]), pointer arithmetic is only 
permitted within arrays. In particular, the behavior of a program is not 
defined if it adds a non-zero offset (or in C, any offset) to a null pointer, 
or that forms a null pointer by subtracting an integer from a non-null pointer, 
and the LLVM optimizer now uses those guarantees for transformations. This may 
lead to unintended behavior in code that performs these operations. The 
Undefined Behavior Sanitizer `-fsanitize=pointer-overflow` check has been 
extended to detect these cases, so that code relying on them can be detected 
and fixed."



Comment at: clang/docs/ReleaseNotes.rst:242
 
-- ...
+- * ``pointer-overflow`` check was extended added to catch the cases where
+a non-zero offset being applied, either to a ``nullptr``, or the result

Add "The" to the start of this bullet.



Comment at: clang/docs/ReleaseNotes.rst:243-244
+- * ``pointer-overflow`` check was extended added to catch the cases where
+a non-zero offset being applied, either to a ``nullptr``, or the result
+of applying of the offset is a ``nullptr``.
+As per C++ Standard ``[expr.add]`` that is undefined behaviour.

"[...] where a non-zero offset is applied to a null pointer, or the result of 
applying the offset is a null pointer."



Comment at: clang/docs/ReleaseNotes.rst:245
+of applying of the offset is a ``nullptr``.
+As per C++ Standard ``[expr.add]`` that is undefined behaviour.
+

I don't think we really need to say this is undefined behavior here.



Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:133
   -  ``-fsanitize=pointer-overflow``: Performing pointer arithmetic which
- overflows.
+ overflows; applying a non-zero (in C++; in C - any) offset to either a
+ non-``nullptr``, or pointer becoming ``nullptr`` after applying the 
offset.

Simplify this a bit: "Performing pointer arithmetic which overflows, or where 
either the old or new pointer value is a null pointer (or in C, when they both 
are)."



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4657
+  Builder.GetInsertBlock()->getParent(), PtrTy->getPointerAddressSpace());
+  // Check for overflows unless the GEP got constant-folded,
+  // and only in the default address space

If we want to split out the "constant folded" case to avoid issuing too many 
sanitizer traps on bogus but common patterns, we should have another sanitizer 
group to re-enable those diagnostics for the constant-folded cases. (I'm fine 
with not doing that in this patch, though.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122



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


[PATCH] D68055: Add -fgnuc-version= to control __GNUC__ and other GCC macros

2019-10-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D68055#1698653 , @rsmith wrote:

> From a big-picture perspective, I would like us to move to treating MS and 
> GNU extensions in the same way (at the cc1 level) to the extent that we can. 
> I think this moves us in that direction.


Sounds good.

> One comment, though: I'm not convinced that the `__EXCEPTIONS` macro should 
> be controlled by this flag. Many compilers define this when exceptions are 
> available (ARM, IBM, HP, EDG in all modes -- including MSVC-compatible mode, 
> ...) and it's not obvious to me whether it is in fact a GNU extension or has 
> more history than that. (It's also a somewhat-documented 
> 
>  Clang feature independent of GCC compatibility.)
> 
> Do you know why we turn that macro off under `MSVCCompat`? If EDG can get 
> away with defining it in MSVC-compatible mode, I would expect that we can too.

I doubt there's any good reason for it. I'll remove that change from this patch 
and treat it separately. I think we can simply define it in all modes that 
enable exceptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68055



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


r374131 - [cxx_status] Note that Clang has supported std::source_location since

2019-10-08 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Tue Oct  8 16:39:56 2019
New Revision: 374131

URL: http://llvm.org/viewvc/llvm-project?rev=374131=rev
Log:
[cxx_status] Note that Clang has supported std::source_location since
version 9.

Modified:
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=374131=374130=374131=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Tue Oct  8 16:39:56 2019
@@ -1247,7 +1247,7 @@ and library features that are not part o
   [TS] Library Fundamentals, Version 2 (source_location)
  http://wg21.link/n4617;>N4617
   N/A
-  No
+  Clang 9 (documentation)
 
 
   [TS] Modules


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


[PATCH] D68578: [HIP] Fix device stub name

2019-10-08 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D68578#1698864 , @t-tye wrote:

> From a source language point of view, the device function comprises the code 
> that is launched as a grid. We need this fact to be present in the symbols 
> used. Only the device function should have a symbol name matching the mangled 
> name of the device function.


What do you have in mind when you use 'symbol name' here? Is that a symbol as 
seen by linker? If that's the case, do host and device share this name space on 
AMD GPUs? In case of CUDA, linker symbols are per-target (i.e. host and each 
GPU have their own spaces), so they never clash, but the kernel names must have 
identical mangled name on host and all devices, so the host can refer to the 
device-side kernel when it needs to launch it.

> It the device function has both a host and device implementation then both 
> can have the source language function name for the symbol since both actually 
> implement the device function. If the user asks to set a breakpoint in the 
> device function then the debugger would set in both implementations so the 
> user is notified when the source program executes the device function, 
> regardless of which implementation is invoked. This is similar to the 
> debugger setting a breakpoint in a function that is inlined into multiple 
> places: the debugger sets breeakpoints in all the inlined places so the user 
> can tstill think of the program debugging in terms of the source language 
> semantics.

OK. This sounds like `__host__`/`__device__` function overloads and what you're 
saying does make sense for that.

> In contrast, the stub is effectively part of the implementation of actually 
> launching the device function. It should have a distinct name.

I'm not sure how the requirement of distinct name follows from the fact that 
the stub is the host-side part of the device-side kernel? To me it looks like 
an argument for them to have the same name so it's clear that they are both 
part of the same function as written in the source.

The don't have to be different. CUDA (and HIP) does not allow overloading of 
kernels, so the stub and the kernel can have identical names as in the example 
of `__host__` and `__device__` overloads you've described above, only now it's 
`__host__` stub + `__global__` kernel itself, instead of two user-implemented 
functions. Debugger, of course, will need to know about that to pick the stub 
or kernel as the breakpoint location, but that appears doable.

> The debugger can still be used to set a breakpoint in it, or to step into it. 
> But that should be done in terms of the stub name. If the debugger wants to 
> support source language specific intelligence it can provide a helper library 
> that understands the stub names. This helper library (similar to the thread 
> helper library) can be used by the debugger to present a cleaner language 
> view to the user. In fact OpenMP has also done this and provides a helper 
> library called OMPD that can be used by tools such as a debugger to hide 
> OpenMP trampoline functions etc.

Do I understand it correctly that giving the stub distinct name would 
effectively get it out of the way when a breakpoint is set on the kernel? I.e. 
it's essentially a work around the fact that debugger may not have convenient 
way to specify "set breakpoint on this name in device code only". Perhaps it 
would make sense to prove this ability as it sounds quite useful. I.e I may 
want to set breakpoint on all inlined host/device functions, but only on device 
side. That would be handy.

What happens if the stub and the kernel do have identical names?
My understanding, based on your comments above is that debugger does know about 
host and device 'spaces' and that it can find pointers to both host and device 
functions and set appropriate breakpoints for both. In this case it would 
normally attempt to set breakpoint on both the stub and the kernel as it would 
in case of `__host__`/`__device__` overloads you've described above. In case of 
stub/kernel, we would want the breakpoint set only on the kernel itself. Given 
that debugger does have ability to tell host and device functions/symbols 
apart, the difficulty is really in being able to tell a real host function from 
the stub, so we can skip it.

Is that indeed what we want/need? Is there something else?

Does debugger know that device-side function is a kernel? In case of CUDA, the 
kernels are distinct from regular device-side functions. I don't know whether 
that's the case for AMDGPU.
If debugger can tell that particular device function is a kernel, that can be 
used to infer that the matching host-side symbol is a stub and skip setting a 
breakpoint on it.
If that does not work, debugger presumably has access to the mangled symbols 
for the potential breakpoint locations. The stub currently has distinct `.stub` 
suffix. This can also be used to tell it apart from a regular `__host__` 

r374130 - Factor out some duplication. NFC.

2019-10-08 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Tue Oct  8 16:37:49 2019
New Revision: 374130

URL: http://llvm.org/viewvc/llvm-project?rev=374130=rev
Log:
Factor out some duplication. NFC.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=374130=374129=374130=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Oct  8 16:37:49 2019
@@ -137,7 +137,7 @@ namespace {
 
   /// Given an expression, determine the type used to store the result of
   /// evaluating that expression.
-  static QualType getStorageType(ASTContext , Expr *E) {
+  static QualType getStorageType(const ASTContext , const Expr *E) {
 if (E->isRValue())
   return E->getType();
 return Ctx.getLValueReferenceType(E->getType());
@@ -13569,10 +13569,8 @@ bool Expr::EvaluateAsConstantExpr(EvalRe
   if (!Info.discardCleanups())
 llvm_unreachable("Unhandled cleanup; missing full expression marker?");
 
-  QualType T = getType();
-  if (!isRValue())
-T = Ctx.getLValueReferenceType(T);
-  return CheckConstantExpression(Info, getExprLoc(), T, Result.Val, Usage) &&
+  return CheckConstantExpression(Info, getExprLoc(), getStorageType(Ctx, this),
+ Result.Val, Usage) &&
  CheckMemoryLeaks(Info);
 }
 


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


[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

2019-10-08 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX marked 2 inline comments as done.
SouraVX added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp:25
+
+  //FIXME -- clang will not mark above member funtions, excluding constructors
+  // as out of class. If we did not mark destructor or other member functions

This is the case, checking for Out of class definition. I've been mentioning in 
llvm-dev mails.



Comment at: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp:9
+
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted

SouraVX wrote:
> This test case is failing, checking DISPFlagNotDefaulted.
Please note here that, backend and llvm-dwarfdump is fine without this. 
Since it's value is '0' , we are able to query this using isNotDefaulted() -- 
hence attribute 
DW_AT_defaulted having value DW_DEFAULTED_no is getting set and emitted and 
dumped fine by llvm-dwarfdump.


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

https://reviews.llvm.org/D68117



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


[PATCH] D68634: [ASTImporter] Imported FunctionDecls inherit attributes from existing functions

2019-10-08 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment.

Ouch! Sorry, Gabor -_\\


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68634



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


[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

2019-10-08 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX marked 3 inline comments as done.
SouraVX added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1619
+  else {
+SPFlags |= llvm::DISubprogram::SPFlagNotDefaulted;
+  }

Previously SPFlagNotDefaulted is setted to SPFlagZero as it's normal value is, 
to save a bit. Hence in generated IR this flag is not getting set. instead 0 is 
getting emitted.
As a result, test cases checking DISPFlagNotDefaulted in IR are failing.



Comment at: clang/test/CodeGenCXX/debug-info-not-defaulted.cpp:9
+
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted
+// ATTR: DISubprogram(name: "not_defaulted", {{.*}}, flags: DIFlagPublic | 
DIFlagPrototyped, spFlags: DISPFlagNotDefaulted

This test case is failing, checking DISPFlagNotDefaulted.


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

https://reviews.llvm.org/D68117



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


[PATCH] D68436: [clang-scan-deps] Improve string/character literal skipping

2019-10-08 Thread Alex Lorenz via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
arphaman marked 5 inline comments as done.
Closed by commit rGa13f0da1d0bc: [clang-scan-deps] Improve string/character 
literal skipping (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D68436?vs=223125=223955#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68436

Files:
  clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -594,6 +594,50 @@
   EXPECT_STREQ("#pragma once\n#include \n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest,
+ SkipLineStringCharLiteralsUntilNewline) {
+  SmallVector Out;
+
+  StringRef Source = R"(#if NEVER_ENABLED
+#define why(fmt, ...) #error don't try me
+#endif
+
+void foo();
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ(
+  "#if NEVER_ENABLED\n#define why(fmt,...) #error don't try me\n#endif\n",
+  Out.data());
+
+  Source = R"(#if NEVER_ENABLED
+  #define why(fmt, ...) "quote dropped
+  #endif
+
+  void foo();
+  )";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ(
+  "#if NEVER_ENABLED\n#define why(fmt,...) \"quote dropped\n#endif\n",
+  Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest,
+ SupportWhitespaceBeforeLineContinuationInStringSkipping) {
+  SmallVector Out;
+
+  StringRef Source = "#define X '\\ \t\nx'\nvoid foo() {}";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define X '\\ \t\nx'\n", Out.data());
+
+  Source = "#define X \"\\ \r\nx\"\nvoid foo() {}";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define X \"\\ \r\nx\"\n", Out.data());
+
+  Source = "#define X \"\\ \r\nx\n#include \n";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define X \"\\ \r\nx\n#include \n", Out.data());
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, CxxModules) {
   SmallVector Out;
   SmallVector Tokens;
Index: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -185,17 +185,6 @@
   }
 }
 
-static void skipString(const char *, const char *const End) {
-  assert(*First == '\'' || *First == '"' || *First == '<');
-  const char Terminator = *First == '<' ? '>' : *First;
-  for (++First; First != End && *First != Terminator; ++First)
-if (*First == '\\')
-  if (++First == End)
-return;
-  if (First != End)
-++First; // Finish off the string.
-}
-
 // Returns the length of EOL, either 0 (no end-of-line), 1 (\n) or 2 (\r\n)
 static unsigned isEOL(const char *First, const char *const End) {
   if (First == End)
@@ -206,6 +195,35 @@
   return !!isVerticalWhitespace(First[0]);
 }
 
+static void skipString(const char *, const char *const End) {
+  assert(*First == '\'' || *First == '"' || *First == '<');
+  const char Terminator = *First == '<' ? '>' : *First;
+  for (++First; First != End && *First != Terminator; ++First) {
+// String and character literals don't extend past the end of the line.
+if (isVerticalWhitespace(*First))
+  return;
+if (*First != '\\')
+  continue;
+// Skip past backslash to the next character. This ensures that the
+// character right after it is skipped as well, which matters if it's
+// the terminator.
+if (++First == End)
+  return;
+if (!isWhitespace(*First))
+  continue;
+// Whitespace after the backslash might indicate a line continuation.
+const char *FirstAfterBackslashPastSpace = First;
+skipOverSpaces(FirstAfterBackslashPastSpace, End);
+if (unsigned NLSize = isEOL(FirstAfterBackslashPastSpace, End)) {
+  // Advance the character pointer to the next line for the next
+  // iteration.
+  First = FirstAfterBackslashPastSpace + NLSize - 1;
+}
+  }
+  if (First != End)
+++First; // Finish off the string.
+}
+
 // Returns the length of the skipped newline
 static unsigned skipNewline(const char *, const char *End) {
   if (First == End)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

2019-10-08 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX updated this revision to Diff 223954.
SouraVX added a comment.

Addressed comments, regarding flags.


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

https://reviews.llvm.org/D68117

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-in-class.cpp
  clang/test/CodeGenCXX/debug-info-defaulted-out-of-class.cpp
  clang/test/CodeGenCXX/debug-info-deleted.cpp
  clang/test/CodeGenCXX/debug-info-not-defaulted.cpp
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/BinaryFormat/Dwarf.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DebugInfoMetadata.cpp

Index: llvm/lib/IR/DebugInfoMetadata.cpp
===
--- llvm/lib/IR/DebugInfoMetadata.cpp
+++ llvm/lib/IR/DebugInfoMetadata.cpp
@@ -600,6 +600,7 @@
   switch (Flag) {
   // Appease a warning.
   case SPFlagVirtuality:
+  case SPFlagDefaultedInOrOutOfClass:
 return "";
 #define HANDLE_DISP_FLAG(ID, NAME) \
   case SPFlag##NAME:   \
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1296,6 +1296,19 @@
 addFlag(SPDie, dwarf::DW_AT_elemental);
   if (SP->isRecursive())
 addFlag(SPDie, dwarf::DW_AT_recursive);
+  if (DD->getDwarfVersion() >= 5) {
+if (SP->isDefaultedInClass())
+  addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+  dwarf::DW_DEFAULTED_in_class);
+else if (SP->isDefaultedOutOfClass())
+  addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+  dwarf::DW_DEFAULTED_out_of_class);
+else if (SP->isNotDefaulted())
+  addUInt(SPDie, dwarf::DW_AT_defaulted, dwarf::DW_FORM_data1,
+  dwarf::DW_DEFAULTED_no);
+if (SP->isDeleted())
+  addFlag(SPDie, dwarf::DW_AT_deleted);
+  }
 }
 
 void DwarfUnit::constructSubrangeDIE(DIE , const DISubrange *SR,
Index: llvm/lib/BinaryFormat/Dwarf.cpp
===
--- llvm/lib/BinaryFormat/Dwarf.cpp
+++ llvm/lib/BinaryFormat/Dwarf.cpp
@@ -271,6 +271,19 @@
   return StringRef();
 }
 
+StringRef llvm::dwarf::DefaultedMemberString(unsigned DefaultedEncodings) {
+  switch (DefaultedEncodings) {
+  // Defaulted Member Encodings codes
+  case DW_DEFAULTED_no:
+return "DW_DEFAULTED_no";
+  case DW_DEFAULTED_in_class:
+return "DW_DEFAULTED_in_class";
+  case DW_DEFAULTED_out_of_class:
+return "DW_DEFAULTED_out_of_class";
+  }
+  return StringRef();
+}
+
 StringRef llvm::dwarf::VisibilityString(unsigned Visibility) {
   switch (Visibility) {
   case DW_VIS_local:
@@ -612,6 +625,8 @@
 return ArrayOrderString(Val);
   case DW_AT_APPLE_runtime_class:
 return LanguageString(Val);
+  case DW_AT_defaulted:
+return DefaultedMemberString(Val);
   }
 
   return StringRef();
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1610,7 +1610,10 @@
 #define DISP_FLAG_LARGEST_NEEDED
 #include "llvm/IR/DebugInfoFlags.def"
 SPFlagNonvirtual = SPFlagZero,
+SPFlagNotDefaulted = SPFlagZero,
 SPFlagVirtuality = SPFlagVirtual | SPFlagPureVirtual,
+SPFlagDefaultedInOrOutOfClass =
+SPFlagDefaultedInClass | SPFlagDefaultedOutOfClass,
 LLVM_MARK_AS_BITMASK_ENUM(SPFlagLargest)
   };
 
@@ -1625,10 +1628,11 @@
   SmallVectorImpl );
 
   // Helper for converting old bitfields to new flags word.
-  static DISPFlags toSPFlags(bool IsLocalToUnit, bool IsDefinition,
- bool IsOptimized,
- unsigned Virtuality = SPFlagNonvirtual,
- bool IsMainSubprogram = false) {
+  static DISPFlags
+  toSPFlags(bool IsLocalToUnit, bool IsDefinition, bool IsOptimized,
+unsigned Virtuality = SPFlagNonvirtual,
+unsigned DefaultedInOrOutOfClass = SPFlagNotDefaulted,
+bool IsMainSubprogram = false) {
 // We're assuming virtuality is the low-order field.
 static_assert(
 int(SPFlagVirtual) == int(dwarf::DW_VIRTUALITY_virtual) &&
@@ -1636,6 +1640,7 @@
 "Virtuality constant mismatch");
 return static_cast(
 (Virtuality & SPFlagVirtuality) |
+(DefaultedInOrOutOfClass & SPFlagDefaultedInOrOutOfClass) |
 (IsLocalToUnit ? SPFlagLocalToUnit : SPFlagZero) |
 (IsDefinition ? SPFlagDefinition : SPFlagZero) |
 (IsOptimized ? SPFlagOptimized : SPFlagZero) |
@@ -1758,6 +1763,18 @@
   bool isPure() const { return 

[PATCH] D68634: [ASTImporter] Imported FunctionDecls inherit attributes from existing functions

2019-10-08 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment.

Hello Balasz,
In my opinion, importing and setting the attributes should be handled by the 
stuff used in InitializeImportedDecl(). Can we extend it or reuse the code? It 
will allow us not to miss the required actions while importing a new function 
too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68634



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


r374127 - [clang-scan-deps] Improve string/character literal skipping

2019-10-08 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Tue Oct  8 15:42:44 2019
New Revision: 374127

URL: http://llvm.org/viewvc/llvm-project?rev=374127=rev
Log:
[clang-scan-deps] Improve string/character literal skipping

The existing string/character literal skipping code in the
dependency directives source minimizer has two issues:

- It doesn't stop the scanning when a newline is reached before the terminating 
character,
unlike the lexer which considers the token to be done (even if it's invalid) at 
the end of the line.

- It doesn't support whitespace between '\' and the newline when looking if the 
'\' is used as a line continuation character.

This commit fixes both issues.

Differential Revision: https://reviews.llvm.org/D68436

Modified:
cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Modified: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp?rev=374127=374126=374127=diff
==
--- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp (original)
+++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp Tue Oct  8 
15:42:44 2019
@@ -185,17 +185,6 @@ static void skipRawString(const char *
   }
 }
 
-static void skipString(const char *, const char *const End) {
-  assert(*First == '\'' || *First == '"' || *First == '<');
-  const char Terminator = *First == '<' ? '>' : *First;
-  for (++First; First != End && *First != Terminator; ++First)
-if (*First == '\\')
-  if (++First == End)
-return;
-  if (First != End)
-++First; // Finish off the string.
-}
-
 // Returns the length of EOL, either 0 (no end-of-line), 1 (\n) or 2 (\r\n)
 static unsigned isEOL(const char *First, const char *const End) {
   if (First == End)
@@ -206,6 +195,35 @@ static unsigned isEOL(const char *First,
   return !!isVerticalWhitespace(First[0]);
 }
 
+static void skipString(const char *, const char *const End) {
+  assert(*First == '\'' || *First == '"' || *First == '<');
+  const char Terminator = *First == '<' ? '>' : *First;
+  for (++First; First != End && *First != Terminator; ++First) {
+// String and character literals don't extend past the end of the line.
+if (isVerticalWhitespace(*First))
+  return;
+if (*First != '\\')
+  continue;
+// Skip past backslash to the next character. This ensures that the
+// character right after it is skipped as well, which matters if it's
+// the terminator.
+if (++First == End)
+  return;
+if (!isWhitespace(*First))
+  continue;
+// Whitespace after the backslash might indicate a line continuation.
+const char *FirstAfterBackslashPastSpace = First;
+skipOverSpaces(FirstAfterBackslashPastSpace, End);
+if (unsigned NLSize = isEOL(FirstAfterBackslashPastSpace, End)) {
+  // Advance the character pointer to the next line for the next
+  // iteration.
+  First = FirstAfterBackslashPastSpace + NLSize - 1;
+}
+  }
+  if (First != End)
+++First; // Finish off the string.
+}
+
 // Returns the length of the skipped newline
 static unsigned skipNewline(const char *, const char *End) {
   if (First == End)

Modified: cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp?rev=374127=374126=374127=diff
==
--- cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp 
(original)
+++ cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp Tue Oct 
 8 15:42:44 2019
@@ -594,6 +594,50 @@ TEST(MinimizeSourceToDependencyDirective
   EXPECT_STREQ("#pragma once\n#include \n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest,
+ SkipLineStringCharLiteralsUntilNewline) {
+  SmallVector Out;
+
+  StringRef Source = R"(#if NEVER_ENABLED
+#define why(fmt, ...) #error don't try me
+#endif
+
+void foo();
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ(
+  "#if NEVER_ENABLED\n#define why(fmt,...) #error don't try me\n#endif\n",
+  Out.data());
+
+  Source = R"(#if NEVER_ENABLED
+  #define why(fmt, ...) "quote dropped
+  #endif
+
+  void foo();
+  )";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ(
+  "#if NEVER_ENABLED\n#define why(fmt,...) \"quote dropped\n#endif\n",
+  Out.data());
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest,
+ SupportWhitespaceBeforeLineContinuationInStringSkipping) {
+  SmallVector Out;
+
+  StringRef Source = "#define X '\\ \t\nx'\nvoid foo() {}";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define X '\\ \t\nx'\n", Out.data());
+
+  Source = 

[PATCH] D68665: [HIP] Fix -save-temps

2019-10-08 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Driver/Driver.cpp:4400
+  // HIP image for device compilation with -fno-gpu-rdc is per compilation
+  // unit.
+  bool IsHIPNoRDC = JA.getOffloadingDeviceKind() == Action::OFK_HIP &&

... so we derive the file name from the main file name.



Comment at: lib/Driver/Driver.cpp:4405
+  if (IsHIPNoRDC)
+Output = BaseName.substr(0, BaseName.rfind('.'));
   Output += OffloadingPrefix;

`llvm::sys::path::replace_extension(Output, "");` ?



Comment at: lib/Driver/ToolChains/HIP.cpp:253
+constructLlcCommand(C, JA, Inputs, Args, SubArchName, Prefix, OptCommand,
+true);
   const char *LlcCommand =

`/*OutputIsAsm=*/true`



Comment at: lib/Driver/ToolChains/HIP.h:62
+  const char *InputFileName,
+  bool IsAsm = false) const;
 

`OutputIsAsm`?  Otherwise it's not quite clear what `IsAsm` refers to.


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

https://reviews.llvm.org/D68665



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


[PATCH] D68611: [IRGen] Emit lifetime markers for temporary struct allocas

2019-10-08 Thread Francis Visoiu Mistrih via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
thegameg marked an inline comment as done.
Closed by commit rG143f6b837790: [IRGen] Emit lifetime markers for temporary 
struct allocas (authored by thegameg).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68611

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/aarch64-byval-temp.c

Index: clang/test/CodeGen/aarch64-byval-temp.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-byval-temp.c
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -emit-llvm -triple arm64-- -o - %s -O0 | FileCheck %s --check-prefix=CHECK-O0
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-optzns -triple arm64-- -o - %s -O3 | FileCheck %s --check-prefix=CHECK-O3
+
+struct large {
+void* pointers[8];
+};
+
+void pass_large(struct large);
+
+// For arm64, we don't use byval to pass structs but instead we create
+// temporary allocas.
+//
+// Make sure we generate the appropriate lifetime markers for the temporary
+// allocas so that the optimizer can re-use stack slots if possible.
+void example() {
+struct large l = {0};
+pass_large(l);
+pass_large(l);
+}
+// CHECK-O0-LABEL: define void @example(
+// The alloca for the struct on the stack.
+// CHECK-O0: %[[l:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// The alloca for the temporary stack space that we use to pass the argument.
+// CHECK-O0-NEXT: %[[byvaltemp:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// Another one to pass the argument to the second function call.
+// CHECK-O0-NEXT: %[[byvaltemp1:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// First, memset `l` to 0.
+// CHECK-O0-NEXT: %[[bitcastl:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O0-NEXT: call void @llvm.memset.p0i8.i64(i8* align 8 %[[bitcastl]], i8 0, i64 64, i1 false)
+// Then, memcpy `l` to the temporary stack space.
+// CHECK-O0-NEXT: %[[src:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp]] to i8*
+// CHECK-O0-NEXT: %[[dst:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O0-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %[[src]], i8* align 8 %[[dst]], i64 64, i1 false)
+// Finally, call using a pointer to the temporary stack space.
+// CHECK-O0-NEXT: call void @pass_large(%struct.large* %[[byvaltemp]])
+// Now, do the same for the second call, using the second temporary alloca.
+// CHECK-O0-NEXT: %[[src:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp1]] to i8*
+// CHECK-O0-NEXT: %[[dst:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O0-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %[[src]], i8* align 8 %[[dst]], i64 64, i1 false)
+// CHECK-O0-NEXT: call void @pass_large(%struct.large* %[[byvaltemp1]])
+// CHECK-O0-NEXT: ret void
+//
+// At O3, we should have lifetime markers to help the optimizer re-use the temporary allocas.
+//
+// CHECK-O3-LABEL: define void @example(
+// The alloca for the struct on the stack.
+// CHECK-O3: %[[l:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// The alloca for the temporary stack space that we use to pass the argument.
+// CHECK-O3-NEXT: %[[byvaltemp:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// Another one to pass the argument to the second function call.
+// CHECK-O3-NEXT: %[[byvaltemp1:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+//
+// Mark the start of the lifetime for `l`
+// CHECK-O3-NEXT: %[[bitcastl:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O3-NEXT: call void @llvm.lifetime.start.p0i8(i64 64, i8* %[[bitcastl]])
+//
+// First, memset `l` to 0.
+// CHECK-O3-NEXT: %[[bitcastl:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O3-NEXT: call void @llvm.memset.p0i8.i64(i8* align 8 %[[bitcastl]], i8 0, i64 64, i1 false)
+//
+// Lifetime of the first temporary starts here and ends right after the call.
+// CHECK-O3-NEXT: %[[bitcastbyvaltemp:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp]] to i8*
+// CHECK-O3-NEXT: call void @llvm.lifetime.start.p0i8(i64 64, i8* %[[bitcastbyvaltemp]])
+//
+// Then, memcpy `l` to the temporary stack space.
+// CHECK-O3-NEXT: %[[src:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp]] to i8*
+// CHECK-O3-NEXT: %[[dst:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O3-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %[[src]], i8* align 8 %[[dst]], i64 64, i1 false)
+// Finally, call using a pointer to the temporary stack space.
+// CHECK-O3-NEXT: call void @pass_large(%struct.large* %[[byvaltemp]])
+//
+// The lifetime of the temporary used to pass a pointer to the struct ends here.
+// CHECK-O3-NEXT: %[[bitcastbyvaltemp:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp]] to i8*
+// CHECK-O3-NEXT: call void @llvm.lifetime.end.p0i8(i64 64, i8* %[[bitcastbyvaltemp]])
+//

[PATCH] D68669: [llvm-objdump][WIP] Make llvm-objdump -h compatible with GNU objdump.

2019-10-08 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht planned changes to this revision.
rupprecht added a comment.

Note: herald added reviewers, but this patch is just to provide context. I'll 
send the real patches for review in the coming days.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68669



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


[PATCH] D68669: [llvm-objdump][WIP] Make llvm-objdump -h compatible with GNU objdump.

2019-10-08 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht created this revision.
Herald added subscribers: llvm-commits, cfe-commits, seiya, arphaman, 
jakehehrlich, aheejin, arichardson, sbc100, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: alexshap.
Herald added a reviewer: jhenderson.
Herald added projects: clang, LLVM.
rupprecht planned changes to this revision.
rupprecht added a comment.

Note: herald added reviewers, but this patch is just to provide context. I'll 
send the real patches for review in the coming days.


Note: this patch is large and not intended for submission as-is. Instead, this 
patch presents a poor implementation that makes llvm-objdump GNU compatibile 
for this option (with all existing tests passing, but few added tests, hacky 
code, etc.), and will serve as context for smaller changes to be submitted 
separately with more careful review.

llvm-objdump -h was implemented to be similar to readelf -S (see rL141579 
). However, it is not completely compatible 
with that, and anyone that does want headers displayed that way can use 
llvm-readelf -S now that it exists. Make llvm-objdump compatible with GNU 
objdump instead.

A brief overview of changes:

- Add file offset (with implementations for all filetypes supported by 
llvm-readobj).
- Add 2**n section alignment column (with implementations for all filetypes 
supported by llvm-readobj).
- Section numbers are not the actual section numbers, but something different, 
corresponding to libbfd section numbers. llvm-readelf -s prints the actual 
section numbers if those are desired.
- Filter out certain sections like symtabs/strtabs/relocs. The actual logic is 
a lot more convoluted (and probably isn't fully compatibile, but is pretty 
close).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68669

Files:
  clang/test/Modules/pch_container.m
  lld/test/ELF/bss-start-common.s
  lld/test/ELF/edata-etext.s
  lld/test/ELF/edata-no-bss.s
  lld/test/ELF/emit-relocs-gc.s
  lld/test/ELF/gc-sections-metadata.s
  lld/test/ELF/init_fini_priority.s
  lld/test/ELF/invalid-fde-rel.s
  lld/test/ELF/linkerscript/addr.test
  lld/test/ELF/linkerscript/align-empty.test
  lld/test/ELF/linkerscript/align1.test
  lld/test/ELF/linkerscript/align2.test
  lld/test/ELF/linkerscript/align3.test
  lld/test/ELF/linkerscript/at2.test
  lld/test/ELF/linkerscript/constructor.test
  lld/test/ELF/linkerscript/define.test
  lld/test/ELF/linkerscript/double-bss.test
  lld/test/ELF/linkerscript/eh-frame-emit-relocs.s
  lld/test/ELF/linkerscript/emit-reloc-section-names.s
  lld/test/ELF/linkerscript/expr-sections.test
  lld/test/ELF/linkerscript/input-sec-dup.s
  lld/test/ELF/linkerscript/insert-after.test
  lld/test/ELF/linkerscript/insert-before.test
  lld/test/ELF/linkerscript/memory-include.test
  lld/test/ELF/linkerscript/memory.s
  lld/test/ELF/linkerscript/memory3.s
  lld/test/ELF/linkerscript/memory4.test
  lld/test/ELF/linkerscript/memory5.test
  lld/test/ELF/linkerscript/multi-sections-constraint.s
  lld/test/ELF/linkerscript/non-absolute2.test
  lld/test/ELF/linkerscript/numbers.s
  lld/test/ELF/linkerscript/orphan.s
  lld/test/ELF/linkerscript/orphans.s
  lld/test/ELF/linkerscript/out-of-order-section-in-region.test
  lld/test/ELF/linkerscript/out-of-order.s
  lld/test/ELF/linkerscript/output-section-include.test
  lld/test/ELF/linkerscript/region-alias.s
  lld/test/ELF/linkerscript/repsection-va.s
  lld/test/ELF/linkerscript/section-include.test
  lld/test/ELF/linkerscript/sections-constraint.s
  lld/test/ELF/linkerscript/sections-gc2.s
  lld/test/ELF/linkerscript/sections-keep.s
  lld/test/ELF/linkerscript/sections-sort.s
  lld/test/ELF/linkerscript/sections.s
  lld/test/ELF/linkerscript/sizeof.s
  lld/test/ELF/linkerscript/symbol-only.test
  lld/test/ELF/linkerscript/va.s
  lld/test/ELF/linkerscript/wildcards.s
  lld/test/ELF/linkerscript/wildcards2.s
  lld/test/ELF/relocatable-sections.s
  lld/test/ELF/relocatable.s
  lld/test/ELF/relro-omagic.s
  lld/test/ELF/section-name.s
  lld/test/ELF/sectionstart-noallochdr.s
  lld/test/ELF/sectionstart.s
  lld/test/ELF/strip-all.s
  lld/test/ELF/synthetic-got.s
  llvm/test/MC/COFF/assoc-private.s
  llvm/test/Object/objdump-no-sectionheaders.test
  llvm/test/Object/objdump-sectionheaders.test
  llvm/test/ObjectYAML/CodeView/sections.yaml
  llvm/test/tools/llvm-objcopy/ELF/symtab-error-on-remove-strtab.test
  llvm/test/tools/llvm-objdump/X86/adjust-vma.test
  llvm/test/tools/llvm-objdump/X86/macho-section-headers.test
  llvm/test/tools/llvm-objdump/X86/phdrs-lma.test
  llvm/test/tools/llvm-objdump/X86/phdrs-lma2.test
  llvm/test/tools/llvm-objdump/X86/section-index.s
  llvm/test/tools/llvm-objdump/section-filter.test
  llvm/test/tools/llvm-objdump/wasm.txt
  llvm/test/tools/llvm-objdump/xcoff-section-headers.test
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.h

Index: llvm/tools/llvm-objdump/llvm-objdump.h

r374126 - [IRGen] Emit lifetime markers for temporary struct allocas

2019-10-08 Thread Francis Visoiu Mistrih via cfe-commits
Author: thegameg
Date: Tue Oct  8 15:10:38 2019
New Revision: 374126

URL: http://llvm.org/viewvc/llvm-project?rev=374126=rev
Log:
[IRGen] Emit lifetime markers for temporary struct allocas

When passing arguments using temporary allocas, we need to add the
appropriate lifetime markers so that the stack coloring passes can
re-use the stack space.

This patch keeps track of all the lifetime.start calls emited before the
codegened call, and adds the corresponding lifetime.end calls after the
call.

Differential Revision: https://reviews.llvm.org/D68611

Added:
cfe/trunk/test/CodeGen/aarch64-byval-temp.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=374126=374125=374126=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Oct  8 15:10:38 2019
@@ -3878,6 +3878,11 @@ RValue CodeGenFunction::EmitCall(const C
   Address swiftErrorTemp = Address::invalid();
   Address swiftErrorArg = Address::invalid();
 
+  // When passing arguments using temporary allocas, we need to add the
+  // appropriate lifetime markers. This vector keeps track of all the lifetime
+  // markers that need to be ended right after the call.
+  SmallVector CallLifetimeEndAfterCall;
+
   // Translate all of the arguments as necessary to match the IR lowering.
   assert(CallInfo.arg_size() == CallArgs.size() &&
  "Mismatch between function signature & arguments.");
@@ -3994,6 +3999,18 @@ RValue CodeGenFunction::EmitCall(const C
   Address AI = CreateMemTempWithoutCast(
   I->Ty, ArgInfo.getIndirectAlign(), "byval-temp");
   IRCallArgs[FirstIRArg] = AI.getPointer();
+
+  // Emit lifetime markers for the temporary alloca.
+  uint64_t ByvalTempElementSize =
+  CGM.getDataLayout().getTypeAllocSize(AI.getElementType());
+  llvm::Value *LifetimeSize =
+  EmitLifetimeStart(ByvalTempElementSize, AI.getPointer());
+
+  // Add cleanup code to emit the end lifetime marker after the call.
+  if (LifetimeSize) // In case we disabled lifetime markers.
+CallLifetimeEndAfterCall.emplace_back(AI, LifetimeSize);
+
+  // Generate the copy.
   I->copyInto(*this, AI);
 } else {
   // Skip the extra memcpy call.
@@ -4562,6 +4579,11 @@ RValue CodeGenFunction::EmitCall(const C
 }
   }
 
+  // Explicitly call CallLifetimeEnd::Emit just to re-use the code even though
+  // we can't use the full cleanup mechanism.
+  for (CallLifetimeEnd  : CallLifetimeEndAfterCall)
+LifetimeEnd.Emit(*this, /*Flags=*/{});
+
   return Ret;
 }
 

Added: cfe/trunk/test/CodeGen/aarch64-byval-temp.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/aarch64-byval-temp.c?rev=374126=auto
==
--- cfe/trunk/test/CodeGen/aarch64-byval-temp.c (added)
+++ cfe/trunk/test/CodeGen/aarch64-byval-temp.c Tue Oct  8 15:10:38 2019
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -emit-llvm -triple arm64-- -o - %s -O0 | FileCheck %s 
--check-prefix=CHECK-O0
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-optzns -triple arm64-- -o - %s -O3 
| FileCheck %s --check-prefix=CHECK-O3
+
+struct large {
+void* pointers[8];
+};
+
+void pass_large(struct large);
+
+// For arm64, we don't use byval to pass structs but instead we create
+// temporary allocas.
+//
+// Make sure we generate the appropriate lifetime markers for the temporary
+// allocas so that the optimizer can re-use stack slots if possible.
+void example() {
+struct large l = {0};
+pass_large(l);
+pass_large(l);
+}
+// CHECK-O0-LABEL: define void @example(
+// The alloca for the struct on the stack.
+// CHECK-O0: %[[l:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// The alloca for the temporary stack space that we use to pass the argument.
+// CHECK-O0-NEXT: %[[byvaltemp:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// Another one to pass the argument to the second function call.
+// CHECK-O0-NEXT: %[[byvaltemp1:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// First, memset `l` to 0.
+// CHECK-O0-NEXT: %[[bitcastl:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] 
to i8*
+// CHECK-O0-NEXT: call void @llvm.memset.p0i8.i64(i8* align 8 %[[bitcastl]], 
i8 0, i64 64, i1 false)
+// Then, memcpy `l` to the temporary stack space.
+// CHECK-O0-NEXT: %[[src:[0-9A-Za-z-]+]] = bitcast %struct.large* 
%[[byvaltemp]] to i8*
+// CHECK-O0-NEXT: %[[dst:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O0-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %[[src]], 
i8* align 8 %[[dst]], i64 64, i1 false)
+// Finally, call using a pointer to the temporary stack space.
+// CHECK-O0-NEXT: call void @pass_large(%struct.large* %[[byvaltemp]])
+// Now, do the same for 

[PATCH] D68665: [HIP] Fix -save-temps

2019-10-08 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: tra, ashi1.

Currently clang does not save some of the intermediate file generated during 
device compilation for HIP when -save-temps is specified.

This patch fixes that.


https://reviews.llvm.org/D68665

Files:
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/HIP.cpp
  lib/Driver/ToolChains/HIP.h
  test/Driver/hip-save-temps.hip

Index: test/Driver/hip-save-temps.hip
===
--- /dev/null
+++ test/Driver/hip-save-temps.hip
@@ -0,0 +1,27 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -nogpulib -save-temps \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK,NORDC %s
+
+// RUN: %clang -### -target x86_64-linux-gnu -nogpulib -save-temps \
+// RUN:   -fgpu-rdc -x hip --cuda-gpu-arch=gfx900 %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK,RDC %s
+
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900.cui"
+// CHECK: {{.*}}llvm-link{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900-linked.bc"
+// CHECK: {{.*}}opt{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900-optimized.bc"
+// CHECK: {{.*}}llc{{.*}}"-filetype=asm"{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900.s"
+// CHECK: {{.*}}llc{{.*}}"-filetype=obj"{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900.o"
+// NORDC: {{.*}}lld{{.*}}"-o" "hip-save-temps-hip-amdgcn-amd-amdhsa-gfx900.out"
+// RDC: {{.*}}lld{{.*}}"-o" "a.out-hip-amdgcn-amd-amdhsa-gfx900"
+// NORDC: {{.*}}clang-offload-bundler{{.*}}"-outputs=hip-save-temps.hip-hip-amdgcn-amd-amdhsa.hipfb"
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps-host-x86_64-unknown-linux-gnu.cui"
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps-host-x86_64-unknown-linux-gnu.bc"
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps-host-x86_64-unknown-linux-gnu.s"
+// CHECK: {{.*}}clang{{.*}}"-o" "hip-save-temps{{.*}}.o"
+// RDC: {{.*}}clang-offload-bundler{{.*}}"-outputs=a.out.hipfb"
+// CHECK: {{.*}}ld{{.*}}"-o" "a.out"
+
Index: lib/Driver/ToolChains/HIP.h
===
--- lib/Driver/ToolChains/HIP.h
+++ lib/Driver/ToolChains/HIP.h
@@ -58,7 +58,8 @@
   const llvm::opt::ArgList ,
   llvm::StringRef SubArchName,
   llvm::StringRef OutputFilePrefix,
-  const char *InputFileName) const;
+  const char *InputFileName,
+  bool IsAsm = false) const;
 
   void constructLldCommand(Compilation , const JobAction ,
const InputInfoList , const InputInfo ,
Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -48,6 +48,20 @@
   D.Diag(diag::err_drv_no_such_file) << BCName;
 }
 
+static const char *getOutputFileName(Compilation , StringRef Base,
+ const char *Postfix,
+ const char *Extension) {
+  const char *OutputFileName;
+  if (C.getDriver().isSaveTempsEnabled()) {
+OutputFileName =
+C.getArgs().MakeArgString(Base.str() + Postfix + "." + Extension);
+  } else {
+std::string TmpName =
+C.getDriver().GetTemporaryPath(Base.str() + Postfix, Extension);
+OutputFileName = C.addTempFile(C.getArgs().MakeArgString(TmpName));
+  }
+  return OutputFileName;
+}
 } // namespace
 
 const char *AMDGCN::Linker::constructLLVMLinkCommand(
@@ -61,10 +75,7 @@
 
   // Add an intermediate output file.
   CmdArgs.push_back("-o");
-  std::string TmpName =
-  C.getDriver().GetTemporaryPath(OutputFilePrefix.str() + "-linked", "bc");
-  const char *OutputFileName =
-  C.addTempFile(C.getArgs().MakeArgString(TmpName));
+  auto OutputFileName = getOutputFileName(C, OutputFilePrefix, "-linked", "bc");
   CmdArgs.push_back(OutputFileName);
   SmallString<128> ExecPath(C.getDriver().Dir);
   llvm::sys::path::append(ExecPath, "llvm-link");
@@ -109,10 +120,8 @@
   }
 
   OptArgs.push_back("-o");
-  std::string TmpFileName = C.getDriver().GetTemporaryPath(
-  OutputFilePrefix.str() + "-optimized", "bc");
-  const char *OutputFileName =
-  C.addTempFile(C.getArgs().MakeArgString(TmpFileName));
+  auto OutputFileName =
+  getOutputFileName(C, OutputFilePrefix, "-optimized", "bc");
   OptArgs.push_back(OutputFileName);
   SmallString<128> OptPath(C.getDriver().Dir);
   llvm::sys::path::append(OptPath, "opt");
@@ -124,11 +133,13 @@
 const char *AMDGCN::Linker::constructLlcCommand(
 Compilation , const JobAction , const InputInfoList ,
 const llvm::opt::ArgList , llvm::StringRef SubArchName,
-

r374119 - Fix crash or wrong code bug if a lifetime-extended temporary contains a

2019-10-08 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Tue Oct  8 14:26:03 2019
New Revision: 374119

URL: http://llvm.org/viewvc/llvm-project?rev=374119=rev
Log:
Fix crash or wrong code bug if a lifetime-extended temporary contains a
"non-constant" value.

If the constant evaluator evaluates part of a variable initializer,
including the initializer for some lifetime-extended temporary, but
fails to fully evaluate the initializer, it can leave behind wrong
values for temporaries encountered in that initialization. Don't try to
emit those from CodeGen! Instead, look at the values that constant
evaluation produced if (and only if) it actually succeeds and we're
emitting the lifetime-extending declaration's initializer as a constant.

Added:
cfe/trunk/test/CodeGenCXX/no-const-init-cxx2a.cpp
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=374119=374118=374119=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Oct  8 14:26:03 2019
@@ -13569,8 +13569,10 @@ bool Expr::EvaluateAsConstantExpr(EvalRe
   if (!Info.discardCleanups())
 llvm_unreachable("Unhandled cleanup; missing full expression marker?");
 
-  return CheckConstantExpression(Info, getExprLoc(), getType(), Result.Val,
- Usage) &&
+  QualType T = getType();
+  if (!isRValue())
+T = Ctx.getLValueReferenceType(T);
+  return CheckConstantExpression(Info, getExprLoc(), T, Result.Val, Usage) &&
  CheckMemoryLeaks(Info);
 }
 

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=374119=374118=374119=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Oct  8 14:26:03 2019
@@ -4978,14 +4978,13 @@ ConstantAddress CodeGenModule::GetAddrOf
   VD, E->getManglingNumber(), Out);
 
   APValue *Value = nullptr;
-  if (E->getStorageDuration() == SD_Static) {
-// We might have a cached constant initializer for this temporary. Note
-// that this might have a different value from the value computed by
-// evaluating the initializer if the surrounding constant expression
-// modifies the temporary.
+  if (E->getStorageDuration() == SD_Static && VD && VD->evaluateValue()) {
+// If the initializer of the extending declaration is a constant
+// initializer, we should have a cached constant initializer for this
+// temporary. Note that this might have a different value from the value
+// computed by evaluating the initializer if the surrounding constant
+// expression modifies the temporary.
 Value = getContext().getMaterializedTemporaryValue(E, false);
-if (Value && Value->isAbsent())
-  Value = nullptr;
   }
 
   // Try evaluating it now, it might have a constant initializer.

Added: cfe/trunk/test/CodeGenCXX/no-const-init-cxx2a.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/no-const-init-cxx2a.cpp?rev=374119=auto
==
--- cfe/trunk/test/CodeGenCXX/no-const-init-cxx2a.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/no-const-init-cxx2a.cpp Tue Oct  8 14:26:03 2019
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -std=c++2a | 
FileCheck %s
+
+// CHECK-DAG: @p = {{.*}} null
+// CHECK-DAG: @_ZGR1p_ = {{.*}} null
+int *const  = new int;
+
+struct d {
+  constexpr d(int &) : e(f) {}
+  int 
+};
+
+// CHECK-DAG: @g = {{.*}} null
+// CHECK-DAG: @_ZGR1g_ = {{.*}} zeroinitializer
+d &{{0}};
+
+// CHECK: define {{.*}} @__cxx_global_var_init
+// CHECK: define {{.*}} @__cxx_global_var_init
+// CHECK-NOT: define {{.*}} @__cxx_global_var_init


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


[PATCH] D68652: [driver][hip] Skip bundler if host action is nothing.

2019-10-08 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/test/Driver/hip-syntax-only.hip:7
+
+// CHECK-DAG: clang{{.*}}" "-cc1" {{.*}} "-fcuda-is-device"
+// CHECK-DAG: clang{{.*}}" "-cc1" "-triple" "x86_64"

hliao wrote:
> tra wrote:
> > I'd include `-target ` and a comment describing that we're making sure 
> > that both host and device compilations are still executed.
> won't -fcuda-is-device be sufficient? that's option specific to device-side 
> compilation.
It's sufficient, but we're currently comparing sort of apples 
(`-fcuda-is-device`) and oranges (`-triple FOO`) on these two lines. Changing 
it into `-triple GPU  -fcuda-is-device` vs. `-triple HOST` would make it easier 
to understand what's the intent here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68652



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


[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-10-08 Thread Melanie Blower via Phabricator via cfe-commits
mibintc updated this revision to Diff 223940.
mibintc added a comment.

I added a test case to show the warning diagnostics when options conflicting 
with fp-model are provided. I fixed a couple bugs in RenderFloatingPointOptions 
when issueing diagnostics.  still owe a test case showing how the fp-model, 
rounding, and trapping options are rendered by the Driver for cc1


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fast-math.c
  clang/test/Driver/fp-model.c
  llvm/include/llvm/Target/TargetOptions.h

Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -107,7 +107,7 @@
   public:
 TargetOptions()
 : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
-  NoNaNsFPMath(false), NoTrappingFPMath(false),
+  NoNaNsFPMath(false), NoTrappingFPMath(true), RoundingFPMath(false),
   NoSignedZerosFPMath(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
@@ -154,6 +154,11 @@
 /// specifies that there are no trap handlers to handle exceptions.
 unsigned NoTrappingFPMath : 1;
 
+/// RoundingFPMath - This flag is enabled when the
+/// -enable-rounding-fp-math is specified on the command line. This
+/// specifies dynamic rounding mode.
+unsigned RoundingFPMath : 1;
+
 /// NoSignedZerosFPMath - This flag is enabled when the
 /// -enable-no-signed-zeros-fp-math is specified on the command line. This
 /// specifies that optimizations are allowed to treat the sign of a zero
Index: clang/test/Driver/fp-model.c
===
--- /dev/null
+++ clang/test/Driver/fp-model.c
@@ -0,0 +1,64 @@
+// Test that incompatible combinations of -ffp-model= options
+// and other floating point options get a warning diagnostic.
+//
+// REQUIRES: clang-driver
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN %s
+// WARN: warning: overriding '-ffp-model=fast' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN1 %s
+// WARN1: warning: overriding '-ffp-model=fast' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fassociative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN2 %s
+// WARN2: warning: overriding '-ffp-model=strict' option with '-fassociative-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN3 %s
+// WARN3: warning: overriding '-ffp-model=strict' option with '-ffast-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffinite-math-only -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN4 %s
+// WARN4: warning: overriding '-ffp-model=strict' option with '-ffinite-math-only' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN5 %s
+// WARN5: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN6 %s
+// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN7 %s
+// WARN7: warning: overriding '-ffp-model=strict' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-infinities -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN8 %s
+// WARN8: warning: overriding '-ffp-model=strict' option with '-fno-honor-infinities' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-nans -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN9 %s
+// WARN9: warning: overriding '-ffp-model=strict' option with '-fno-honor-nans' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-rounding-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNa %s
+// WARNa: warning: overriding '-ffp-model=strict' 

[PATCH] D68663: [clang-offload-bundler] Support `.cui` and `.d`.

2019-10-08 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

Maybe, add a TODO to eventually remove `.d` . 
`.cui` should probably remain as it's yet another variant of preprocessed 
output that we allow bundling for C/C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68663



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


[PATCH] D68660: [tooling] Teach Tooling to understand compilation with offloading.

2019-10-08 Thread Artem Belevich via Phabricator via cfe-commits
tra added a reviewer: klimek.
tra added a comment.

Added Manuel as someone familiar with tooling.




Comment at: clang/lib/Tooling/Tooling.cpp:117
   // The one job we find should be to invoke clang again.
   const auto  = cast(*Jobs.begin());
   if (StringRef(Cmd.getCreator().getName()) != "clang") {

Is this still the right job for us to pick? I think we want this to be the host 
compilation. 

As things are right now my guess is that we're probably picking the first 
device-side compilation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68660



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


[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68640



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


[PATCH] D68610: [clang] enable_trivial_var_init_zero should not be Joined<>

2019-10-08 Thread Vitaly Buka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc831ce8c0619: [clang] enable_trivial_var_init_zero should 
not be Joined (authored by vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68610

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1720,7 +1720,7 @@
 def ftrivial_auto_var_init : Joined<["-"], "ftrivial-auto-var-init=">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Initialize trivial automatic stack 
variables: uninitialized (default)"
   " | pattern">, Values<"uninitialized,pattern">;
-def enable_trivial_var_init_zero : Joined<["-"], 
"enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
+def enable_trivial_var_init_zero : Flag<["-"], 
"enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
   Flags<[CC1Option, CoreOption]>,
   HelpText<"Trivial automatic variable initialization to zero is only here for 
benchmarks, it'll eventually be removed, and I'm OK with that because I'm only 
using it to benchmark">;
 def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group, 
Flags<[CoreOption]>,


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1720,7 +1720,7 @@
 def ftrivial_auto_var_init : Joined<["-"], "ftrivial-auto-var-init=">, Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Initialize trivial automatic stack variables: uninitialized (default)"
   " | pattern">, Values<"uninitialized,pattern">;
-def enable_trivial_var_init_zero : Joined<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
+def enable_trivial_var_init_zero : Flag<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
   Flags<[CC1Option, CoreOption]>,
   HelpText<"Trivial automatic variable initialization to zero is only here for benchmarks, it'll eventually be removed, and I'm OK with that because I'm only using it to benchmark">;
 def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group, Flags<[CoreOption]>,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r374113 - [clang] enable_trivial_var_init_zero should not be Joined<>

2019-10-08 Thread Vitaly Buka via cfe-commits
Author: vitalybuka
Date: Tue Oct  8 13:34:53 2019
New Revision: 374113

URL: http://llvm.org/viewvc/llvm-project?rev=374113=rev
Log:
[clang] enable_trivial_var_init_zero should not be Joined<>

Reviewers: rnk

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D68610

Modified:
cfe/trunk/include/clang/Driver/Options.td

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=374113=374112=374113=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Oct  8 13:34:53 2019
@@ -1720,7 +1720,7 @@ def fstack_protector : Flag<["-"], "fsta
 def ftrivial_auto_var_init : Joined<["-"], "ftrivial-auto-var-init=">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Initialize trivial automatic stack 
variables: uninitialized (default)"
   " | pattern">, Values<"uninitialized,pattern">;
-def enable_trivial_var_init_zero : Joined<["-"], 
"enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
+def enable_trivial_var_init_zero : Flag<["-"], 
"enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
   Flags<[CC1Option, CoreOption]>,
   HelpText<"Trivial automatic variable initialization to zero is only here for 
benchmarks, it'll eventually be removed, and I'm OK with that because I'm only 
using it to benchmark">;
 def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group, 
Flags<[CoreOption]>,


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


[PATCH] D68663: [clang-offload-bundler] Support `.cui` and `.d`.

2019-10-08 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

Adding this support is aimed to prevent crashing or failure, it's not intended 
to be final output for -M or -E. So, hip fails on these options due to the 
unsupported type of clang-offload-bundler. Before we nail down the details on 
the expected output of -M or -E, especially -M, we at least should not fail 
them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68663



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


[PATCH] D68610: [clang] enable_trivial_var_init_zero should not be Joined<>

2019-10-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68610



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


[PATCH] D68663: [clang-offload-bundler] Support `.cui` and `.d`.

2019-10-08 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added reviewers: tra, yaxunl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68663

Files:
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -71,6 +71,8 @@
"Current supported types are:\n"
"  i   - cpp-output\n"
"  ii  - c++-cpp-output\n"
+   "  cui - cuda/hip-output\n"
+   "  d   - dependency\n"
"  ll  - llvm\n"
"  bc  - llvm-bc\n"
"  s   - assembler\n"
@@ -628,6 +630,10 @@
 return new TextFileHandler(/*Comment=*/"//");
   if (FilesType == "ii")
 return new TextFileHandler(/*Comment=*/"//");
+  if (FilesType == "cui")
+return new TextFileHandler(/*Comment=*/"//");
+  if (FilesType == "d")
+return new TextFileHandler(/*Comment=*/"#");
   if (FilesType == "ll")
 return new TextFileHandler(/*Comment=*/";");
   if (FilesType == "bc")


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -71,6 +71,8 @@
"Current supported types are:\n"
"  i   - cpp-output\n"
"  ii  - c++-cpp-output\n"
+   "  cui - cuda/hip-output\n"
+   "  d   - dependency\n"
"  ll  - llvm\n"
"  bc  - llvm-bc\n"
"  s   - assembler\n"
@@ -628,6 +630,10 @@
 return new TextFileHandler(/*Comment=*/"//");
   if (FilesType == "ii")
 return new TextFileHandler(/*Comment=*/"//");
+  if (FilesType == "cui")
+return new TextFileHandler(/*Comment=*/"//");
+  if (FilesType == "d")
+return new TextFileHandler(/*Comment=*/"#");
   if (FilesType == "ll")
 return new TextFileHandler(/*Comment=*/";");
   if (FilesType == "bc")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r374110 - [clang] Add llvm-ifs in test deps

2019-10-08 Thread Vitaly Buka via cfe-commits
Author: vitalybuka
Date: Tue Oct  8 13:23:24 2019
New Revision: 374110

URL: http://llvm.org/viewvc/llvm-project?rev=374110=rev
Log:
[clang] Add llvm-ifs in test deps

Modified:
cfe/trunk/test/CMakeLists.txt

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=374110=374109=374110=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Tue Oct  8 13:23:24 2019
@@ -108,6 +108,7 @@ if( NOT CLANG_BUILT_STANDALONE )
 llvm-cxxfilt
 llvm-dis
 llvm-dwarfdump
+llvm-ifs
 llvm-lto2
 llvm-modextract
 llvm-nm


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


[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

2019-10-08 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

Gentle ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55326



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


[PATCH] D67112: [Sema] Add implicit cast for conversion of function references

2019-10-08 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert planned changes to this revision.
aaronpuchert added a comment.

In D67112#1655937 , @aaronpuchert 
wrote:

> If anyone shares my feeling that the boolean output parameters of 
> `CompareReferenceRelationship` should rather move to the return value, I 
> would be happy to do that.


I'll do that in a separate change and then rebase this on top.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67112



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


[PATCH] D68340: Add AIX toolchain and basic linker functionality

2019-10-08 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:35
+  // Only support 32 and 64 bit
+  if (!IsArch32Bit && !IsArch64Bit)
+llvm_unreachable("Unsupported bit width value");

Xiangling_L wrote:
> Is there any reason to use llvm_unreachable here? I think we should use  
> 'assertion' instead here:
> 
> ```
> assert((IsArch32Bit || IsArch64Bit) && "...");
> ```
IsArch64Bit used only in the assertion could cause warning when the assertion 
is turned off. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68340



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


[PATCH] D68340: Add AIX toolchain and basic linker functionality

2019-10-08 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:38
+
+  if (!Args.hasArg(options::OPT_nostdlib)) {
+CmdArgs.push_back("-e");

Test with Clangtana on terran, when no '-nostdlib' specified, since '-e' & 
'__start' are the default behavior for AIX system linker, so there are no 
explicitly '-e' & '__start' found on linker input commanline, so I am wondering 
do we need to explicitly add them to 'CmdArgs'?



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:47
+  else
+CmdArgs.push_back("-bso");
+

Ditto. Since by default, AIX linker is dynamically linked, '-bso' is implicitly 
set on AIX system linker when testing with Clangtana, so do we need to 
explicitly set '-bso' in LLVM?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68340



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


Re: r374098 - [NFC] Attempt to make ubsan-blacklist-vfs test pass on Windows

2019-10-08 Thread Jan Korous via cfe-commits
Thanks for the fix!

> On Oct 8, 2019, at 12:06 PM, Nico Weber  wrote:
> 
> Almost: http://45.33.8.238/win/78/step_6.txt 
> 
> 
> C:\src\llvm-project\clang\test\CodeGen\ubsan-blacklist-vfs.c:11:25: error: 
> INVALID-MAPPED-FILE: expected string not found in input
> // INVALID-MAPPED-FILE: invalid-virtual-file.blacklist': No such file or 
> directory
> ^
> :1:1: note: scanning from here
> fatal error: error in backend: can't open file 
> 'C:/src/llvm-project/out/gn/obj/clang/test/CodeGen/Output/invalid-virtual-file.blacklist':
>  no such file or directory
> 
> 
> Looks like it expects a "No such file" with a capital "No" but gets a "no 
> such file" with a lower-case n. {{[Nn]}}o probably fixes this.
> 
> On Tue, Oct 8, 2019 at 2:10 PM Jan Korous via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: jkorous
> Date: Tue Oct  8 11:13:04 2019
> New Revision: 374098
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=374098=rev 
> 
> Log:
> [NFC] Attempt to make ubsan-blacklist-vfs test pass on Windows
> 
> Previously disabled in d0c2d5daa3e
> 
> Modified:
> cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c
> 
> Modified: cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c?rev=374098=374097=374098=diff
>  
> 
> ==
> --- cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c (original)
> +++ cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c Tue Oct  8 11:13:04 2019
> @@ -1,5 +1,3 @@
> -// UNSUPPORTED: system-windows
> -
>  // Verify ubsan doesn't emit checks for blacklisted functions and files
>  // RUN: echo "fun:hash" > %t-func.blacklist
>  // RUN: echo "src:%s" | sed -e 's/\\//g' > %t-file.blacklist
> @@ -7,9 +5,9 @@
>  // RUN: rm -f %t-vfsoverlay.yaml
>  // RUN: rm -f %t-nonexistent.blacklist
>  // RUN: sed -e "s|@DIR@|%/T|g" %S/Inputs/sanitizer-blacklist-vfsoverlay.yaml 
> | sed -e "s|@REAL_FILE@|%/t-func.blacklist|g" | sed -e 
> "s|@NONEXISTENT_FILE@|%/t-nonexistent.blacklist|g" > %t-vfsoverlay.yaml
> -// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
> %t-vfsoverlay.yaml -fsanitize-blacklist=%T/only-virtual-file.blacklist 
> -emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
> +// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
> %t-vfsoverlay.yaml -fsanitize-blacklist=%/T/only-virtual-file.blacklist 
> -emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
> 
> -// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
> %t-vfsoverlay.yaml -fsanitize-blacklist=%T/invalid-virtual-file.blacklist 
> -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID-MAPPED-FILE
> +// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
> %t-vfsoverlay.yaml -fsanitize-blacklist=%/T/invalid-virtual-file.blacklist 
> -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID-MAPPED-FILE
>  // INVALID-MAPPED-FILE: invalid-virtual-file.blacklist': No such file or 
> directory
> 
>  // RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
> %t-vfsoverlay.yaml -fsanitize-blacklist=%t-nonexistent.blacklist -emit-llvm 
> %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
> 

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


r374107 - [OPENMP50]Multiple vendors in vendor context must be treated as logical

2019-10-08 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Tue Oct  8 12:44:16 2019
New Revision: 374107

URL: http://llvm.org/viewvc/llvm-project?rev=374107=rev
Log:
[OPENMP50]Multiple vendors in vendor context must be treated as logical
and of vendors, not or.

If several vendors are provided in the same vendor context trait, the
context shall match only if all vendors are matching, not one of them.
This is per OpenMP 5.0, 2.3.3 Matching and Scoring Context Selectors,
all selectors in the construct, device, and implementation sets of the
context selector appear in the corresponding trait set of the OpenMP
context.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/OpenMP/declare_variant_ast_print.c
cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp
cfe/trunk/test/OpenMP/declare_variant_implementation_vendor_codegen.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=374107=374106=374107=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Oct  8 12:44:16 2019
@@ -3309,7 +3309,7 @@ def OMPDeclareVariant : InheritableAttr
  [
"CtxUnknown", "CtxVendor"
  ]>,
-StringArgument<"ImplVendor", 1>
+VariadicStringArgument<"ImplVendors">
   ];
   let AdditionalMembers = [{
 void printScore(raw_ostream & OS, const PrintingPolicy ) const {
@@ -3337,7 +3337,11 @@ def OMPDeclareVariant : InheritableAttr
 case CtxVendor:
   OS << "vendor(";
   printScore(OS, Policy);
-  OS << getImplVendor();
+  if (implVendors_size() > 0) {
+OS << *implVendors(). begin();
+for (StringRef VendorName : llvm::drop_begin(implVendors(), 1))
+  OS << ", " << VendorName;
+  }
   OS << ")";
   break;
 case CtxUnknown:

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=374107=374106=374107=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Oct  8 12:44:16 2019
@@ -9110,15 +9110,15 @@ public:
 OMPDeclareVariantAttr::CtxSetUnknown;
 OMPDeclareVariantAttr::CtxSelectorType Ctx =
 OMPDeclareVariantAttr::CtxUnknown;
-StringRef ImplVendor;
+MutableArrayRef ImplVendors;
 ExprResult CtxScore;
 explicit OpenMPDeclareVariantCtsSelectorData() = default;
 explicit OpenMPDeclareVariantCtsSelectorData(
 OMPDeclareVariantAttr::CtxSelectorSetType CtxSet,
-OMPDeclareVariantAttr::CtxSelectorType Ctx, StringRef ImplVendor,
-ExprResult CtxScore)
-: CtxSet(CtxSet), Ctx(Ctx), ImplVendor(ImplVendor), CtxScore(CtxScore) 
{
-}
+OMPDeclareVariantAttr::CtxSelectorType Ctx,
+MutableArrayRef ImplVendors, ExprResult CtxScore)
+: CtxSet(CtxSet), Ctx(Ctx), ImplVendors(ImplVendors),
+  CtxScore(CtxScore) {}
   };
 
   /// Checks if the variant/multiversion functions are compatible.

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=374107=374106=374107=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Oct  8 12:44:16 2019
@@ -11172,7 +11172,8 @@ template <>
 bool checkContext(
 const OMPDeclareVariantAttr *A) {
-  return !A->getImplVendor().compare("llvm");
+  return llvm::all_of(A->implVendors(),
+  [](StringRef S) { return !S.compare_lower("llvm"); });
 }
 
 static bool greaterCtxScore(ASTContext , const Expr *LHS, const Expr *RHS) 
{

Modified: cfe/trunk/lib/Parse/ParseOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseOpenMP.cpp?rev=374107=374106=374107=diff
==
--- cfe/trunk/lib/Parse/ParseOpenMP.cpp (original)
+++ cfe/trunk/lib/Parse/ParseOpenMP.cpp Tue Oct  8 12:44:16 2019
@@ -853,6 +853,7 @@ static void parseImplementationSelector(
 (void)T.expectAndConsume(diag::err_expected_lparen_after,
  CtxSelectorName.data());
 const ExprResult Score = parseContextScore(P);
+SmallVector, 4> Vendors;
 do {
   // Parse .
   StringRef VendorName;
@@ -860,18 +861,14 @@ static void parseImplementationSelector(
 Buffer.clear();
 VendorName = 

[PATCH] D68660: [tooling] Teach Tooling to understand compilation with offloading.

2019-10-08 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added reviewers: tra, yaxunl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- So far, we only recognize the compilation with offloading and skip the 
offloading part.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68660

Files:
  clang/lib/Tooling/Tooling.cpp
  clang/test/Tooling/clang-check-offload.cpp


Index: clang/test/Tooling/clang-check-offload.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-offload.cpp
@@ -0,0 +1,4 @@
+// RUN: not clang-check "%s" -- -c -x hip -nogpulib 2>&1 | FileCheck %s
+
+// CHECK: C++ requires
+invalid;
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -91,7 +91,20 @@
   // We expect to get back exactly one Command job, if we didn't something
   // failed. Extract that job from the Compilation.
   const driver::JobList  = Compilation->getJobs();
-  if (Jobs.size() != 1 || !isa(*Jobs.begin())) {
+  bool OffloadCompilation = false;
+  if (Jobs.size() > 1) {
+for (auto  : Compilation->getActions()){
+  // On MacOSX real actions may end up being wrapped in BindArchAction
+  if (isa(A))
+A = *A->input_begin();
+  if (isa(A)) {
+OffloadCompilation = true;
+break;
+  }
+}
+  }
+  if (Jobs.size() == 0 || !isa(*Jobs.begin()) ||
+  (Jobs.size() > 1 && !OffloadCompilation)) {
 SmallString<256> error_msg;
 llvm::raw_svector_ostream error_stream(error_msg);
 Jobs.Print(error_stream, "; ", true);


Index: clang/test/Tooling/clang-check-offload.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-offload.cpp
@@ -0,0 +1,4 @@
+// RUN: not clang-check "%s" -- -c -x hip -nogpulib 2>&1 | FileCheck %s
+
+// CHECK: C++ requires
+invalid;
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -91,7 +91,20 @@
   // We expect to get back exactly one Command job, if we didn't something
   // failed. Extract that job from the Compilation.
   const driver::JobList  = Compilation->getJobs();
-  if (Jobs.size() != 1 || !isa(*Jobs.begin())) {
+  bool OffloadCompilation = false;
+  if (Jobs.size() > 1) {
+for (auto  : Compilation->getActions()){
+  // On MacOSX real actions may end up being wrapped in BindArchAction
+  if (isa(A))
+A = *A->input_begin();
+  if (isa(A)) {
+OffloadCompilation = true;
+break;
+  }
+}
+  }
+  if (Jobs.size() == 0 || !isa(*Jobs.begin()) ||
+  (Jobs.size() > 1 && !OffloadCompilation)) {
 SmallString<256> error_msg;
 llvm::raw_svector_ostream error_stream(error_msg);
 Jobs.Print(error_stream, "; ", true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message

2019-10-08 Thread Dávid Bolvanský via cfe-commits

Yeah, I tried to fix even that case but is not so simple so not worth any extra 
time/complexivity.

> Dňa 8. 10. 2019 o 19:09 užívateľ Arthur O'Dwyer  
> napísal:
> 
> 
> On Mon, Oct 7, 2019 at 6:58 PM Dávid Bolvanský  
> wrote:
> >> FWIW I found the "always evaluates to 'true'" bit important to
> >> understand the warning.
> >
> > Yeah. I moved this check somewhere else, so we can print precise message:
> > r373973 should emit "bitwise negation of a boolean expression always
> > evaluates to 'true'; did you mean logical negation?" where possible.
> > In the suspicious case like int i = ~b there is a general message
> > "bitwise negation of a boolean expression; did you mean logical
> > negation?".
> >
> > I like it now. What do you think? fine for you?
> 
> I see. Yes, all the cases I tried produce appropriate diagnostics. I like it!
> 
>> Hm, there is no "bitwise negation of a boolean expression always
>> evaluates to 'true'; did you mean logical negation?" for chromium
>> case [ https://bugs.chromium.org/p/chromium/issues/detail?id=1011810 ]. I 
>> will try to fix it.
> 
> The important part there seems to be that the result of `~b` (which must be 
> either -1 or -2) is used as the operand to `!=` or `==`.
> My opinion is that it is not worth the extra complication just to improve the 
> error message for this case.
> It would be interesting to do some kind of general-purpose dataflow before 
> emitting diagnostics...
> I notice that Clang's optimizer is smart enough to optimize
> bool foo(bool a, bool b) {
> return a == ~b;
> }
> bool bar(int x) {
> return x + 1 < -INT_MAX;
> }
> into `return 0`. If it could propagate that information up and produce a 
> diagnostic, users might appreciate that. But the challenge as always is that 
> we can never tell if the user might sometimes be doing that sort of thing on 
> purpose (in inlined code, in macros, in generated code, etc).
> 
> my $.02,
> –Arthur
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68587: [hip] Assume host-only compilation if the final phase is ahead of `backend`.

2019-10-08 Thread Michael Liao via Phabricator via cfe-commits
hliao abandoned this revision.
hliao added a comment.

need to more clarification and re-design


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68587



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


[PATCH] D68587: [hip] Assume host-only compilation if the final phase is ahead of `backend`.

2019-10-08 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

In D68587#1700178 , @tra wrote:

> TL; DR; 
>  +1 to formalizing how we want -M*/-E/-S/-emit-llvm/-fsyntax-only to behave. 
>  OK with -M/-E/-S defaulting to host, and erroring out if applied to multiple 
> sub-compilations.
>  I'm still convinced that the tooling issue with multiple subcompilations is 
> orthogonal to this change and should be handled in libclang and that 
> -fsyntax-only should not default to one sub-compilation.


I am starting to fix clang tooling issues found in both driver 
(https://reviews.llvm.org/D68652) and tooling parts. Will submit tooling part 
for review soon. After that, we may nail down the details on expected output 
from -M*/-E and etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68587



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


r374105 - Try to get ubsan-blacklist-vfs.c pass more on Windows

2019-10-08 Thread Nico Weber via cfe-commits
Author: nico
Date: Tue Oct  8 12:25:49 2019
New Revision: 374105

URL: http://llvm.org/viewvc/llvm-project?rev=374105=rev
Log:
Try to get ubsan-blacklist-vfs.c pass more on Windows

Modified:
cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c

Modified: cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c?rev=374105=374104=374105=diff
==
--- cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c (original)
+++ cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c Tue Oct  8 12:25:49 2019
@@ -8,10 +8,10 @@
 // RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%/T/only-virtual-file.blacklist 
-emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
 
 // RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%/T/invalid-virtual-file.blacklist 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID-MAPPED-FILE
-// INVALID-MAPPED-FILE: invalid-virtual-file.blacklist': No such file or 
directory
+// INVALID-MAPPED-FILE: invalid-virtual-file.blacklist': {{[Nn]}}o such file 
or directory
 
 // RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%t-nonexistent.blacklist -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=INVALID
-// INVALID: nonexistent.blacklist': No such file or directory
+// INVALID: nonexistent.blacklist': {{[Nn]}}o such file or directory
 
 unsigned i;
 


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


[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8cb804a3c9ce: Try to get readability-deleted-default.cpp to 
pass on Windows. (authored by thakis).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68640

Files:
  clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp


Index: clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
+++ clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
@@ -2,6 +2,21 @@
 // RUN:   -config="{CheckOptions: \
 // RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
 // RUN:   value: 0}]}"
+//
+// With -fms-compatibility and -DEXTERNINLINE, the extern inline shouldn't
+// produce additional diagnostics, so same check suffix as before:
+// RUN: %check_clang_tidy %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fms-compatibility -DEXTERNINLINE
+//
+// With -fno-ms-compatiblity, DEXTERNINLINE causes additional output.
+// (The leading ',' means "default checks in addition to NOMSCOMPAT checks.)
+// RUN: %check_clang_tidy -check-suffix=,NOMSCOMPAT \
+// RUN:   %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fno-ms-compatibility -DEXTERNINLINE
 
 extern int Xyz;
 extern int Xyz; // Xyz
@@ -74,6 +89,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
 // CHECK-FIXES: {{^}}// g{{$}}
 
+#if defined(EXTERNINLINE)
 extern inline void g(); // extern g
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant 'g' declaration
-// CHECK-FIXES: {{^}}// extern g{{$}}
+// CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' 
declaration
+// CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
+#endif


Index: clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
+++ clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
@@ -2,6 +2,21 @@
 // RUN:   -config="{CheckOptions: \
 // RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
 // RUN:   value: 0}]}"
+//
+// With -fms-compatibility and -DEXTERNINLINE, the extern inline shouldn't
+// produce additional diagnostics, so same check suffix as before:
+// RUN: %check_clang_tidy %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fms-compatibility -DEXTERNINLINE
+//
+// With -fno-ms-compatiblity, DEXTERNINLINE causes additional output.
+// (The leading ',' means "default checks in addition to NOMSCOMPAT checks.)
+// RUN: %check_clang_tidy -check-suffix=,NOMSCOMPAT \
+// RUN:   %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fno-ms-compatibility -DEXTERNINLINE
 
 extern int Xyz;
 extern int Xyz; // Xyz
@@ -74,6 +89,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
 // CHECK-FIXES: {{^}}// g{{$}}
 
+#if defined(EXTERNINLINE)
 extern inline void g(); // extern g
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant 'g' declaration
-// CHECK-FIXES: {{^}}// extern g{{$}}
+// CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' declaration
+// CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

2019-10-08 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman requested changes to this revision.
arphaman added a comment.
This revision now requires changes to proceed.

@kousikk Thanks, I understand your patch better now. It makes more sense for 
sure.

When we're opening the file we shouldn't `stat` before calling `open`, as 
there's a race condition introduced, where the value of the `stat` could change 
between the call between `stat` and `open` is performed. We've seen problems 
like this before, and it ends up in crashes and mismatch size errors as Clang 
is getting invalid size from the stat if the file is modified in that time. We 
should still call `open` + `fstat` like we used. So I would recommend not 
changing the `createFileEntry` function to take in a stat, and do the fstat 
after opening the file like it used to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68193



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


[PATCH] D68634: [ASTImporter] Imported FunctionDecls inherit attributes from existing functions

2019-10-08 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:7842
+// This implementation is inspired by Sema::mergeDeclAttributes.
+void ASTNodeImporter::CopyInheritedAttributes(FunctionDecl *Old,
+  FunctionDecl *New) {

There are other attributes that [apply to functions as 
well](https://en.cppreference.com/w/cpp/language/attributes): `nodiscard`, 
`maybe_unused` and `deprecated`. Is there a reason not to support those as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68634



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


[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you for this!


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

https://reviews.llvm.org/D68640



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


[clang-tools-extra] r374103 - Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Nico Weber via cfe-commits
Author: nico
Date: Tue Oct  8 12:14:34 2019
New Revision: 374103

URL: http://llvm.org/viewvc/llvm-project?rev=374103=rev
Log:
Try to get readability-deleted-default.cpp to pass on Windows.

In MS compatibility mode, "extern inline void g()" is not a redundant
declaration for "inline void g()", because of redeclForcesDefMSVC()
(see PR19264, r205485).

To fix, run the test with -fms-compatiblity forced on and off
and explicit check for the differing behavior for extern inline.

Final bit of PR43593.

Differential Revision: https://reviews.llvm.org/D68640

Modified:

clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp

Modified: 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp?rev=374103=374102=374103=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp 
Tue Oct  8 12:14:34 2019
@@ -2,6 +2,21 @@
 // RUN:   -config="{CheckOptions: \
 // RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
 // RUN:   value: 0}]}"
+//
+// With -fms-compatibility and -DEXTERNINLINE, the extern inline shouldn't
+// produce additional diagnostics, so same check suffix as before:
+// RUN: %check_clang_tidy %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fms-compatibility -DEXTERNINLINE
+//
+// With -fno-ms-compatiblity, DEXTERNINLINE causes additional output.
+// (The leading ',' means "default checks in addition to NOMSCOMPAT checks.)
+// RUN: %check_clang_tidy -check-suffix=,NOMSCOMPAT \
+// RUN:   %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fno-ms-compatibility -DEXTERNINLINE
 
 extern int Xyz;
 extern int Xyz; // Xyz
@@ -74,6 +89,8 @@ inline void g(); // g
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
 // CHECK-FIXES: {{^}}// g{{$}}
 
+#if defined(EXTERNINLINE)
 extern inline void g(); // extern g
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant 'g' declaration
-// CHECK-FIXES: {{^}}// extern g{{$}}
+// CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' 
declaration
+// CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
+#endif


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


Re: r374098 - [NFC] Attempt to make ubsan-blacklist-vfs test pass on Windows

2019-10-08 Thread Nico Weber via cfe-commits
Almost: http://45.33.8.238/win/78/step_6.txt

C:\src\llvm-project\clang\test\CodeGen\ubsan-blacklist-vfs.c:11:25: error:
INVALID-MAPPED-FILE: expected string not found in input
// INVALID-MAPPED-FILE: invalid-virtual-file.blacklist': No such file or
directory
^
:1:1: note: scanning from here
fatal error: error in backend: can't open file
'C:/src/llvm-project/out/gn/obj/clang/test/CodeGen/Output/invalid-virtual-file.blacklist':
no such file or directory


Looks like it expects a "No such file" with a capital "No" but gets a "no
such file" with a lower-case n. {{[Nn]}}o probably fixes this.

On Tue, Oct 8, 2019 at 2:10 PM Jan Korous via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: jkorous
> Date: Tue Oct  8 11:13:04 2019
> New Revision: 374098
>
> URL: http://llvm.org/viewvc/llvm-project?rev=374098=rev
> Log:
> [NFC] Attempt to make ubsan-blacklist-vfs test pass on Windows
>
> Previously disabled in d0c2d5daa3e
>
> Modified:
> cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c
>
> Modified: cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c?rev=374098=374097=374098=diff
>
> ==
> --- cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c (original)
> +++ cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c Tue Oct  8 11:13:04 2019
> @@ -1,5 +1,3 @@
> -// UNSUPPORTED: system-windows
> -
>  // Verify ubsan doesn't emit checks for blacklisted functions and files
>  // RUN: echo "fun:hash" > %t-func.blacklist
>  // RUN: echo "src:%s" | sed -e 's/\\//g' > %t-file.blacklist
> @@ -7,9 +5,9 @@
>  // RUN: rm -f %t-vfsoverlay.yaml
>  // RUN: rm -f %t-nonexistent.blacklist
>  // RUN: sed -e "s|@DIR@|%/T|g"
> %S/Inputs/sanitizer-blacklist-vfsoverlay.yaml | sed -e 
> "s|@REAL_FILE@|%/t-func.blacklist|g"
> | sed -e "s|@NONEXISTENT_FILE@|%/t-nonexistent.blacklist|g" >
> %t-vfsoverlay.yaml
> -// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay
> %t-vfsoverlay.yaml -fsanitize-blacklist=%T/only-virtual-file.blacklist
> -emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
> +// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay
> %t-vfsoverlay.yaml -fsanitize-blacklist=%/T/only-virtual-file.blacklist
> -emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
>
> -// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay
> %t-vfsoverlay.yaml -fsanitize-blacklist=%T/invalid-virtual-file.blacklist
> -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID-MAPPED-FILE
> +// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay
> %t-vfsoverlay.yaml -fsanitize-blacklist=%/T/invalid-virtual-file.blacklist
> -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID-MAPPED-FILE
>  // INVALID-MAPPED-FILE: invalid-virtual-file.blacklist': No such file or
> directory
>
>  // RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay
> %t-vfsoverlay.yaml -fsanitize-blacklist=%t-nonexistent.blacklist -emit-llvm
> %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis updated this revision to Diff 223917.
thakis edited the summary of this revision.
thakis added a comment.

make test more explicit


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

https://reviews.llvm.org/D68640

Files:
  clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp


Index: clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
+++ clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
@@ -2,6 +2,21 @@
 // RUN:   -config="{CheckOptions: \
 // RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
 // RUN:   value: 0}]}"
+//
+// With -fms-compatibility and -DEXTERNINLINE, the extern inline shouldn't
+// produce additional diagnostics, so same check suffix as before:
+// RUN: %check_clang_tidy %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fms-compatibility -DEXTERNINLINE
+//
+// With -fno-ms-compatiblity, DEXTERNINLINE causes additional output.
+// (The leading ',' means "default checks in addition to NOMSCOMPAT checks.)
+// RUN: %check_clang_tidy -check-suffix=,NOMSCOMPAT \
+// RUN:   %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fno-ms-compatibility -DEXTERNINLINE
 
 extern int Xyz;
 extern int Xyz; // Xyz
@@ -74,6 +89,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
 // CHECK-FIXES: {{^}}// g{{$}}
 
+#if defined(EXTERNINLINE)
 extern inline void g(); // extern g
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant 'g' declaration
-// CHECK-FIXES: {{^}}// extern g{{$}}
+// CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' 
declaration
+// CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
+#endif


Index: clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
===
--- clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
+++ clang-tools-extra/test/clang-tidy/readability-redundant-declaration.cpp
@@ -2,6 +2,21 @@
 // RUN:   -config="{CheckOptions: \
 // RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
 // RUN:   value: 0}]}"
+//
+// With -fms-compatibility and -DEXTERNINLINE, the extern inline shouldn't
+// produce additional diagnostics, so same check suffix as before:
+// RUN: %check_clang_tidy %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fms-compatibility -DEXTERNINLINE
+//
+// With -fno-ms-compatiblity, DEXTERNINLINE causes additional output.
+// (The leading ',' means "default checks in addition to NOMSCOMPAT checks.)
+// RUN: %check_clang_tidy -check-suffix=,NOMSCOMPAT \
+// RUN:   %s readability-redundant-declaration %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-declaration.IgnoreMacros, \
+// RUN:   value: 0}]}" -- -fno-ms-compatibility -DEXTERNINLINE
 
 extern int Xyz;
 extern int Xyz; // Xyz
@@ -74,6 +89,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant 'g' declaration
 // CHECK-FIXES: {{^}}// g{{$}}
 
+#if defined(EXTERNINLINE)
 extern inline void g(); // extern g
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: redundant 'g' declaration
-// CHECK-FIXES: {{^}}// extern g{{$}}
+// CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' declaration
+// CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68587: [hip] Assume host-only compilation if the final phase is ahead of `backend`.

2019-10-08 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

TL; DR; 
+1 to formalizing how we want -M*/-E/-S/-emit-llvm/-fsyntax-only to behave. 
OK with -M/-E/-S defaulting to host, and erroring out if applied to multiple 
sub-compilations.
I'm still convinced that the tooling issue with multiple subcompilations is 
orthogonal to this change and should be handled in libclang and that 
-fsyntax-only should not default to one sub-compilation.

See the details below.

In D68587#1698777 , @hliao wrote:

> In D68587#1698247 , @tra wrote:
>
> > In D68587#1698102 , @hliao wrote:
> >
> > > for most compilation tools, single input and single output are expected. 
> > > Without assuming `-fsyntax-only` alone is host-compilation only, that at 
> > > least run syntax checking twice.
> >
> >
> > I believe the driver will not run subsequent jobs if one of the device 
> > compilations fails. You may see duplicate warnings from multiple stages, 
> > but overall the error handling works in a fairly predictable way now.
>
>
> It still runs and gives the same error (if that error is applicable to both 
> sides) at least twice if you just specify `-fsyntax-only` or `-E`. That won't 
> happen for regular compilation option (`-c`) due to the additional device 
> dependencies added.


Are you talking about the behavior with or without  this patch?

Without the patch I do see the driver stopping as soon as one of the 
subcompilations errors out and it needs *all* subcompilations to succeed in 
order for the whole invocation to succeed.

E.g.: err.cu:

  common error;
  
  #if __CUDA_ARCH__
  __device__ void foo() {
device error;
  }
  #else
  __host__ void bar() {
host error;
  }
  #endif



  $ bin/clang --cuda-path=$HOME/local/cuda-10.0 --cuda-gpu-arch=sm_30 
-fsyntax-only err.cu
  err.cu:1:1: error: unknown type name 'common'
  common error;
  ^
  err.cu:9:3: error: unknown type name 'host'
host error;
^
  2 errors generated when compiling for host.

If I comment out the host and common errors, then clang finishes host 
compilation, moves on to device-side compilation and reports the error there:

  err.cu:5:3: error: unknown type name 'device'; did you mean 'CUdevice'?
device error;
^~
CUdevice
  /usr/local/google/home/tra/local/cuda-10.0/include/cuda.h:252:13: note: 
'CUdevice' declared here
  typedef int CUdevice; /**< CUDA device */
  ^
  1 error generated when compiling for sm_30.

Again, it reports that there's an error on device side. My understanding is 
that with this patch clang would've succeeded, which is, IMO, incorrect.

> The error itself is, in fact, should be clear enough, the most confusing part 
> is the diagnostic message and suggestions from clang as host- and device-side 
> compilations are quite different, especially the error message may be mixed 
> with other-side the normal output.

Mixing errors and output will be confusing no matter what you do. That's why 
diags go to stderr by default, so you can separate them from the regular output.
As far as errors go, clang clearly labels which side of the compilation 
produced them:

`1 error generated when compiling for sm_30.`

>> To think of it, I'm starting to doubt that this patch is an improvement for 
>> `-M` either. You will get the dependencies for the host, but they are not 
>> necessarily the same as the dependencies for the device-side compilation. 
>> Producing a partial list of dependencies will be potentially incorrect. IMO 
>> we do need dependencies info from all sub-compilations.
> 
> Even without this patch, `-M` or more specifically `-MD` already breaks now 
> as we just run the dependency generation action twice for each side. The 
> later will overwrite the former *.d file. We need special handling of `-M` to 
> match nvcc.

This sounds like a bug to me. We should've refused to write multiple files -- 
similar to how we refuse to preprocess if we have more than one sub-compilation.
OK. In this sense defaulting to host-only here would be a marginal improvement. 
Until we can produce complete dependency info for all sub-compilations letting 
user specifically select one (with host being default) is probably the best 
choice we can make at the moment.

> 
> 
>> Perhaps we should limit the scope of this patch to -E only for now?
> 
> Just found nvcc's `-E` returns the output of the device-side compilation for 
> the first GPU arch specified. Anyway, whether to match that behavior is just 
> another question.

NVCC is not a good guideline here. Considering that they do source code 
transformation, it's not quite clear what exactly -E means for NVCC at all.
Defaulting to host-side -E would probably be fine as long as user can override 
it with --cuda-device-only.

> but some tools, like clang-tidy, may be found difficult to insert that option 
> properly, says `clang-tidy -p` 

[PATCH] D68099: [MS ABI]: Fix mangling function arguments for template types to be compatible with MSVC

2019-10-08 Thread Adam Folwarczny via Phabricator via cfe-commits
adamf added a comment.

@thakis Yes please, I cannot commit this patch by myself, because I don't have 
commit access.
Surely, next time I will add more context in diff.
Thanks a lot for the effort.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68099



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


[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

> Can you clarify what exactly the TODO is? As-is, the check suggests removing 
> the redeclaration where it's a no-op (non-ms) but not where it isn't (C, ms 
> compat). If I understand your reply correctly, this is desired behavior. Is 
> the TODO then to have test coverage for the ms compat case? If so: Can 
> clang-tidy checks have different CHECK-MESSAGES suffixes in the same file? If 
> so, we can just run the test once with -fms-compat and once with 
> -fno-ms-compat and expect the diag in one case and not in the other.

To answer my question: 
https://clang.llvm.org/extra/clang-tidy/Contributing.html#testing-checks 
explains how to do this. I'll update the patch.


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

https://reviews.llvm.org/D68640



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


[PATCH] D68099: [MS ABI]: Fix mangling function arguments for template types to be compatible with MSVC

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision.
thakis added a comment.

lgtm. I finally got around to understanding the patch. Sorry for the delay, 
thanks for the fix! Next time you upload a patch, please upload it with more 
context (see 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

Note to self: The mangleSourceName() call mentioned in the patch description is 
14 lines further up. `mangleSourceName(TemplateMangling);` outputs the number 
if there's a backref, else it outputs the argument followed by a '@'. Since we 
only cache mangleSourceName()'s argument, we need to append '@' ourselves. It 
seems a tiny bit nicer to store the '@' in the cache, but as-is is fine too.

Do you need someone to commit this for you?


Repository:
  rC Clang

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

https://reviews.llvm.org/D68099



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


[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-10-08 Thread Melanie Blower via Phabricator via cfe-commits
mibintc updated this revision to Diff 223911.
mibintc added a comment.

clean up some dead code


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fast-math.c
  llvm/include/llvm/Target/TargetOptions.h

Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -107,7 +107,7 @@
   public:
 TargetOptions()
 : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
-  NoNaNsFPMath(false), NoTrappingFPMath(false),
+  NoNaNsFPMath(false), NoTrappingFPMath(true), RoundingFPMath(false),
   NoSignedZerosFPMath(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
@@ -154,6 +154,11 @@
 /// specifies that there are no trap handlers to handle exceptions.
 unsigned NoTrappingFPMath : 1;
 
+/// RoundingFPMath - This flag is enabled when the
+/// -enable-rounding-fp-math is specified on the command line. This
+/// specifies dynamic rounding mode.
+unsigned RoundingFPMath : 1;
+
 /// NoSignedZerosFPMath - This flag is enabled when the
 /// -enable-no-signed-zeros-fp-math is specified on the command line. This
 /// specifies that optimizations are allowed to treat the sign of a zero
Index: clang/test/Driver/fast-math.c
===
--- clang/test/Driver/fast-math.c
+++ clang/test/Driver/fast-math.c
@@ -170,11 +170,11 @@
 // RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -ffinite-math-only \
-// RUN: -fno-math-errno -ffp-contract=fast -c %s 2>&1 \
+// RUN: -fno-math-errno -ffp-contract=fast -fno-rounding-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -fno-honor-infinities -fno-honor-nans -fno-math-errno \
 // RUN: -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN: -fno-trapping-math -ffp-contract=fast -c %s 2>&1 \
+// RUN: -fno-trapping-math -ffp-contract=fast -fno-rounding-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // CHECK-FAST-MATH: "-cc1"
 // CHECK-FAST-MATH: "-ffast-math"
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -320,7 +320,6 @@
 // RUN: -fprefetch-loop-arrays\
 // RUN: -fprofile-correction  \
 // RUN: -fprofile-values  \
-// RUN: -frounding-math   \
 // RUN: -fschedule-insns  \
 // RUN: -fsignaling-nans  \
 // RUN: -fstrength-reduce \
@@ -385,7 +384,6 @@
 // CHECK-WARNING-DAG: optimization flag '-fprefetch-loop-arrays' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fprofile-correction' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fprofile-values' is not supported
-// CHECK-WARNING-DAG: optimization flag '-frounding-math' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fschedule-insns' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fsignaling-nans' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fstrength-reduce' is not supported
Index: clang/test/CodeGen/fpconstrained.c
===
--- /dev/null
+++ clang/test/CodeGen/fpconstrained.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -ftrapping-math -frounding-math -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=FPMODELSTRICT
+// RUN: %clang_cc1 -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s -check-prefix=PRECISE
+// RUN: %clang_cc1 -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s -check-prefix=FAST
+// RUN: %clang_cc1 -ffast-math -emit-llvm -o - %s | FileCheck %s -check-prefix=FAST
+// RUN: %clang_cc1 -ffast-math -ffp-contract=fast -ffp-exception-behavior=ignore -emit-llvm -o - %s | 

[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-10-08 Thread Melanie Blower via Phabricator via cfe-commits
mibintc marked 2 inline comments as done.
mibintc added inline comments.



Comment at: clang/include/clang/Driver/Options.td:927
 def fdenormal_fp_math_EQ : Joined<["-"], "fdenormal-fp-math=">, 
Group, Flags<[CC1Option]>;
+def ffp_model_EQ : Joined<["-"], "ffp-model=">, Group, 
Flags<[DriverOption]>,
+  HelpText<"Controls the semantics of floating-point calculations.">;

The ffp-model= option is just  a Driver option, it is rewritten into 
combinations of lower level options like ffp-exception-behavior and 
frounding-math: it's not a cc1 option.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2326
   bool SignedZeros = true;
-  bool TrappingMath = true;
+  bool TrappingMath = false;
+  bool RoundingFPMath = false;

By default, floating point exceptions are masked. Previously this was set to 
true, but the value wasn't used. This patch implements support for trapping-math


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-10-08 Thread Melanie Blower via Phabricator via cfe-commits
mibintc marked 2 inline comments as done.
mibintc added inline comments.



Comment at: clang/docs/UsersManual.rst:1341
+   has been selected, then the compiler will issue a diagnostic warning
+   that the override has occurred.
+

rjmccall wrote:
> That's not typical driver behavior; why this choice?
The rationale for the warnings is that the floating point options are 
sufficiently complicated that it makes sense to warn the uses that one of the 
later options supplied on the command line is undoing a choice made earlier.  
It's not obvious that e.g. the setting for fassociative-math is also controlled 
by  -fp-model=strict


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D68099: [MS ABI]: Fix mangling function arguments for template types to be compatible with MSVC

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

(It regressed in D62780 ; I had bisected that 
bit.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D68099



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


[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

In D68640#1699628 , @aaron.ballman 
wrote:

> In D68640#1699605 , @thakis wrote:
>
> > In D68640#1699563 , @gribozavr 
> > wrote:
> >
> > > It looks to me that a better fix is to fix the checker to not emit this 
> > > warning in MS compatibility mode.
> >
> >
> > The warning is about emitting "here's a redundant declaration", and the 
> > test expects an "extern inline" decl to be redundant with an "inline" 
> > definition. In ms compat mode they aren't, so the checker does not emit the 
> > warning in ms mode (but does elsewhere).
> >
> > Arguably having a check that suggests removing a bunch of code that's 
> > necessary in some modes (ms compat, or C) is a bit weird, so maybe we 
> > should never emit this diag for extern inlines. I don't know which policy 
> > decisions clang-tidy usually makes for cross-platform development – does it 
> > prioritize cross-platform dev, or completeness assuming single-platform dev?
>
>
> I think it depends on the check, but for a check in the `readability` module, 
> I'm not certain there's a clear answer. My gut feeling is to diagnose based 
> on platform behavior because the goal is to remove redundancy and that 
> requires platform-specific knowledge. But it's not a strong opinion.
>
> > (Finally, these tests have been broken for months, folks are landing lots 
> > of stuff with "UNSUPPORTED: win32" (clang VFS patches recently, for 
> > example) and we're struggling just keeping tests green on Windows. There's 
> > no shortage of possible implicit TODOs :) I think it's better to land this 
> > to get the check-clang-tools target green than it is to mark the test 
> > UNSUPPORTED.)
>
> To be clear, I wasn't suggesting we fix it in this patch, just that we add a 
> FIXME/TODO to the test and make sure we're tracking the bug. Explicit TODO > 
> implicit TODO.


Can you clarify what exactly the TODO is? As-is, the check suggests removing 
the redeclaration where it's a no-op (non-ms) but not where it isn't (C, ms 
compat). If I understand your reply correctly, this is desired behavior. Is the 
TODO then to have test coverage for the ms compat case? If so: Can clang-tidy 
checks have different CHECK-MESSAGES suffixes in the same file? If so, we can 
just run the test once with -fms-compat and once with -fno-ms-compat and expect 
the diag in one case and not in the other.


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

https://reviews.llvm.org/D68640



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


[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-10-08 Thread Melanie Blower via Phabricator via cfe-commits
mibintc updated this revision to Diff 223908.
mibintc added a comment.

I made a couple wording changes suggested by @rjmccall


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fast-math.c
  llvm/include/llvm/Target/TargetOptions.h

Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -107,7 +107,7 @@
   public:
 TargetOptions()
 : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
-  NoNaNsFPMath(false), NoTrappingFPMath(false),
+  NoNaNsFPMath(false), NoTrappingFPMath(true), RoundingFPMath(false),
   NoSignedZerosFPMath(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
@@ -154,6 +154,11 @@
 /// specifies that there are no trap handlers to handle exceptions.
 unsigned NoTrappingFPMath : 1;
 
+/// RoundingFPMath - This flag is enabled when the
+/// -enable-rounding-fp-math is specified on the command line. This
+/// specifies dynamic rounding mode.
+unsigned RoundingFPMath : 1;
+
 /// NoSignedZerosFPMath - This flag is enabled when the
 /// -enable-no-signed-zeros-fp-math is specified on the command line. This
 /// specifies that optimizations are allowed to treat the sign of a zero
Index: clang/test/Driver/fast-math.c
===
--- clang/test/Driver/fast-math.c
+++ clang/test/Driver/fast-math.c
@@ -170,11 +170,11 @@
 // RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -ffinite-math-only \
-// RUN: -fno-math-errno -ffp-contract=fast -c %s 2>&1 \
+// RUN: -fno-math-errno -ffp-contract=fast -fno-rounding-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -fno-honor-infinities -fno-honor-nans -fno-math-errno \
 // RUN: -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN: -fno-trapping-math -ffp-contract=fast -c %s 2>&1 \
+// RUN: -fno-trapping-math -ffp-contract=fast -fno-rounding-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // CHECK-FAST-MATH: "-cc1"
 // CHECK-FAST-MATH: "-ffast-math"
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -320,7 +320,6 @@
 // RUN: -fprefetch-loop-arrays\
 // RUN: -fprofile-correction  \
 // RUN: -fprofile-values  \
-// RUN: -frounding-math   \
 // RUN: -fschedule-insns  \
 // RUN: -fsignaling-nans  \
 // RUN: -fstrength-reduce \
@@ -385,7 +384,6 @@
 // CHECK-WARNING-DAG: optimization flag '-fprefetch-loop-arrays' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fprofile-correction' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fprofile-values' is not supported
-// CHECK-WARNING-DAG: optimization flag '-frounding-math' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fschedule-insns' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fsignaling-nans' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fstrength-reduce' is not supported
Index: clang/test/CodeGen/fpconstrained.c
===
--- /dev/null
+++ clang/test/CodeGen/fpconstrained.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -ftrapping-math -frounding-math -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=FPMODELSTRICT
+// RUN: %clang_cc1 -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s -check-prefix=PRECISE
+// RUN: %clang_cc1 -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s -check-prefix=FAST
+// RUN: %clang_cc1 -ffast-math -emit-llvm -o - %s | FileCheck %s -check-prefix=FAST
+// RUN: %clang_cc1 -ffast-math -ffp-contract=fast 

r374099 - [BPF] do compile-once run-everywhere relocation for bitfields

2019-10-08 Thread Yonghong Song via cfe-commits
Author: yhs
Date: Tue Oct  8 11:23:17 2019
New Revision: 374099

URL: http://llvm.org/viewvc/llvm-project?rev=374099=rev
Log:
[BPF] do compile-once run-everywhere relocation for bitfields

A bpf specific clang intrinsic is introduced:
   u32 __builtin_preserve_field_info(member_access, info_kind)
Depending on info_kind, different information will
be returned to the program. A relocation is also
recorded for this builtin so that bpf loader can
patch the instruction on the target host.
This clang intrinsic is used to get certain information
to facilitate struct/union member relocations.

The offset relocation is extended by 4 bytes to
include relocation kind.
Currently supported relocation kinds are
 enum {
FIELD_BYTE_OFFSET = 0,
FIELD_BYTE_SIZE,
FIELD_EXISTENCE,
FIELD_SIGNEDNESS,
FIELD_LSHIFT_U64,
FIELD_RSHIFT_U64,
 };
for __builtin_preserve_field_info. The old
access offset relocation is covered by
FIELD_BYTE_OFFSET = 0.

An example:
struct s {
int a;
int b1:9;
int b2:4;
};
enum {
FIELD_BYTE_OFFSET = 0,
FIELD_BYTE_SIZE,
FIELD_EXISTENCE,
FIELD_SIGNEDNESS,
FIELD_LSHIFT_U64,
FIELD_RSHIFT_U64,
};

void bpf_probe_read(void *, unsigned, const void *);
int field_read(struct s *arg) {
  unsigned long long ull = 0;
  unsigned offset = __builtin_preserve_field_info(arg->b2, FIELD_BYTE_OFFSET);
  unsigned size = __builtin_preserve_field_info(arg->b2, FIELD_BYTE_SIZE);
 #ifdef USE_PROBE_READ
  bpf_probe_read(, size, (const void *)arg + offset);
  unsigned lshift = __builtin_preserve_field_info(arg->b2, FIELD_LSHIFT_U64);
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
  lshift = lshift + (size << 3) - 64;
 #endif
 #else
  switch(size) {
  case 1:
ull = *(unsigned char *)((void *)arg + offset); break;
  case 2:
ull = *(unsigned short *)((void *)arg + offset); break;
  case 4:
ull = *(unsigned int *)((void *)arg + offset); break;
  case 8:
ull = *(unsigned long long *)((void *)arg + offset); break;
  }
  unsigned lshift = __builtin_preserve_field_info(arg->b2, FIELD_LSHIFT_U64);
 #endif
  ull <<= lshift;
  if (__builtin_preserve_field_info(arg->b2, FIELD_SIGNEDNESS))
return (long long)ull >> __builtin_preserve_field_info(arg->b2, 
FIELD_RSHIFT_U64);
  return ull >> __builtin_preserve_field_info(arg->b2, FIELD_RSHIFT_U64);
}

There is a minor overhead for bpf_probe_read() on big endian.

The code and relocation generated for field_read where bpf_probe_read() is
used to access argument data on little endian mode:
r3 = r1
r1 = 0
r1 = 4  <=== relocation (FIELD_BYTE_OFFSET)
r3 += r1
r1 = r10
r1 += -8
r2 = 4  <=== relocation (FIELD_BYTE_SIZE)
call bpf_probe_read
r2 = 51 <=== relocation (FIELD_LSHIFT_U64)
r1 = *(u64 *)(r10 - 8)
r1 <<= r2
r2 = 60 <=== relocation (FIELD_RSHIFT_U64)
r0 = r1
r0 >>= r2
r3 = 1  <=== relocation (FIELD_SIGNEDNESS)
if r3 == 0 goto LBB0_2
r1 s>>= r2
r0 = r1
LBB0_2:
exit

Compare to the above code between relocations FIELD_LSHIFT_U64 and
FIELD_LSHIFT_U64, the code with big endian mode has four more
instructions.
r1 = 41   <=== relocation (FIELD_LSHIFT_U64)
r6 += r1
r6 += -64
r6 <<= 32
r6 >>= 32
r1 = *(u64 *)(r10 - 8)
r1 <<= r6
r2 = 60   <=== relocation (FIELD_RSHIFT_U64)

The code and relocation generated when using direct load.
r2 = 0
r3 = 4
r4 = 4
if r4 s> 3 goto LBB0_3
if r4 == 1 goto LBB0_5
if r4 == 2 goto LBB0_6
goto LBB0_9
LBB0_6: # %sw.bb1
r1 += r3
r2 = *(u16 *)(r1 + 0)
goto LBB0_9
LBB0_3: # %entry
if r4 == 4 goto LBB0_7
if r4 == 8 goto LBB0_8
goto LBB0_9
LBB0_8: # %sw.bb9
r1 += r3
r2 = *(u64 *)(r1 + 0)
goto LBB0_9
LBB0_5: # %sw.bb
r1 += r3
r2 = *(u8 *)(r1 + 0)
goto LBB0_9
LBB0_7: # %sw.bb5
r1 += r3
r2 = *(u32 *)(r1 + 0)
LBB0_9: # %sw.epilog
r1 = 51
r2 <<= r1
r1 = 60
r0 = r2
r0 >>= r1
r3 = 1
if r3 == 0 goto LBB0_11
r2 s>>= r1
r0 = r2
LBB0_11:# %sw.epilog
exit

Considering verifier is able to do limited constant
propogation following branches. The following is the
code actually traversed.
r2 = 0
r3 = 4   <=== relocation
r4 = 4   <=== relocation
if r4 s> 3 goto LBB0_3
LBB0_3: # %entry
if r4 == 4 goto LBB0_7
LBB0_7: # %sw.bb5
r1 += r3
r2 = *(u32 *)(r1 + 0)
LBB0_9: # %sw.epilog
r1 = 51   

[PATCH] D68531: [WebAssembly] Add builtin and intrinsic for v8x16.swizzle

2019-10-08 Thread Thomas Lively via Phabricator via cfe-commits
tlively marked an inline comment as done.
tlively added a comment.

In D68531#1699855 , @aheejin wrote:

> > LLVM produces a poison value if the dynamic swizzle indices are greater 
> > than the vector size, but the WebAssembly instruction sets the 
> > corresponding output lane to zero.
>
> Where do we set those undef or poison lanes to zero?


We don't do that in the toolchain. It's the runtime semantics implemented in 
engines that set the output lanes to zero.




Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:63
 // SIMD builtins
+TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", 
"unimplemented-simd128")
+

aheejin wrote:
> Is the second indices vector always v8x16 too?
Yes, that's the only version of swizzling we have for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68531



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


r374098 - [NFC] Attempt to make ubsan-blacklist-vfs test pass on Windows

2019-10-08 Thread Jan Korous via cfe-commits
Author: jkorous
Date: Tue Oct  8 11:13:04 2019
New Revision: 374098

URL: http://llvm.org/viewvc/llvm-project?rev=374098=rev
Log:
[NFC] Attempt to make ubsan-blacklist-vfs test pass on Windows

Previously disabled in d0c2d5daa3e

Modified:
cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c

Modified: cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c?rev=374098=374097=374098=diff
==
--- cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c (original)
+++ cfe/trunk/test/CodeGen/ubsan-blacklist-vfs.c Tue Oct  8 11:13:04 2019
@@ -1,5 +1,3 @@
-// UNSUPPORTED: system-windows
-
 // Verify ubsan doesn't emit checks for blacklisted functions and files
 // RUN: echo "fun:hash" > %t-func.blacklist
 // RUN: echo "src:%s" | sed -e 's/\\//g' > %t-file.blacklist
@@ -7,9 +5,9 @@
 // RUN: rm -f %t-vfsoverlay.yaml
 // RUN: rm -f %t-nonexistent.blacklist
 // RUN: sed -e "s|@DIR@|%/T|g" %S/Inputs/sanitizer-blacklist-vfsoverlay.yaml | 
sed -e "s|@REAL_FILE@|%/t-func.blacklist|g" | sed -e 
"s|@NONEXISTENT_FILE@|%/t-nonexistent.blacklist|g" > %t-vfsoverlay.yaml
-// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%T/only-virtual-file.blacklist 
-emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
+// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%/T/only-virtual-file.blacklist 
-emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
 
-// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%T/invalid-virtual-file.blacklist 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID-MAPPED-FILE
+// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%/T/invalid-virtual-file.blacklist 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID-MAPPED-FILE
 // INVALID-MAPPED-FILE: invalid-virtual-file.blacklist': No such file or 
directory
 
 // RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay 
%t-vfsoverlay.yaml -fsanitize-blacklist=%t-nonexistent.blacklist -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=INVALID


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


[PATCH] D68652: [driver][hip] Skip bundler if host action is nothing.

2019-10-08 Thread Michael Liao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6334a59454ef: [driver][hip] Skip bundler if host action is 
nothing. (authored by hliao).

Changed prior to commit:
  https://reviews.llvm.org/D68652?vs=223890=223905#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68652

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-syntax-only.hip


Index: clang/test/Driver/hip-syntax-only.hip
===
--- /dev/null
+++ clang/test/Driver/hip-syntax-only.hip
@@ -0,0 +1,11 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -x hip -nogpulib -target x86_64 -fsyntax-only %s 2>&1 | 
FileCheck %s
+
+// Check that there are commands for both host- and device-side compilations.
+//
+// CHECK-DAG: clang{{.*}}" "-cc1" {{.*}} "-fcuda-is-device"
+// CHECK-DAG: clang{{.*}}" "-cc1" "-triple" "x86_64"
+// CHECK-NOT: clang-offload-bundler"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3108,7 +3108,8 @@
 // the resulting list. Otherwise, just append the device actions. For
 // device only compilation, HostAction is a null pointer, therefore only do
 // this when HostAction is not a null pointer.
-if (CanUseBundler && HostAction && !OffloadAL.empty()) {
+if (CanUseBundler && HostAction &&
+HostAction->getType() != types::TY_Nothing && !OffloadAL.empty()) {
   // Add the host action to the list in order to create the bundling 
action.
   OffloadAL.push_back(HostAction);
 


Index: clang/test/Driver/hip-syntax-only.hip
===
--- /dev/null
+++ clang/test/Driver/hip-syntax-only.hip
@@ -0,0 +1,11 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -x hip -nogpulib -target x86_64 -fsyntax-only %s 2>&1 | FileCheck %s
+
+// Check that there are commands for both host- and device-side compilations.
+//
+// CHECK-DAG: clang{{.*}}" "-cc1" {{.*}} "-fcuda-is-device"
+// CHECK-DAG: clang{{.*}}" "-cc1" "-triple" "x86_64"
+// CHECK-NOT: clang-offload-bundler"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3108,7 +3108,8 @@
 // the resulting list. Otherwise, just append the device actions. For
 // device only compilation, HostAction is a null pointer, therefore only do
 // this when HostAction is not a null pointer.
-if (CanUseBundler && HostAction && !OffloadAL.empty()) {
+if (CanUseBundler && HostAction &&
+HostAction->getType() != types::TY_Nothing && !OffloadAL.empty()) {
   // Add the host action to the list in order to create the bundling action.
   OffloadAL.push_back(HostAction);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r374097 - [driver][hip] Skip bundler if host action is nothing.

2019-10-08 Thread Michael Liao via cfe-commits
Author: hliao
Date: Tue Oct  8 11:06:51 2019
New Revision: 374097

URL: http://llvm.org/viewvc/llvm-project?rev=374097=rev
Log:
[driver][hip] Skip bundler if host action is nothing.

Reviewers: sfantao, tra, yaxunl

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D68652

Added:
cfe/trunk/test/Driver/hip-syntax-only.hip
Modified:
cfe/trunk/lib/Driver/Driver.cpp

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=374097=374096=374097=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Tue Oct  8 11:06:51 2019
@@ -3108,7 +3108,8 @@ public:
 // the resulting list. Otherwise, just append the device actions. For
 // device only compilation, HostAction is a null pointer, therefore only do
 // this when HostAction is not a null pointer.
-if (CanUseBundler && HostAction && !OffloadAL.empty()) {
+if (CanUseBundler && HostAction &&
+HostAction->getType() != types::TY_Nothing && !OffloadAL.empty()) {
   // Add the host action to the list in order to create the bundling 
action.
   OffloadAL.push_back(HostAction);
 

Added: cfe/trunk/test/Driver/hip-syntax-only.hip
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hip-syntax-only.hip?rev=374097=auto
==
--- cfe/trunk/test/Driver/hip-syntax-only.hip (added)
+++ cfe/trunk/test/Driver/hip-syntax-only.hip Tue Oct  8 11:06:51 2019
@@ -0,0 +1,11 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -x hip -nogpulib -target x86_64 -fsyntax-only %s 2>&1 | 
FileCheck %s
+
+// Check that there are commands for both host- and device-side compilations.
+//
+// CHECK-DAG: clang{{.*}}" "-cc1" {{.*}} "-fcuda-is-device"
+// CHECK-DAG: clang{{.*}}" "-cc1" "-triple" "x86_64"
+// CHECK-NOT: clang-offload-bundler"


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


[PATCH] D68652: [driver][hip] Skip bundler if host action is nothing.

2019-10-08 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Driver/hip-syntax-only.hip:7
+
+// CHECK-DAG: clang{{.*}}" "-cc1" {{.*}} "-fcuda-is-device"
+// CHECK-DAG: clang{{.*}}" "-cc1" "-triple" "x86_64"

I'd include `-target ` and a comment describing that we're making sure 
that both host and device compilations are still executed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68652



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


[PATCH] D33029: [clang-format] add option for dangling parenthesis

2019-10-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

I tried the patch and rebased it locally (as there were conflicts with the 
current trunk)

The change caused other tests to fail (which doesn't completely surprise me)

most tests failures look associated with positioning of trailing brace for 
example

  @@ -1,4 +1,4 @@
   void f() {
 bar([]() {} // Did not respect SpacesBeforeTrailingComments
  -  );
  + );
   }

Below is a list of the tests that fail

  [==] 700 tests from 21 test cases ran. (6687 ms total)
  [  PASSED  ] 692 tests.
  [  FAILED  ] 8 tests, listed below:
  [  FAILED  ] FormatTest.WrapsTemplateDeclarations
  [  FAILED  ] FormatTest.BreaksStringLiteralsWithin_TMacro
  [  FAILED  ] FormatTest.FormatsLambdas
  [  FAILED  ] FormatTest.WrappedClosingParenthesisIndent
  [  FAILED  ] FormatTestJS.FunctionParametersTrailingComma
  [  FAILED  ] FormatTestJS.FunctionLiterals
  [  FAILED  ] FormatTestJS.ArrowFunctions
  [  FAILED  ] FormatTestJS.NestedLiterals
  
   8 FAILED TESTS


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

https://reviews.llvm.org/D33029



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


r374093 - [OPENMP50]Do not allow multiple same context traits in the same context

2019-10-08 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Tue Oct  8 10:47:52 2019
New Revision: 374093

URL: http://llvm.org/viewvc/llvm-project?rev=374093=rev
Log:
[OPENMP50]Do not allow multiple same context traits in the same context
selector.

According to OpenMP 5.0, 2.3.2 Context Selectors, Restrictions, each
trait-selector-name can only be specified once. Added check for this
restriction.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/test/OpenMP/declare_variant_ast_print.c
cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp
cfe/trunk/test/OpenMP/declare_variant_messages.c
cfe/trunk/test/OpenMP/declare_variant_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=374093=374092=374093=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Oct  8 10:47:52 
2019
@@ -1215,6 +1215,11 @@ def err_omp_declare_variant_ctx_set_muti
   "context selector set '%0' is used already in the same 'omp declare variant' 
directive">;
 def note_omp_declare_variant_ctx_set_used_here : Note<
   "previously context selector set '%0' used here">;
+def err_omp_expected_comma_brace : Error<"expected '}' or ',' after '%0'">;
+def err_omp_declare_variant_ctx_mutiple_use : Error<
+  "context trait selector '%0' is used already in the same '%1' context 
selector set of 'omp declare variant' directive">;
+def note_omp_declare_variant_ctx_used_here : Note<
+  "previously context trait selector '%0' used here">;
 def warn_omp_more_one_device_type_clause : Warning<
   "more than one 'device_type' clause is specified">,
   InGroup;

Modified: cfe/trunk/lib/Parse/ParseOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseOpenMP.cpp?rev=374093=374092=374093=diff
==
--- cfe/trunk/lib/Parse/ParseOpenMP.cpp (original)
+++ cfe/trunk/lib/Parse/ParseOpenMP.cpp Tue Oct  8 10:47:52 2019
@@ -815,7 +815,7 @@ static ExprResult parseContextScore(Pars
 /// 'vendor' '(' [ 'score' '('  ')' ':' ]  { ','  
}
 /// ')'
 static void parseImplementationSelector(
-Parser , SourceLocation Loc,
+Parser , SourceLocation Loc, llvm::StringMap ,
 llvm::function_ref
 Callback) {
@@ -832,6 +832,15 @@ static void parseImplementationSelector(
   }
   SmallString<16> Buffer;
   StringRef CtxSelectorName = P.getPreprocessor().getSpelling(Tok, Buffer);
+  auto Res = UsedCtx.try_emplace(CtxSelectorName, Tok.getLocation());
+  if (!Res.second) {
+// OpenMP 5.0, 2.3.2 Context Selectors, Restrictions.
+// Each trait-selector-name can only be specified once.
+P.Diag(Tok.getLocation(), diag::err_omp_declare_variant_ctx_mutiple_use)
+<< CtxSelectorName << "implementation";
+P.Diag(Res.first->getValue(), diag::note_omp_declare_variant_ctx_used_here)
+<< CtxSelectorName;
+  }
   OMPDeclareVariantAttr::CtxSelectorType CSKind =
   OMPDeclareVariantAttr::CtxUnknown;
   (void)OMPDeclareVariantAttr::ConvertStrToCtxSelectorType(CtxSelectorName,
@@ -932,17 +941,25 @@ bool Parser::parseOpenMPContextSelectors
   OMPDeclareVariantAttr::CtxSetUnknown;
   (void)OMPDeclareVariantAttr::ConvertStrToCtxSelectorSetType(
   CtxSelectorSetName, CSSKind);
-  switch (CSSKind) {
-  case OMPDeclareVariantAttr::CtxSetImplementation:
-parseImplementationSelector(*this, Loc, Callback);
-break;
-  case OMPDeclareVariantAttr::CtxSetUnknown:
-// Skip until either '}', ')', or end of directive.
-while (!SkipUntil(tok::r_brace, tok::r_paren,
-  tok::annot_pragma_openmp_end, StopBeforeMatch))
-  ;
-break;
-  }
+  llvm::StringMap UsedCtx;
+  do {
+switch (CSSKind) {
+case OMPDeclareVariantAttr::CtxSetImplementation:
+  parseImplementationSelector(*this, Loc, UsedCtx, Callback);
+  break;
+case OMPDeclareVariantAttr::CtxSetUnknown:
+  // Skip until either '}', ')', or end of directive.
+  while (!SkipUntil(tok::r_brace, tok::r_paren,
+tok::annot_pragma_openmp_end, StopBeforeMatch))
+;
+  break;
+}
+const Token PrevTok = Tok;
+if (!TryConsumeToken(tok::comma) && Tok.isNot(tok::r_brace))
+  Diag(Tok, diag::err_omp_expected_comma_brace)
+  << (PrevTok.isAnnotation() ? "context selector trait"
+ : PP.getSpelling(PrevTok));
+  } while (Tok.is(tok::identifier));
   // Parse '}'.
   (void)TBr.consumeClose();
 }

Modified: cfe/trunk/test/OpenMP/declare_variant_ast_print.c
URL: 

[PATCH] D67980: [BPF] do compile-once run-everywhere relocation for bitfields

2019-10-08 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast accepted this revision.
ast added a comment.

perfect. ship it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67980



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


[PATCH] D68562: [clangd] Add RemoveUsingNamespace tweak.

2019-10-08 Thread UTKARSH SAXENA via Phabricator via cfe-commits
usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:99
+return false;
+  if (!dyn_cast(TargetDirective->getDeclContext()))
+return false;

ilya-biryukov wrote:
> I believe this check is redundant in presence of `isTopLevelDecl()`...
> (It used to check the 'using namespace' is not inside a function body)
Aah. Makes sense.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:781
+namespace bb { struct map {}; }
+using namespace bb; // Qualifies this.
+  }

ilya-biryukov wrote:
> Argh, this should definitely be fixed :-(
> One simple way to handle this particular only qualify references, which come 
> **after** the first `using namespace` we are removing.
> 
> There's a `SourceManager::isBeforeInTranslationUnit` function that can be 
> used to find out whether something is written before or after a particular 
> point.
> 
> This won't fix all of the cases, but fixing in the general case seems hard.
We also need to qualify the map in such cases.
I have made an attempt to fix this by traversing all the parents namespace decl 
and checking whether they are equal to the concerned namespace.
But this introduces other problems:
```
  namespace aa { namespace bb { struct map {}; }}
  using namespace aa::bb;
  using namespace a^a;
  int main() {
map m;
  }
```
get changed to 
```
  namespace aa { namespace bb { struct map {}; }}
  using namespace aa::bb;
  
  int main() {
aa::map m;
  }
```
Here aa::map is invalid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68562



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


[PATCH] D68562: [clangd] Add RemoveUsingNamespace tweak.

2019-10-08 Thread UTKARSH SAXENA via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 223898.
usaxena95 marked 8 inline comments as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68562

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -69,7 +69,8 @@
   EXPECT_EQ(apply("^if (true) {return 100;} else {continue;}"),
 "if (true) {continue;} else {return 100;}");
   EXPECT_EQ(apply("^if () {return 100;} else {continue;}"),
-"if () {continue;} else {return 100;}") << "broken condition";
+"if () {continue;} else {return 100;}")
+  << "broken condition";
   EXPECT_AVAILABLE("^i^f^^(^t^r^u^e^) { return 100; } ^e^l^s^e^ { continue; }");
   EXPECT_UNAVAILABLE("if (true) {^return ^100;^ } else { ^continue^;^ }");
   // Available in subexpressions of the condition;
@@ -100,7 +101,7 @@
   EXPECT_UNAVAILABLE(R"cpp(R"(multi )" ^"token " u8"str\ning")cpp"); // nonascii
   EXPECT_UNAVAILABLE(R"cpp(^R^"^(^multi )" "token " "str\ning")cpp"); // raw
   EXPECT_UNAVAILABLE(R"cpp(^"token\n" __FILE__)cpp"); // chunk is macro
-  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp"); // forbidden escape char
+  EXPECT_UNAVAILABLE(R"cpp(^"a\r\n";)cpp");   // forbidden escape char
 
   const char *Input = R"cpp(R"(multi
 token)" "\nst^ring\n" "literal")cpp";
@@ -286,11 +287,11 @@
  void f(int a) {
int y = PLUS([[1+a]]);
  })cpp",
-  /*FIXME: It should be extracted like this.
-   R"cpp(#define PLUS(x) x++
- void f(int a) {
-   auto dummy = 1+a; int y = PLUS(dummy);
- })cpp"},*/
+   /*FIXME: It should be extracted like this.
+R"cpp(#define PLUS(x) x++
+  void f(int a) {
+auto dummy = 1+a; int y = PLUS(dummy);
+  })cpp"},*/
R"cpp(#define PLUS(x) x++
  void f(int a) {
auto dummy = PLUS(1+a); int y = dummy;
@@ -301,13 +302,13 @@
if(1)
 LOOP(5 + [[3]])
  })cpp",
-  /*FIXME: It should be extracted like this. SelectionTree needs to be
-* fixed for macros.
-   R"cpp(#define LOOP(x) while (1) {a = x;}
-   void f(int a) {
- auto dummy = 3; if(1)
-  LOOP(5 + dummy)
-   })cpp"},*/
+   /*FIXME: It should be extracted like this. SelectionTree needs to be
+ * fixed for macros.
+R"cpp(#define LOOP(x) while (1) {a = x;}
+void f(int a) {
+  auto dummy = 3; if(1)
+   LOOP(5 + dummy)
+})cpp"},*/
R"cpp(#define LOOP(x) while (1) {a = x;}
  void f(int a) {
auto dummy = LOOP(5 + 3); if(1)
@@ -403,8 +404,8 @@
  void f() {
auto dummy = S(2) + S(3) + S(4); S x = S(1) + dummy + S(5);
  })cpp"},
-   // Don't try to analyze across macro boundaries
-   // FIXME: it'd be nice to do this someday (in a safe way)
+  // Don't try to analyze across macro boundaries
+  // FIXME: it'd be nice to do this someday (in a safe way)
   {R"cpp(#define ECHO(X) X
  void f() {
int x = 1 + [[ECHO(2 + 3) + 4]] + 5;
@@ -521,7 +522,7 @@
   StartsWith("fail: Could not expand type of lambda expression"));
   // inline namespaces
   EXPECT_EQ(apply("au^to x = inl_ns::Visible();"),
-  "Visible x = inl_ns::Visible();");
+"Visible x = inl_ns::Visible();");
   // local class
   EXPECT_EQ(apply("namespace x { void y() { struct S{}; ^auto z = S(); } }"),
 "namespace x { void y() { struct S{}; S z = S(); } }");
@@ -656,6 +657,214 @@
   EXPECT_THAT(apply(" for(;;) { [[while(1) break; break;]] }"),
   StartsWith("fail"));
 }
+
+TWEAK_TEST(RemoveUsingNamespace);
+TEST_F(RemoveUsingNamespaceTest, All) {
+  std::pair Cases[] = {
+  {// Remove all occurrences of ns. Qualify only unqualified.
+   R"cpp(
+  namespace ns1 { struct vector {}; }
+  namespace ns2 { struct map {}; }
+  using namespace n^s1;
+  using namespace ns2;
+  using namespace ns1;
+  int main() {
+ns1::vector v1;
+vector v2;
+map m1;
+  }
+)cpp",
+   R"cpp(
+  namespace ns1 { struct vector {}; }
+  namespace 

Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message

2019-10-08 Thread Arthur O'Dwyer via cfe-commits
On Mon, Oct 7, 2019 at 6:58 PM Dávid Bolvanský 
wrote:
>> FWIW I found the "always evaluates to 'true'" bit important to
>> understand the warning.
>
> Yeah. I moved this check somewhere else, so we can print precise message:
> r373973 should emit "bitwise negation of a boolean expression always
> evaluates to 'true'; did you mean logical negation?" where possible.
> In the suspicious case like int i = ~b there is a general message
> "bitwise negation of a boolean expression; did you mean logical
> negation?".
>
> I like it now. What do you think? fine for you?

I see. Yes, all the cases I tried produce appropriate diagnostics. I like
it!

Hm, there is no "bitwise negation of a boolean expression always
> evaluates to 'true'; did you mean logical negation?" for chromium
> case [ https://bugs.chromium.org/p/chromium/issues/detail?id=1011810 ].
> I will try to fix it.


The important part there seems to be that the result of `~b` (which must be
either -1 or -2) is used as the operand to `!=` or `==`.
My opinion is that it is not worth the extra complication just to improve
the error message for this case.
It would be interesting to do some kind of *general*-purpose dataflow
before emitting diagnostics...
I notice that Clang's optimizer is smart enough to optimize
bool foo(bool a, bool b) {
return a == ~b;
}
bool bar(int x) {
return x + 1 < -INT_MAX;
}
into `return 0`. If it could propagate that information up and produce a
diagnostic, users might appreciate that. But the challenge as always is
that we can never tell if the user might sometimes be doing that sort of
thing on purpose (in inlined code, in macros, in generated code, etc).

my $.02,
–Arthur
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-10-08 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Thank you, this looks very clean now.




Comment at: clang/docs/UsersManual.rst:1318
+   mode informs the compiler that it must not assume any particular
+   rounding mode.
+

"represent *the* corresponding IEEE rounding rules"



Comment at: clang/docs/UsersManual.rst:1330
+   and ``fast``.
+   Details:
+

"provided by other, single-purpose floating point options."



Comment at: clang/docs/UsersManual.rst:1341
+   has been selected, then the compiler will issue a diagnostic warning
+   that the override has occurred.
+

That's not typical driver behavior; why this choice?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D68652: [driver][hip] Skip bundler if host action is nothing.

2019-10-08 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added reviewers: sfantao, tra, yaxunl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68652

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-syntax-only.hip


Index: clang/test/Driver/hip-syntax-only.hip
===
--- /dev/null
+++ clang/test/Driver/hip-syntax-only.hip
@@ -0,0 +1,9 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -x hip -nogpulib -target x86_64 -fsyntax-only %s 2>&1 | 
FileCheck %s
+
+// CHECK-DAG: clang{{.*}}" "-cc1" {{.*}} "-fcuda-is-device"
+// CHECK-DAG: clang{{.*}}" "-cc1" "-triple" "x86_64"
+// CHECK-NOT: clang-offload-bundler"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3105,7 +3105,8 @@
 // the resulting list. Otherwise, just append the device actions. For
 // device only compilation, HostAction is a null pointer, therefore only do
 // this when HostAction is not a null pointer.
-if (CanUseBundler && HostAction && !OffloadAL.empty()) {
+if (CanUseBundler && HostAction &&
+HostAction->getType() != types::TY_Nothing && !OffloadAL.empty()) {
   // Add the host action to the list in order to create the bundling 
action.
   OffloadAL.push_back(HostAction);
 


Index: clang/test/Driver/hip-syntax-only.hip
===
--- /dev/null
+++ clang/test/Driver/hip-syntax-only.hip
@@ -0,0 +1,9 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -x hip -nogpulib -target x86_64 -fsyntax-only %s 2>&1 | FileCheck %s
+
+// CHECK-DAG: clang{{.*}}" "-cc1" {{.*}} "-fcuda-is-device"
+// CHECK-DAG: clang{{.*}}" "-cc1" "-triple" "x86_64"
+// CHECK-NOT: clang-offload-bundler"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3105,7 +3105,8 @@
 // the resulting list. Otherwise, just append the device actions. For
 // device only compilation, HostAction is a null pointer, therefore only do
 // this when HostAction is not a null pointer.
-if (CanUseBundler && HostAction && !OffloadAL.empty()) {
+if (CanUseBundler && HostAction &&
+HostAction->getType() != types::TY_Nothing && !OffloadAL.empty()) {
   // Add the host action to the list in order to create the bundling action.
   OffloadAL.push_back(HostAction);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68627: [Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize.

2019-10-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/include/clang/Basic/TargetInfo.h:947
 
-  virtual bool validateOutputSize(StringRef /*Constraint*/,
+  virtual bool validateOutputSize(const llvm::StringMap ,
+  StringRef /*Constraint*/,

Other Parameter names are commented out (presumably to avoid an unused param 
warning?).  Do we need to do that for this parameter as well?



Comment at: clang/lib/Sema/SemaStmtAsm.cpp:240
 
+void getFunctionFeatureMap(llvm::StringMap ,
+   DiagnosticsEngine ,

First, this ought to be static.  Second, since this is basically identical to 
what we do for CodeGen, I wonder if this needs to just be a member of 
Sema/ASTContext or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68627



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


[PATCH] D68340: Add AIX toolchain and basic linker functionality

2019-10-08 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:35
+  // Only support 32 and 64 bit
+  if (!IsArch32Bit && !IsArch64Bit)
+llvm_unreachable("Unsupported bit width value");

Is there any reason to use llvm_unreachable here? I think we should use  
'assertion' instead here:

```
assert((IsArch32Bit || IsArch64Bit) && "...");
```



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:54
+  } else {
+assert(Output.isNothing() && "Invalid output.");
+  }

I am not sure, if we compile with assertion off, does this extra 'else' {} have 
any side effect?



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:70
+
+  if (!Args.hasArg(options::OPT_nostdlib)) {
+const char *crt0 = nullptr;

line 38 and line 70 use the same query, should they be put together? Or is 
there any exact order we should follow to push args into 'CmdArgs'?



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:93
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+// Support POSIX threads if "-pthreads" or 
+// "-pthread" is present

One line of comment can be <= 80 characters.



Comment at: clang/lib/Driver/ToolChains/AIX.h:33
+const char *LinkingOutput) const override;
+};
+} // end namespace aix

An extra blank line preferred below.



Comment at: clang/lib/Driver/ToolChains/AIX.h:43
+  const llvm::opt::ArgList );
+  ~AIX() override;
+

Since we are not doing anything special in AIX toolchain destructor, seems like 
we don't need to override it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68340



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


[PATCH] D68531: [WebAssembly] Add builtin and intrinsic for v8x16.swizzle

2019-10-08 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin accepted this revision.
aheejin added a comment.
This revision is now accepted and ready to land.

> LLVM produces a poison value if the dynamic swizzle indices are greater than 
> the vector size, but the WebAssembly instruction sets the corresponding 
> output lane to zero.

Where do we set those undef or poison lanes to zero?




Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:63
 // SIMD builtins
+TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", 
"unimplemented-simd128")
+

Is the second indices vector always v8x16 too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68531



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


[PATCH] D68578: [HIP] Fix device stub name

2019-10-08 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D68578#1698864 , @t-tye wrote:

> I am a little unclear what this patch is doing as it is mentioned that the 
> mangled name has a _stub in it. My understanding is that the intention was to 
> create a distinct unmangled name for the stub, and then mangle it so that the 
> resulting symbol was a legal mangled name. It sounded like this was the 
> preferred approach, and makes sense to me based on my current understanding. 
> Am I understanding this correctly?


Yes this patch does this.


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

https://reviews.llvm.org/D68578



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


r374072 - [OPENMP50]Prohibit multiple context selector sets in context selectors.

2019-10-08 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Tue Oct  8 08:56:43 2019
New Revision: 374072

URL: http://llvm.org/viewvc/llvm-project?rev=374072=rev
Log:
[OPENMP50]Prohibit multiple context selector sets in context selectors.

According to OpenMP 5.0, 2.3.2 Context Selectors, Restrictions, each
trait-set-selector-name can only be specified once. Added check to
implement this restriction.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/test/OpenMP/declare_variant_ast_print.c
cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp
cfe/trunk/test/OpenMP/declare_variant_messages.c
cfe/trunk/test/OpenMP/declare_variant_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=374072=374071=374072=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Oct  8 08:56:43 
2019
@@ -1211,6 +1211,10 @@ def warn_omp_declare_variant_cs_name_exp
   InGroup;
 def err_omp_declare_variant_item_expected : Error<
   "expected %0 in '%1' context selector of '%2' selector set of 'omp declare 
variant' directive">;
+def err_omp_declare_variant_ctx_set_mutiple_use : Error<
+  "context selector set '%0' is used already in the same 'omp declare variant' 
directive">;
+def note_omp_declare_variant_ctx_set_used_here : Note<
+  "previously context selector set '%0' used here">;
 def warn_omp_more_one_device_type_clause : Warning<
   "more than one 'device_type' clause is specified">,
   InGroup;

Modified: cfe/trunk/lib/Parse/ParseOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseOpenMP.cpp?rev=374072=374071=374072=diff
==
--- cfe/trunk/lib/Parse/ParseOpenMP.cpp (original)
+++ cfe/trunk/lib/Parse/ParseOpenMP.cpp Tue Oct  8 08:56:43 2019
@@ -892,6 +892,7 @@ bool Parser::parseOpenMPContextSelectors
 llvm::function_ref
 Callback) {
+  llvm::StringMap UsedCtxSets;
   do {
 // Parse inner context selector set name.
 if (!Tok.is(tok::identifier)) {
@@ -901,6 +902,16 @@ bool Parser::parseOpenMPContextSelectors
 }
 SmallString<16> Buffer;
 StringRef CtxSelectorSetName = PP.getSpelling(Tok, Buffer);
+auto Res = UsedCtxSets.try_emplace(CtxSelectorSetName, Tok.getLocation());
+if (!Res.second) {
+  // OpenMP 5.0, 2.3.2 Context Selectors, Restrictions.
+  // Each trait-set-selector-name can only be specified once.
+  Diag(Tok.getLocation(), 
diag::err_omp_declare_variant_ctx_set_mutiple_use)
+  << CtxSelectorSetName;
+  Diag(Res.first->getValue(),
+   diag::note_omp_declare_variant_ctx_set_used_here)
+  << CtxSelectorSetName;
+}
 // Parse '='.
 (void)ConsumeToken();
 if (Tok.isNot(tok::equal)) {

Modified: cfe/trunk/test/OpenMP/declare_variant_ast_print.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_variant_ast_print.c?rev=374072=374071=374072=diff
==
--- cfe/trunk/test/OpenMP/declare_variant_ast_print.c (original)
+++ cfe/trunk/test/OpenMP/declare_variant_ast_print.c Tue Oct  8 08:56:43 2019
@@ -8,7 +8,7 @@ int foo(void);
 
 #pragma omp declare variant(foo) match(xxx={}, yyy={ccc})
 #pragma omp declare variant(foo) match(xxx={vvv})
-#pragma omp declare variant(foo) match(implementation={vendor(ibm)}, 
implementation={vendor(llvm)})
+#pragma omp declare variant(foo) match(implementation={vendor(llvm)})
 #pragma omp declare variant(foo) match(implementation={vendor(unknown)})
 #pragma omp declare variant(foo) match(implementation={vendor(score(5): ibm, 
xxx)})
 int bar(void);
@@ -17,6 +17,5 @@ int bar(void);
 // CHECK-NEXT: #pragma omp declare variant(foo) 
match(implementation={vendor(score(5):ibm)})
 // CHECK-NEXT: #pragma omp declare variant(foo) 
match(implementation={vendor(score(5):xxx)})
 // CHECK-NEXT: #pragma omp declare variant(foo) 
match(implementation={vendor(unknown)})
-// CHECK-NEXT: #pragma omp declare variant(foo) 
match(implementation={vendor(ibm)})
 // CHECK-NEXT: #pragma omp declare variant(foo) 
match(implementation={vendor(llvm)})
 // CHECK-NEXT: int bar();

Modified: cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp?rev=374072=374071=374072=diff
==
--- cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp Tue Oct  8 08:56:43 2019
@@ -19,12 +19,11 @@ T foofoo() { return T(); }
 
 // CHECK:  #pragma omp declare variant(foofoo) 

[PATCH] D58531: [clang] Specify type of pthread_create builtin

2019-10-08 Thread James Clarke via Phabricator via cfe-commits
jrtc27 marked an inline comment as done.
jrtc27 added a comment.

In D58531#1662466 , @jdoerfert wrote:

> I like the patch and I think it is fine.
>
> Small nits: 
>  Could we have a test for " we can detect dodgy pthread_create declarations" 
> and maybe `pthread[_attr]_t`?
>  There is an unresolved comment by @shrines.
>  @probinson: "it seems straightforward enough although clearly needs 
> clang-format-diff run over it."
>
> I'll accept this assuming the above points are easy to fix and given that no 
> one expressed concerns but only positive comments were made.


I believe I have addressed everything, with the possible exception of:

> Could we have a test for " we can detect dodgy pthread_create declarations" 
> and maybe `pthread[_attr]_t`?

Is that not what I had already added to 
`clang/test/Sema/implicit-builtin-decl.c`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58531



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


[PATCH] D58531: [clang] Specify type of pthread_create builtin

2019-10-08 Thread James Clarke via Phabricator via cfe-commits
jrtc27 updated this revision to Diff 223871.
jrtc27 added a comment.

Rebased, added explicit no-warning test, clang-format'ed, updated assertion 
message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58531

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-builtin-redecl.c

Index: clang/test/Sema/implicit-builtin-redecl.c
===
--- clang/test/Sema/implicit-builtin-redecl.c
+++ clang/test/Sema/implicit-builtin-redecl.c
@@ -24,3 +24,9 @@
 }
 
 typedef int rindex;
+
+// PR40692
+typedef void pthread_t;
+typedef void pthread_attr_t;
+int pthread_create(pthread_t *, const pthread_attr_t *, // no-warning
+   void *(*)(void *), void *);
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -68,4 +68,4 @@
 // CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
 
 // PR40692
-void pthread_create(); // no warning expected
+void pthread_create(); // expected-warning{{declaration of built-in function 'pthread_create' requires inclusion of the header }}
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4966,6 +4966,8 @@
   AddTypeRef(Context.ObjCClassRedefinitionType, SpecialTypes);
   AddTypeRef(Context.ObjCSelRedefinitionType, SpecialTypes);
   AddTypeRef(Context.getucontext_tType(), SpecialTypes);
+  AddTypeRef(Context.getpthread_tType(), SpecialTypes);
+  AddTypeRef(Context.getpthread_attr_tType(), SpecialTypes);
 
   if (Chain) {
 // Write the mapping information describing our module dependencies and how
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4847,6 +4847,43 @@
 }
   }
 }
+
+if (unsigned Pthread_t = SpecialTypes[SPECIAL_TYPE_PTHREAD_T]) {
+  QualType Pthread_tType = GetType(Pthread_t);
+  if (Pthread_tType.isNull()) {
+Error("pthread_t type is NULL");
+return;
+  }
+
+  if (!Context.pthread_tDecl) {
+if (const TypedefType *Typedef = Pthread_tType->getAs())
+  Context.setpthread_tDecl(Typedef->getDecl());
+else {
+  const TagType *Tag = Pthread_tType->getAs();
+  assert(Tag && "Invalid pthread_t type in AST file");
+  Context.setpthread_tDecl(Tag->getDecl());
+}
+  }
+}
+
+if (unsigned Pthread_attr_t = SpecialTypes[SPECIAL_TYPE_PTHREAD_ATTR_T]) {
+  QualType Pthread_attr_tType = GetType(Pthread_attr_t);
+  if (Pthread_attr_tType.isNull()) {
+Error("pthread_attr_t type is NULL");
+return;
+  }
+
+  if (!Context.pthread_attr_tDecl) {
+if (const TypedefType *Typedef =
+Pthread_attr_tType->getAs())
+  Context.setpthread_attr_tDecl(Typedef->getDecl());
+else {
+  const TagType *Tag = Pthread_attr_tType->getAs();
+  assert(Tag && "Invalid pthread_attr_t type in AST file");
+  Context.setpthread_attr_tDecl(Tag->getDecl());
+}
+  }
+}
   }
 
   ReadPragmaDiagnosticMappings(Context.getDiagnostics());
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1968,6 +1968,8 @@
 return "setjmp.h";
   case ASTContext::GE_Missing_ucontext:
 return "ucontext.h";
+  case ASTContext::GE_Missing_pthread:
+return "pthread.h";
   }
   llvm_unreachable("unhandled error kind");
 }
@@ -5970,6 +5972,10 @@
 Context.setsigjmp_bufDecl(NewTD);
   else if (II->isStr("ucontext_t"))
 Context.setucontext_tDecl(NewTD);
+  else if (II->isStr("pthread_t"))
+Context.setpthread_tDecl(NewTD);
+  else if (II->isStr("pthread_attr_t"))
+Context.setpthread_attr_tDecl(NewTD);
 }
 
   return NewTD;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -9383,6 +9383,11 @@
 //  Builtin Type Computation
 //===--===//
 
+static QualType DecodeFunctionTypeFromStr(
+const char *, bool IsNested, bool IsNoReturn, bool IsNoThrow,
+bool 

r374061 - [clang][ifs] Clang Interface Stubs ToolChain plumbing.

2019-10-08 Thread Puyan Lotfi via cfe-commits
Author: zer0
Date: Tue Oct  8 08:23:14 2019
New Revision: 374061

URL: http://llvm.org/viewvc/llvm-project?rev=374061=rev
Log:
[clang][ifs] Clang Interface Stubs ToolChain plumbing.

Second Landing Attempt:

This patch enables end to end support for generating ELF interface stubs
directly from clang. Now the following:

clang -emit-interface-stubs -o libfoo.so a.cpp b.cpp c.cpp

will product an ELF binary with visible symbols populated. Visibility attributes
and -fvisibility can be used to control what gets populated.

* Adding ToolChain support for clang Driver IFS Merge Phase
* Implementing a default InterfaceStubs Merge clang Tool, used by ToolChain
* Adds support for the clang Driver to involve llvm-ifs on ifs files.
* Adds -emit-merged-ifs flag, to tell llvm-ifs to emit a merged ifs text file
  instead of the final object format (normally ELF)


Differential Revision: https://reviews.llvm.org/D63978



Added:
cfe/trunk/lib/Driver/ToolChains/InterfaceStubs.cpp
cfe/trunk/lib/Driver/ToolChains/InterfaceStubs.h
cfe/trunk/test/InterfaceStubs/conflict-type.ifs
cfe/trunk/test/InterfaceStubs/driver-test.c
cfe/trunk/test/InterfaceStubs/func.ifs
cfe/trunk/test/InterfaceStubs/merge-conflict-test.c
cfe/trunk/test/InterfaceStubs/object-double.c
cfe/trunk/test/InterfaceStubs/object-float.c
cfe/trunk/test/InterfaceStubs/object.c
cfe/trunk/test/InterfaceStubs/object.ifs
Removed:
cfe/trunk/test/InterfaceStubs/object.cpp
Modified:
cfe/trunk/include/clang/Driver/Action.h
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/Phases.h
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/include/clang/Driver/Types.def
cfe/trunk/lib/Driver/Action.cpp
cfe/trunk/lib/Driver/CMakeLists.txt
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/Phases.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/Types.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/InterfaceStubs/bad-format.cpp
cfe/trunk/test/InterfaceStubs/class-template-specialization.cpp
cfe/trunk/test/InterfaceStubs/externstatic.c
cfe/trunk/test/InterfaceStubs/function-template-specialization.cpp
cfe/trunk/test/InterfaceStubs/inline.c
cfe/trunk/test/InterfaceStubs/template-namespace-function.cpp
cfe/trunk/test/InterfaceStubs/weak.cpp
cfe/trunk/test/lit.cfg.py

Modified: cfe/trunk/include/clang/Driver/Action.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Action.h?rev=374061=374060=374061=diff
==
--- cfe/trunk/include/clang/Driver/Action.h (original)
+++ cfe/trunk/include/clang/Driver/Action.h Tue Oct  8 08:23:14 2019
@@ -65,6 +65,7 @@ public:
 BackendJobClass,
 AssembleJobClass,
 LinkJobClass,
+IfsMergeJobClass,
 LipoJobClass,
 DsymutilJobClass,
 VerifyDebugInfoJobClass,
@@ -485,6 +486,17 @@ public:
   }
 };
 
+class IfsMergeJobAction : public JobAction {
+  void anchor() override;
+
+public:
+  IfsMergeJobAction(ActionList , types::ID Type);
+
+  static bool classof(const Action *A) {
+return A->getKind() == IfsMergeJobClass;
+  }
+};
+
 class LinkJobAction : public JobAction {
   void anchor() override;
 

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=374061=374060=374061=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Oct  8 08:23:14 2019
@@ -633,6 +633,9 @@ def emit_llvm : Flag<["-"], "emit-llvm">
   HelpText<"Use the LLVM representation for assembler and object files">;
 def emit_iterface_stubs : Flag<["-"], "emit-interface-stubs">, 
Flags<[CC1Option]>, Group,
   HelpText<"Generate Inteface Stub Files.">;
+def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
+  Flags<[CC1Option]>, Group,
+  HelpText<"Generate Interface Stub Files, emit merged text not binary.">;
 def iterface_stub_version_EQ : JoinedOrSeparate<["-"], 
"interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, Group;

Modified: cfe/trunk/include/clang/Driver/Phases.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Phases.h?rev=374061=374060=374061=diff
==
--- cfe/trunk/include/clang/Driver/Phases.h (original)
+++ cfe/trunk/include/clang/Driver/Phases.h Tue Oct  8 08:23:14 2019
@@ -20,7 +20,8 @@ namespace phases {
 Compile,
 Backend,
 Assemble,
-Link
+Link,
+IfsMerge,
   };
 
   enum {

Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL: 

r374057 - [OPENMP50]Allow functions in declare variant directive to have different

2019-10-08 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Tue Oct  8 07:56:20 2019
New Revision: 374057

URL: http://llvm.org/viewvc/llvm-project?rev=374057=rev
Log:
[OPENMP50]Allow functions in declare variant directive to have different
C linkage.

After some discussion with OpenMP developers, it was decided that the
functions with the different C linkage can be used in declare variant
directive.

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp
cfe/trunk/test/OpenMP/declare_variant_implementation_vendor_codegen.cpp
cfe/trunk/test/OpenMP/declare_variant_messages.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=374057=374056=374057=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Oct  8 07:56:20 2019
@@ -9128,7 +9128,7 @@ public:
   const PartialDiagnosticAt ,
   const PartialDiagnosticAt ,
   const PartialDiagnosticAt , bool TemplatesSupported,
-  bool ConstexprSupported);
+  bool ConstexprSupported, bool CLinkageMayDiffer);
 
   /// Function tries to capture lambda's captured variables in the OpenMP 
region
   /// before the original lambda is captured.

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=374057=374056=374057=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Oct  8 07:56:20 2019
@@ -9685,7 +9685,7 @@ bool Sema::areMultiversionVariantFunctio
 const PartialDiagnosticAt ,
 const PartialDiagnosticAt ,
 const PartialDiagnosticAt , bool TemplatesSupported,
-bool ConstexprSupported) {
+bool ConstexprSupported, bool CLinkageMayDiffer) {
   enum DoesntSupport {
 FuncTemplates = 0,
 VirtFuncs = 1,
@@ -9778,7 +9778,7 @@ bool Sema::areMultiversionVariantFunctio
 if (OldFD->getStorageClass() != NewFD->getStorageClass())
   return Diag(DiffDiagIDAt.first, DiffDiagIDAt.second) << StorageClass;
 
-if (OldFD->isExternC() != NewFD->isExternC())
+if (!CLinkageMayDiffer && OldFD->isExternC() != NewFD->isExternC())
   return Diag(DiffDiagIDAt.first, DiffDiagIDAt.second) << Linkage;
 
 if (CheckEquivalentExceptionSpec(
@@ -9831,7 +9831,8 @@ static bool CheckMultiVersionAdditionalR
   PartialDiagnosticAt(NewFD->getLocation(),
   S.PDiag(diag::err_multiversion_diff)),
   /*TemplatesSupported=*/false,
-  /*ConstexprSupported=*/!IsCPUSpecificCPUDispatchMVType);
+  /*ConstexprSupported=*/!IsCPUSpecificCPUDispatchMVType,
+  /*CLinkageMayDiffer=*/false);
 }
 
 /// Check the validity of a multiversion function declaration that is the

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=374057=374056=374057=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Oct  8 07:56:20 2019
@@ -5109,7 +5109,8 @@ Sema::checkOpenMPDeclareVariantFunction(
   PartialDiagnosticAt(VariantRef->getExprLoc(),
   PDiag(diag::err_omp_declare_variant_diff)
   << FD->getLocation()),
-  /*TemplatesSupported=*/true, /*ConstexprSupported=*/false))
+  /*TemplatesSupported=*/true, /*ConstexprSupported=*/false,
+  /*CLinkageMayDiffer=*/true))
 return None;
   return std::make_pair(FD, cast(DRE));
 }

Modified: cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp?rev=374057=374056=374057=diff
==
--- cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp Tue Oct  8 07:56:20 2019
@@ -201,3 +201,22 @@ void ba() {
   s.foo1();
   static_f();
 }
+
+// CHECK: int fn_linkage_variant();
+// CHECK: extern "C" {
+// CHECK: #pragma omp declare variant(fn_linkage_variant) 
match(implementation={vendor(xxx)})
+// CHECK: int fn_linkage();
+// CHECK: }
+int fn_linkage_variant();
+extern "C" {
+#pragma omp declare variant(fn_linkage_variant) match(implementation = 
{vendor(xxx)})
+int fn_linkage();
+}
+
+// CHECK: extern "C" int fn_linkage_variant1()
+// CHECK: #pragma omp declare variant(fn_linkage_variant1) 
match(implementation={vendor(xxx)})
+// CHECK: int fn_linkage1();
+extern "C" int fn_linkage_variant1();
+#pragma omp declare variant(fn_linkage_variant1) match(implementation = 
{vendor(xxx)})
+int fn_linkage1();
+


[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D68640#1699605 , @thakis wrote:

> In D68640#1699563 , @gribozavr wrote:
>
> > It looks to me that a better fix is to fix the checker to not emit this 
> > warning in MS compatibility mode.
>
>
> The warning is about emitting "here's a redundant declaration", and the test 
> expects an "extern inline" decl to be redundant with an "inline" definition. 
> In ms compat mode they aren't, so the checker does not emit the warning in ms 
> mode (but does elsewhere).
>
> Arguably having a check that suggests removing a bunch of code that's 
> necessary in some modes (ms compat, or C) is a bit weird, so maybe we should 
> never emit this diag for extern inlines. I don't know which policy decisions 
> clang-tidy usually makes for cross-platform development – does it prioritize 
> cross-platform dev, or completeness assuming single-platform dev?


I think it depends on the check, but for a check in the `readability` module, 
I'm not certain there's a clear answer. My gut feeling is to diagnose based on 
platform behavior because the goal is to remove redundancy and that requires 
platform-specific knowledge. But it's not a strong opinion.

> (Finally, these tests have been broken for months, folks are landing lots of 
> stuff with "UNSUPPORTED: win32" (clang VFS patches recently, for example) and 
> we're struggling just keeping tests green on Windows. There's no shortage of 
> possible implicit TODOs :) I think it's better to land this to get the 
> check-clang-tools target green than it is to mark the test UNSUPPORTED.)

To be clear, I wasn't suggesting we fix it in this patch, just that we add a 
FIXME/TODO to the test and make sure we're tracking the bug. Explicit TODO > 
implicit TODO.


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

https://reviews.llvm.org/D68640



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


[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

2019-10-08 Thread Lewis Revill via Phabricator via cfe-commits
lewis-revill updated this revision to Diff 223855.
lewis-revill added a comment.

Rebased to fix conflicts with recent split SP adjustments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-features.c
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.h
  llvm/lib/Target/RISCV/RISCVMachineFunctionInfo.h
  llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
  llvm/lib/Target/RISCV/RISCVRegisterInfo.h
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/saverestore.ll

Index: llvm/test/CodeGen/RISCV/saverestore.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/saverestore.ll
@@ -0,0 +1,640 @@
+; RUN: llc -mtriple=riscv32 < %s | FileCheck %s -check-prefix=RV32I
+; RUN: llc -mtriple=riscv64 < %s | FileCheck %s -check-prefix=RV64I
+; RUN: llc -mtriple=riscv32 -mattr=+save-restore < %s | FileCheck %s -check-prefix=RV32I-SR
+; RUN: llc -mtriple=riscv64 -mattr=+save-restore < %s | FileCheck %s -check-prefix=RV64I-SR
+; RUN: llc -mtriple=riscv32 -mattr=+f,+save-restore -target-abi=ilp32f < %s | FileCheck %s -check-prefix=RV32I-FP-SR
+; RUN: llc -mtriple=riscv64 -mattr=+f,+d,+save-restore -target-abi=lp64d < %s | FileCheck %s -check-prefix=RV64I-FP-SR
+
+; Check that the correct save/restore libcalls are generated.
+
+@var0 = global [18 x i32] zeroinitializer
+@var1 = global [24 x i32] zeroinitializer
+@var2 = global [30 x i32] zeroinitializer
+
+define void @callee_saved0() nounwind {
+; RV32I-LABEL: callee_saved0:
+; RV32I: addi sp, sp, -32
+; RV32I-NEXT:sw s0, 28(sp)
+; RV32I-NEXT:sw s1, 24(sp)
+; RV32I-NEXT:sw s2, 20(sp)
+; RV32I-NEXT:sw s3, 16(sp)
+; RV32I-NEXT:sw s4, 12(sp)
+; RV32I: lw s4, 12(sp)
+; RV32I-NEXT:lw s3, 16(sp)
+; RV32I-NEXT:lw s2, 20(sp)
+; RV32I-NEXT:lw s1, 24(sp)
+; RV32I-NEXT:lw s0, 28(sp)
+; RV32I-NEXT:addi sp, sp, 32
+; RV32I-NEXT:ret
+;
+; RV64I-LABEL: callee_saved0:
+; RV64I: addi sp, sp, -48
+; RV64I-NEXT:sd s0, 40(sp)
+; RV64I-NEXT:sd s1, 32(sp)
+; RV64I-NEXT:sd s2, 24(sp)
+; RV64I-NEXT:sd s3, 16(sp)
+; RV64I: ld s4, 8(sp)
+; RV64I-NEXT:ld s3, 16(sp)
+; RV64I-NEXT:ld s2, 24(sp)
+; RV64I-NEXT:ld s1, 32(sp)
+; RV64I-NEXT:ld s0, 40(sp)
+; RV64I-NEXT:addi sp, sp, 48
+; RV64I-NEXT:ret
+;
+; RV32I-SR-LABEL: callee_saved0:
+; RV32I-SR: call t0, __riscv_save_5
+; RV32I-SR: tail __riscv_restore_5
+;
+; RV64I-SR-LABEL: callee_saved0:
+; RV64I-SR: call t0, __riscv_save_5
+; RV64I-SR: tail __riscv_restore_5
+;
+; RV32I-FP-SR-LABEL: callee_saved0:
+; RV32I-FP-SR: call t0, __riscv_save_5
+; RV32I-FP-SR: tail __riscv_restore_5
+;
+; RV64I-FP-SR-LABEL: callee_saved0:
+; RV64I-FP-SR: call t0, __riscv_save_5
+; RV64I-FP-SR: tail __riscv_restore_5
+  %val = load [18 x i32], [18 x i32]* @var0
+  store volatile [18 x i32] %val, [18 x i32]* @var0
+  ret void
+}
+
+define void @callee_saved1() nounwind {
+; RV32I-LABEL: callee_saved1:
+; RV32I: addi sp, sp, -48
+; RV32I-NEXT:sw s0, 44(sp)
+; RV32I-NEXT:sw s1, 40(sp)
+; RV32I-NEXT:sw s2, 36(sp)
+; RV32I-NEXT:sw s3, 32(sp)
+; RV32I-NEXT:sw s4, 28(sp)
+; RV32I-NEXT:sw s5, 24(sp)
+; RV32I-NEXT:sw s6, 20(sp)
+; RV32I-NEXT:sw s7, 16(sp)
+; RV32I-NEXT:sw s8, 12(sp)
+; RV32I-NEXT:sw s9, 8(sp)
+; RV32I-NEXT:sw s10, 4(sp)
+; RV32I: lw s10, 4(sp)
+; RV32I-NEXT:lw s9, 8(sp)
+; RV32I-NEXT:lw s8, 12(sp)
+; RV32I-NEXT:lw s7, 16(sp)
+; RV32I-NEXT:lw s6, 20(sp)
+; RV32I-NEXT:lw s5, 24(sp)
+; RV32I-NEXT:lw s4, 28(sp)
+; RV32I-NEXT:lw s3, 32(sp)
+; RV32I-NEXT:lw s2, 36(sp)
+; RV32I-NEXT:lw s1, 40(sp)
+; RV32I-NEXT:lw s0, 44(sp)
+; RV32I-NEXT:addi sp, sp, 48
+; RV32I-NEXT:ret
+;
+; RV64I-LABEL: callee_saved1:
+; RV64I: addi sp, sp, -96
+; RV64I-NEXT:sd s0, 88(sp)
+; RV64I-NEXT:sd s1, 80(sp)
+; RV64I-NEXT:sd s2, 72(sp)
+; RV64I-NEXT:sd s3, 64(sp)
+; RV64I-NEXT:sd s4, 56(sp)
+; RV64I-NEXT:sd s5, 48(sp)
+; RV64I-NEXT:sd s6, 40(sp)
+; RV64I-NEXT:sd s7, 32(sp)
+; RV64I-NEXT:sd s8, 24(sp)
+; RV64I-NEXT:sd s9, 16(sp)
+; RV64I-NEXT:sd s10, 8(sp)
+; RV64I: ld s10, 8(sp)
+; RV64I-NEXT:ld s9, 16(sp)
+; RV64I-NEXT:ld s8, 24(sp)
+; RV64I-NEXT:ld s7, 32(sp)
+; RV64I-NEXT:ld s6, 40(sp)
+; RV64I-NEXT:ld s5, 48(sp)
+; RV64I-NEXT:ld s4, 56(sp)
+; RV64I-NEXT:ld s3, 64(sp)
+; RV64I-NEXT:ld s2, 72(sp)
+; RV64I-NEXT:ld s1, 80(sp)
+; RV64I-NEXT:ld s0, 88(sp)
+; RV64I-NEXT:addi sp, sp, 96
+; RV64I-NEXT:ret
+;
+; RV32I-SR-LABEL: callee_saved1:
+; RV32I-SR: call t0, __riscv_save_11
+; RV32I-SR: 

[PATCH] D68637: [libTooling] Move Transformer files to their own directory/library.

2019-10-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 223856.
ymandel added a comment.

update another cmakelists file (uncovered by linking for shared libs)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68637

Files:
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
  clang/include/clang/Tooling/Refactoring/MatchConsumer.h
  clang/include/clang/Tooling/Refactoring/RangeSelector.h
  clang/include/clang/Tooling/Refactoring/SourceCode.h
  clang/include/clang/Tooling/Refactoring/SourceCodeBuilders.h
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/include/clang/Tooling/Transformer/MatchConsumer.h
  clang/include/clang/Tooling/Transformer/RangeSelector.h
  clang/include/clang/Tooling/Transformer/SourceCode.h
  clang/include/clang/Tooling/Transformer/SourceCodeBuilders.h
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/RangeSelector.cpp
  clang/lib/Tooling/Refactoring/SourceCode.cpp
  clang/lib/Tooling/Refactoring/SourceCodeBuilders.cpp
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/lib/Tooling/Transformer/CMakeLists.txt
  clang/lib/Tooling/Transformer/RangeSelector.cpp
  clang/lib/Tooling/Transformer/SourceCode.cpp
  clang/lib/Tooling/Transformer/SourceCodeBuilders.cpp
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/RangeSelectorTest.cpp
  clang/unittests/Tooling/SourceCodeBuildersTest.cpp
  clang/unittests/Tooling/SourceCodeTest.cpp
  clang/unittests/Tooling/StencilTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -6,9 +6,9 @@
 //
 //===--===//
 
-#include "clang/Tooling/Refactoring/Transformer.h"
+#include "clang/Tooling/Transformer/Transformer.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/Tooling/Transformer/RangeSelector.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
 #include "clang/Tooling/Tooling.h"
Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- clang/unittests/Tooling/SourceCodeTest.cpp
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "clang/Tooling/Refactoring/SourceCode.h"
+#include "clang/Tooling/Transformer/SourceCode.h"
 #include "TestVisitor.h"
 #include "clang/Basic/Diagnostic.h"
 #include "llvm/Testing/Support/Annotations.h"
Index: clang/unittests/Tooling/SourceCodeBuildersTest.cpp
===
--- clang/unittests/Tooling/SourceCodeBuildersTest.cpp
+++ clang/unittests/Tooling/SourceCodeBuildersTest.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "clang/Tooling/Refactoring/SourceCodeBuilders.h"
+#include "clang/Tooling/Transformer/SourceCodeBuilders.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
Index: clang/unittests/Tooling/RangeSelectorTest.cpp
===
--- clang/unittests/Tooling/RangeSelectorTest.cpp
+++ clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "clang/Tooling/Refactoring/RangeSelector.h"
+#include "clang/Tooling/Transformer/RangeSelector.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Tooling/FixIt.h"
Index: 

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment.

> There's no shortage of possible implicit TODOs

I don't see "-fno-ms-compatibility" as an implicit TODO. It is most commonly 
used as "this test does something that does not work in MS mode". When I read 
it, I can't tell why it is there. When other people write tests, they will 
copy-paste the RUN line into their own test.


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

https://reviews.llvm.org/D68640



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


[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

2019-10-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D68640#1699563 , @gribozavr wrote:

> It looks to me that a better fix is to fix the checker to not emit this 
> warning in MS compatibility mode.


+1

> I'm OK with "fixing" the test like this, however, it should come with a TODO.

I think we should also open a bug (or repurpose the existing one) to track the 
fact that the bug still exists.


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

https://reviews.llvm.org/D68640



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


[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

2019-10-08 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added a comment.

I'm holding off on reviewing the code until we figure out what the rules are.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst:10
+The check conservatively tries to preserve logical costness in favor of
+physical costness. The only operations on ``this`` that this check considers
+to preserve logical constness are

Sorry, it is unclear to me what it means: "the check [...] tries to do X in 
favor of Y"

Also unclear what logical/physical constness mean.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst:17
+* returning const-qualified ``this``
+* passing const-qualified ``this`` as a parameter.
+

These rules need a justification; if not for the users, but for future 
maintainers.

For example, why isn't reading a private member variable OK? Why isn't calling 
a private member function OK?




Comment at: 
clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp:311
+  // This member function must stay non-const, even though
+  // it only calls other private const member functions.
+  int () {

Is it because the const version already exists? Then that should be the rule 
(not calling a private helper function).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68074



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


  1   2   >