[PATCH] D79393: [clang][codegen] Refactor argument loading in function prolog. NFC.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added reviewers: rjmccall, kerbowa, yaxunl.
Herald added subscribers: cfe-commits, nhaehnle, jvesely.
Herald added a project: clang.
hliao abandoned this revision.

- Skip copying function arguments and unnecessary casting by using them 
directly.

[clang][codegen] Hoist parameter attribute setting in function prolog.

- If the coerced type is still a pointer, it should be set with proper 
parameter attributes, such as `noalias`, `nonnull`, and etc. Hoist that 
(pointer) parameter attribute setting so that the coerced pointer parameter 
could be marked properly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79393

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu

Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -67,3 +67,10 @@
   t.x[0][0] += 1.f;
   t.x[1][0] += 2.f;
 }
+
+// Check that coerced pointers retain the noalias attribute when qualified with __restrict.
+// CHECK: define amdgpu_kernel void @_Z7kernel7Pi(i32 addrspace(1)* noalias %x.coerce)
+// HOST: define void @_Z22__device_stub__kernel7Pi(i32* noalias %x)
+__global__ void kernel7(int *__restrict x) {
+  x[0]++;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4355,7 +4355,7 @@
   ///
   /// \param AI - The first function argument of the expansion.
   void ExpandTypeFromArgs(QualType Ty, LValue Dst,
-  SmallVectorImpl::iterator );
+  llvm::Function::arg_iterator );
 
   /// ExpandTypeToArgs - Expand an CallArg \arg Arg, with the LLVM type for \arg
   /// Ty, into individual arguments on the provided vector \arg IRCallArgs,
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1016,8 +1016,8 @@
   }
 }
 
-void CodeGenFunction::ExpandTypeFromArgs(
-QualType Ty, LValue LV, SmallVectorImpl::iterator ) {
+void CodeGenFunction::ExpandTypeFromArgs(QualType Ty, LValue LV,
+ llvm::Function::arg_iterator ) {
   assert(LV.isSimple() &&
  "Unexpected non-simple lvalue during struct expansion.");
 
@@ -1046,17 +1046,17 @@
   ExpandTypeFromArgs(FD->getType(), SubLV, AI);
 }
   } else if (isa(Exp.get())) {
-auto realValue = *AI++;
-auto imagValue = *AI++;
+auto realValue = &*AI++;
+auto imagValue = &*AI++;
 EmitStoreOfComplex(ComplexPairTy(realValue, imagValue), LV, /*init*/ true);
   } else {
 // Call EmitStoreOfScalar except when the lvalue is a bitfield to emit a
 // primitive store.
 assert(isa(Exp.get()));
 if (LV.isBitField())
-  EmitStoreThroughLValue(RValue::get(*AI++), LV);
+  EmitStoreThroughLValue(RValue::get(&*AI++), LV);
 else
-  EmitStoreOfScalar(*AI++, LV);
+  EmitStoreOfScalar(&*AI++, LV);
   }
 }
 
@@ -2323,19 +2323,13 @@
   // simplify.
 
   ClangToLLVMArgMapping IRFunctionArgs(CGM.getContext(), FI);
-  // Flattened function arguments.
-  SmallVector FnArgs;
-  FnArgs.reserve(IRFunctionArgs.totalIRArgs());
-  for (auto  : Fn->args()) {
-FnArgs.push_back();
-  }
-  assert(FnArgs.size() == IRFunctionArgs.totalIRArgs());
+  assert(Fn->arg_size() == IRFunctionArgs.totalIRArgs());
 
   // If we're using inalloca, all the memory arguments are GEPs off of the last
   // parameter, which is a pointer to the complete memory area.
   Address ArgStruct = Address::invalid();
   if (IRFunctionArgs.hasInallocaArg()) {
-ArgStruct = Address(FnArgs[IRFunctionArgs.getInallocaArgNo()],
+ArgStruct = Address(Fn->getArg(IRFunctionArgs.getInallocaArgNo()),
 FI.getArgStructAlignment());
 
 assert(ArgStruct.getType() == FI.getArgStruct()->getPointerTo());
@@ -2343,7 +2337,7 @@
 
   // Name the struct return parameter.
   if (IRFunctionArgs.hasSRetArg()) {
-auto AI = cast(FnArgs[IRFunctionArgs.getSRetArgNo()]);
+auto AI = Fn->getArg(IRFunctionArgs.getSRetArgNo());
 AI->setName("agg.result");
 AI->addAttr(llvm::Attribute::NoAlias);
   }
@@ -2394,7 +2388,8 @@
 
 case ABIArgInfo::Indirect: {
   assert(NumIRArgs == 1);
-  Address ParamAddr = Address(FnArgs[FirstIRArg], ArgI.getIndirectAlign());
+  Address ParamAddr =
+  Address(Fn->getArg(FirstIRArg), ArgI.getIndirectAlign());
 
   if (!hasScalarEvaluationKind(Ty)) {
 // Aggregates and complex variables are accessed by reference.  All we
@@ -2430,16 +2425,18 @@
 
 case ABIArgInfo::Extend:
 case ABIArgInfo::Direct: {
-
-  // If we have the trivial 

[PATCH] D78660: [SemaObjC] Add a warning for dictionary literals with duplicate keys

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Some part of me wishes we could use expression profiling or ODR hashing or 
something like that for this, but I guess the semantics we're going for don't 
really match.




Comment at: clang/lib/Sema/SemaExprObjC.cpp:948
+checkOneKey(IntegralKeys, Result.Val.getInt(), Loc);
+  }
+}

Does `EvaluateAsInt` really just fail cleanly if the argument doesn't have 
integral type?


Repository:
  rC Clang

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

https://reviews.llvm.org/D78660



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


[PATCH] D79276: [FileCheck] Support comment directives

2020-05-04 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 261998.
jdenny added a comment.

Fixed a missed rename in the test code in the last update.


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

https://reviews.llvm.org/D79276

Files:
  clang/test/CodeGen/default-address-space.c
  clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
  clang/test/Driver/hip-device-libs.hip
  llvm/docs/CommandGuide/FileCheck.rst
  llvm/include/llvm/Support/FileCheck.h
  llvm/lib/Support/FileCheck.cpp
  llvm/test/Assembler/drop-debug-info-nonzero-alloca.ll
  llvm/test/FileCheck/comment.txt
  llvm/test/FileCheck/first-character-match.txt
  llvm/test/FileCheck/validate-check-prefix.txt
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -44,6 +44,14 @@
 cl::desc(
 "Alias for -check-prefix permitting multiple comma separated values"));
 
+static cl::list CommentPrefixes(
+"comment-prefixes", cl::CommaSeparated, cl::Hidden,
+cl::desc("Comma-separated list of comment prefixes to use from check file\n"
+ "(defaults to 'COM,RUN'). Please avoid using this feature in\n"
+ "LLVM's LIT-based test suites, which should be easier to\n"
+ "maintain if they all follow a consistent comment style. This\n"
+ "feature is meant for non-LIT test suites using FileCheck."));
+
 static cl::opt NoCanonicalizeWhiteSpace(
 "strict-whitespace",
 cl::desc("Do not treat all horizontal whitespace as equivalent"));
@@ -279,6 +287,8 @@
 return "label";
   case Check::CheckEmpty:
 return "empty";
+  case Check::CheckComment:
+return "com";
   case Check::CheckEOF:
 return "eof";
   case Check::CheckBadNot:
@@ -565,6 +575,9 @@
   for (StringRef Prefix : CheckPrefixes)
 Req.CheckPrefixes.push_back(Prefix);
 
+  for (StringRef Prefix : CommentPrefixes)
+Req.CommentPrefixes.push_back(Prefix);
+
   for (StringRef CheckNot : ImplicitCheckNot)
 Req.ImplicitCheckNot.push_back(CheckNot);
 
Index: llvm/test/FileCheck/validate-check-prefix.txt
===
--- llvm/test/FileCheck/validate-check-prefix.txt
+++ llvm/test/FileCheck/validate-check-prefix.txt
@@ -8,6 +8,6 @@
 
 ; BAD_PREFIX: supplied check prefix must start with a letter and contain only alphanumeric characters, hyphens, and underscores: 'A!'
 
-; DUPLICATE_PREFIX: error: supplied check prefix must be unique among check prefixes: 'REPEAT'
+; DUPLICATE_PREFIX: error: supplied check prefix must be unique among check prefixes and comment prefixes: 'REPEAT'
 
 ; EMPTY_PREFIX: error: supplied check prefix must not be the empty string
Index: llvm/test/FileCheck/first-character-match.txt
===
--- llvm/test/FileCheck/first-character-match.txt
+++ llvm/test/FileCheck/first-character-match.txt
@@ -1,2 +1,2 @@
-RUN: FileCheck -check-prefix=RUN -input-file %s %s
+RUN: FileCheck -check-prefix=RUN --comment-prefixes=COM -input-file %s %s
 // Prefix is at the first character in the file. The run line then matches itself.
Index: llvm/test/FileCheck/comment.txt
===
--- /dev/null
+++ llvm/test/FileCheck/comment.txt
@@ -0,0 +1,176 @@
+// Comment directives successfully comment out check directives.
+//
+// Check all default comment prefixes.
+// Check that a comment directive at the beginning/end of the file is handled.
+// Check that the preceding/following line's check directive is not affected.
+// RUN: echo 'foo'>  %t-supresses1.in
+// RUN: echo 'COM: CHECK: bar'>  %t-supresses1.chk
+// RUN: echo 'CHECK: foo' >> %t-supresses1.chk
+// RUN: echo 'RUN: echo "CHECK: baz"' >> %t-supresses1.chk
+// RUN: %ProtectFileCheckOutput \
+// RUN: FileCheck -vv %t-supresses1.chk < %t-supresses1.in 2>&1 \
+// RUN: | FileCheck -check-prefix=SUPPRESSES-CHECKS -DCHECK_LINE=2 %s
+//
+// Check the case of one user-specified comment prefix.
+// Check that a comment directive not at the beginning of a line is handled.
+// RUN: echo 'foo'  >  %t-suppresses2.in
+// RUN: echo 'CHECK: foo'   >  %t-suppresses2.chk
+// RUN: echo 'letters then space MY-PREFIX: CHECK: bar' >> %t-suppresses2.chk
+// RUN: %ProtectFileCheckOutput \
+// RUN: FileCheck -vv %t-suppresses2.chk -comment-prefixes=MY-PREFIX \
+// RUN:   < %t-suppresses2.in 2>&1 \
+// RUN: | FileCheck -check-prefix=SUPPRESSES-CHECKS -DCHECK_LINE=1 %s
+//
+// Check the case of multiple user-specified comment prefixes.
+// RUN: echo 'foo'   >  %t-suppresses3.in
+// RUN: echo 'Bar_2: CHECK: Bar' >  %t-suppresses3.chk
+// RUN: echo 'CHECK: foo'>> %t-suppresses3.chk
+// RUN: echo 'Foo_1: CHECK: Foo' >> %t-suppresses3.chk

[PATCH] D72281: [Matrix] Add matrix type to Clang.

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

The test cases for function template partial specialization would look 
something like this:

  template 
  using matrix = T __attribute__((matrix_type(R, C)));
  
  template  struct selector {};
  
  template 
  selector<0> use_matrix(matrix m) {}
  
  template 
  selector<1> use_matrix(matrix m) {}
  
  template 
  selector<2> use_matrix(matrix m) {}
  
  void test() {
selector<2> x = use_matrix(matrix());
selector<1> y = use_matrix(matrix());
selector<0> z = use_matrix(matrix());
  }

But you should include some weirder kinds of template, expressions that aren't 
deducible, and so on.




Comment at: clang/lib/AST/ASTContext.cpp:3840
+ RowExpr, ColumnExpr, AttrLoc);
+  } else {
+QualType CanonicalMatrixElementType = getCanonicalType(MatrixElementType);

fhahn wrote:
> rjmccall wrote:
> > Is this logic copied from the dependent vector code?  It's actually wrong 
> > in a sense — it'll end up creating a non-canonical matrix type wrapping 
> > Canon even if all the components are exactly the same.  Probably the only 
> > impact is that we waste a little memory, although it's also possible that 
> > type-printing in diagnostics will be a little weird.  It'd be better to 
> > recognize that the components match Canon exactly and avoid creating a 
> > redundant node.
> > 
> > Please cache the result of calling `getCanonicalType(MatrixElementType)` 
> > above.
> > Is this logic copied from the dependent vector code? It's actually wrong in 
> > a sense — it'll end up creating a non-canonical matrix type wrapping Canon 
> > even if all the components are exactly the same. Probably the only impact 
> > is that we waste a little memory, although it's also possible that 
> > type-printing in diagnostics will be a little weird. It'd be better to 
> > recognize that the components match Canon exactly and avoid creating a 
> > redundant node.
> 
> Yes I think it is similar to what the code for dependent vectors does. I've 
> updated it to use the Canonical type, it the components match exactly. If 
> that properly addresses your comment I can submit a patch to do the same for 
> the vector case.
This looks right except that you don't want to add it again to the `Types` 
list.  You can just early-exit if you don't have to build anything new.



Comment at: clang/lib/AST/ASTContext.cpp:3846
+  New = new (*this, TypeAlignment) DependentSizedMatrixType(
+  *this, ElementTy, QualType(Canon, 0), RowExpr, ColumnExpr, AttrLoc);
+  } else {

Some of this redundancy is avoidable.  I think you can just structure this as:

- Look for an existing canonical matrix type.
- If you find one, and it matches exactly, return it.
- If you don't have a canonical matrix type, build it and add it to both 
`DependentSizedMatrixTypes` and `Types`.
- You now have a canonical matrix type; if it doesn't match exactly (you can 
remember a flag for this), build a non-canonical matrix type (and add it to 
`Types`).
- Return the non-canonical matrix type if you built one, or otherwise the 
canonical matrix type.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3349
+void CXXNameMangler::mangleType(const MatrixType *T) {
+  Out << "Dm" << T->getNumRows() << "_" << T->getNumColumns() << '_';
+  mangleType(T->getElementType());

fhahn wrote:
> rjmccall wrote:
> > fhahn wrote:
> > > rjmccall wrote:
> > > > fhahn wrote:
> > > > > rjmccall wrote:
> > > > > > You need to ask the Itanium C++ ABI for this mangling, since it's 
> > > > > > not using a vendor-extension prefix.  I know that wasn't done for 
> > > > > > vector types, but we're trying to do the right thing.
> > > > > That basically means reaching out to 
> > > > > https://github.com/itanium-cxx-abi/cxx-abi/, right?
> > > > > 
> > > > > Do you think it would be feasible to initially go with a 
> > > > > vendor-extensions mangling (like 
> > > > > `uMatrixxx`)? I've updated 
> > > > > the mangling to this.
> > > > Yeah, you can open an issue there.
> > > > 
> > > > That approach doesn't quite work because mangling the element type can 
> > > > use or introduce substitutions, but the demangler will naturally skip 
> > > > the whole thing.  I think there are two possible approaches here: we 
> > > > could either treat the entire matrix type as a vendor-extended 
> > > > qualifier on the element type (i.e. `U11matrix_typeILi3ELi4EEi`), or we 
> > > > could extend the vendor-extended type mangling to allow template-args 
> > > > (i.e. `u11matrix_typeIiLi3ELi4EE`).  The latter is probably better and 
> > > > should fit cleanly into the mangling grammar.
> > > Thanks for those suggestions. For now I went with the vendor-extended 
> > > qualifier, because IIUC that would fit in the existing mangling scheme 
> > > without any changes, while the second option would require changes to the 
> > > grammar, right?
> > > 
> > Yes, but 

[PATCH] D72281: [Matrix] Add matrix type to Clang.

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2112
+  llvm::APSInt ArgRows(S.Context.getTypeSize(S.Context.IntTy),
+   ConstantMatrixArg->getNumRows());
+  Result = DeduceNonTypeTemplateArgument(

Oh, and this needs to use `size_t`; not just the value but the type of a 
non-type template parameter can be deduced, so you should use an appropriate 
type.  Test case for this deduction is `template  void foo(matrix);` or something like that, with a check that `decltype(R)` is `size_t` 
(which you can do in any number of ways).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72281



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


[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

2020-05-04 Thread Jake Merdich via Phabricator via cfe-commits
JakeMerdichAMD created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently the 'AlignConsecutive*' options incorrectly align across
elif and else statements, even if they are very far away and across
unrelated preprocessor macros.

This failed since on preprocessor run 2+, there is not enough context
about the #ifdefs to actually differentiate one block from another,
causing them to align across different blocks or even large sections of
the file.

Eg, with AlignConsecutiveAssignments:

  \#if FOO  // Run 1
  \#else// Run 1
  int a   = 1;  // Run 2, wrong
  \#endif   // Run 1
  
  \#if FOO  // Run 1
  \#else// Run 1
  int bar = 1;  // Run 2
  \#endif   // Run 1

is read as

  int a   = 1;  // Run 2, wrong
  int bar = 1;  // Run 2

The approach taken to fix this was to add a new flag to Token that
forces breaking alignment across groups of lines (MustBreakAlignBefore)
in a similar manner to the existing flag that forces a line break
(MustBreakBefore). This flag is set for the first Token after a
preprocessor statement or diff conflict marker.

Possible alternatives might be hashing preprocessor state to detect if
two lines come from the same block (but the way that ifdefs are
sometimes deferred makes that tricky) or showing the preprocessor
statements on all passes instead of just the first one (seems harder).

Fixes #25167,#31281


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79388

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2780,6 +2780,27 @@
"   // line 2 about b\n"
"   long b;",
getLLVMStyleWithColumns(80)));
+
+  // Checks an edge case in preprocessor handling.
+  // These comments should *not* be aligned
+  EXPECT_EQ(
+  "#if FOO\n"
+  "#else\n"
+  "long a; // Line about a\n"
+  "#endif\n"
+  "#if BAR\n"
+  "#else\n"
+  "long b_long_name; // Line about b\n"
+  "#endif\n",
+  format("#if FOO\n"
+ "#else\n"
+ "long a;   // Line about a\n" // Previous (bad) behavior
+ "#endif\n"
+ "#if BAR\n"
+ "#else\n"
+ "long b_long_name; // Line about b\n"
+ "#endif\n",
+ getLLVMStyleWithColumns(80)));
 }
 
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11429,6 +11429,29 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+
+  // Bug 25167
+  verifyFormat("#if A\n"
+   "#else\n"
+   "int  = 12;\n"
+   "#endif\n"
+   "#if B\n"
+   "#else\n"
+   "int a = 12;\n"
+   "#endif\n",
+   Alignment);
+  verifyFormat("enum foo {\n"
+   "#if A\n"
+   "#else\n"
+   "   = 12;\n"
+   "#endif\n"
+   "#if B\n"
+   "#else\n"
+   "  a = 12;\n"
+   "#endif\n"
+   "};\n",
+   Alignment);
+
   EXPECT_EQ("int a = 5;\n"
 "\n"
 "int oneTwoThree = 123;",
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -377,9 +377,11 @@
 if (Changes[i].NewlinesBefore != 0) {
   CommasBeforeMatch = 0;
   EndOfSequence = i;
-  // If there is a blank line, or if the last line didn't contain any
-  // matching token, the sequence ends here.
-  if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine)
+  // If there is a blank line, there is a forced-align-break (eg,
+  // preprocessor), or if the last line didn't contain any matching token,
+  // the sequence ends here.
+  if (Changes[i].NewlinesBefore > 1 ||
+  Changes[i].Tok->MustBreakAlignBefore || !FoundMatchOnLine)
 AlignCurrentSequence();
 
   FoundMatchOnLine = false;
@@ -618,6 +620,8 @@
 if (Changes[i].StartOfBlockComment)
   continue;
 Newlines += Changes[i].NewlinesBefore;
+if (Changes[i].Tok->MustBreakAlignBefore)
+  BreakBeforeNext = true;
 if (!Changes[i].IsTrailingComment)
   continue;
 
Index: 

[PATCH] D79213: [hip] Add noalias on restrict qualified coerced hip pointers

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2556-2563
+  // Restrict qualified HIP pointers that were coerced to global pointers
+  // can be marked with the noalias attribute.
+  if (isCoercedHIPGlobalPointer(*this, getLangOpts(), ArgI, Ty) &&
+  Arg->getType().isRestrictQualified()) {
+auto AI = cast(FnArgs[FirstIRArg]);
+AI->addAttr(llvm::Attribute::NoAlias);
+  }

hliao wrote:
> yaxunl wrote:
> > hliao wrote:
> > > I don't think we need to check pointer address and/or HIP specific. As 
> > > the generic argument processing, if the original type has any qualifiers, 
> > > the coerced type (if it has a single value as the original parameter) 
> > > should have those qualifiers as well. Here, we not only miss `restrict` 
> > > but also alignment, `nonnull`, and etc. It should be fixed as a generic 
> > > case instead of a target- or language-specific one.
> > I agree we should migrate other argument properties for promoted 
> > pointer-type kernel arg for HIP, and that should be possible since other 
> > than the address space change, the type is the same.
> > 
> > However, I am not sure if we can do that for coerced function arguments in 
> > general. It may not even be pointer any more.
> If the coerced type is still a pointer but diffs on the element type, address 
> space, or others. They should share the same qualifiers for pointers.
If the parameter type is `restrict`-qualified and there's a single IR argument 
of pointer type, you should be able to apply `NoAlias`, regardless of coercion, 
in all language modes.  You can just hoist that out of the "trivial case" block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79213



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


[PATCH] D79378: PR34581: Don't remove an 'if (p)' guarding a call to 'operator delete(p)' under -Oz.

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Is it reasonable to figure out ahead of time that we can skip the null check 
completely?  It'd be kindof nice to also take advantage of this at -O0 whenever 
we don't have real work to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79378



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


[PATCH] D78655: [HIP] Add -fhip-lambda-host-device

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I agree with Richard that just making lambdas HD by default in all modes seems 
like the right rule.


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

https://reviews.llvm.org/D78655



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


[clang] d75a6e9 - [CUDA][HIP] Fix empty ctor/dtor check for union

2020-05-04 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2020-05-04T21:52:04-04:00
New Revision: d75a6e93ae99bfcd67219454307da56ebd155d45

URL: 
https://github.com/llvm/llvm-project/commit/d75a6e93ae99bfcd67219454307da56ebd155d45
DIFF: 
https://github.com/llvm/llvm-project/commit/d75a6e93ae99bfcd67219454307da56ebd155d45.diff

LOG: [CUDA][HIP] Fix empty ctor/dtor check for union

union ctor does not call ctors of its data members. union dtor does not call 
dtors of its data members.
Also union does not have base class.

Currently when clang checks whether union has an empty ctor/dtor, it checks the 
ctors/dtors of its
data members. This causes incorrectly diagnose device side global variables and 
shared variables as
having non-empty ctors/dtors.

This patch fixes that.

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

Added: 
clang/test/SemaCUDA/union-init.cu

Modified: 
clang/lib/Sema/SemaCUDA.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCUDA.cpp b/clang/lib/Sema/SemaCUDA.cpp
index faab250e58ff..73d190891b0f 100644
--- a/clang/lib/Sema/SemaCUDA.cpp
+++ b/clang/lib/Sema/SemaCUDA.cpp
@@ -426,6 +426,10 @@ bool Sema::isEmptyCudaConstructor(SourceLocation Loc, 
CXXConstructorDecl *CD) {
   if (CD->getParent()->isDynamicClass())
 return false;
 
+  // Union ctor does not call ctors of its data members.
+  if (CD->getParent()->isUnion())
+return true;
+
   // The only form of initializer allowed is an empty constructor.
   // This will recursively check all base classes and member initializers
   if (!llvm::all_of(CD->inits(), [&](const CXXCtorInitializer *CI) {
@@ -465,6 +469,11 @@ bool Sema::isEmptyCudaDestructor(SourceLocation Loc, 
CXXDestructorDecl *DD) {
   if (ClassDecl->isDynamicClass())
 return false;
 
+  // Union does not have base class and union dtor does not call dtors of its
+  // data members.
+  if (DD->getParent()->isUnion())
+return true;
+
   // Only empty destructors are allowed. This will recursively check
   // destructors for all base classes...
   if (!llvm::all_of(ClassDecl->bases(), [&](const CXXBaseSpecifier ) {

diff  --git a/clang/test/SemaCUDA/union-init.cu 
b/clang/test/SemaCUDA/union-init.cu
new file mode 100644
index ..a633975e3776
--- /dev/null
+++ b/clang/test/SemaCUDA/union-init.cu
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-linux-unknown -fsyntax-only 
-o - -verify
+
+#include "Inputs/cuda.h"
+
+struct A {
+  int a;
+  __device__ A() { a = 1; }
+  __device__ ~A() { a = 2; }
+};
+
+// This can be a global var since ctor/dtors of data members are not called.
+union B {
+  A a;
+  __device__ B() {}
+  __device__ ~B() {}
+};
+
+// This cannot be a global var since it has a dynamic ctor.
+union C {
+  A a;
+  __device__ C() { a.a = 3; }
+  __device__ ~C() {}
+};
+
+// This cannot be a global var since it has a dynamic dtor.
+union D {
+  A a;
+  __device__ D() { }
+  __device__ ~D() { a.a = 4; }
+};
+
+__device__ B b;
+__device__ C c;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, 
__constant__, and __shared__ variables.}}
+__device__ D d;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, 
__constant__, and __shared__ variables.}}
+
+__device__ void foo() {
+  __shared__ B b;
+  __shared__ C c;
+  // expected-error@-1 {{initialization is not supported for __shared__ 
variables.}}
+  __shared__ D d;
+  // expected-error@-1 {{initialization is not supported for __shared__ 
variables.}}
+}



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


[PATCH] D79367: [CUDA][HIP] Fix empty ctor/dtor check for union

2020-05-04 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd75a6e93ae99: [CUDA][HIP] Fix empty ctor/dtor check for 
union (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79367

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/test/SemaCUDA/union-init.cu


Index: clang/test/SemaCUDA/union-init.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/union-init.cu
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-linux-unknown -fsyntax-only 
-o - -verify
+
+#include "Inputs/cuda.h"
+
+struct A {
+  int a;
+  __device__ A() { a = 1; }
+  __device__ ~A() { a = 2; }
+};
+
+// This can be a global var since ctor/dtors of data members are not called.
+union B {
+  A a;
+  __device__ B() {}
+  __device__ ~B() {}
+};
+
+// This cannot be a global var since it has a dynamic ctor.
+union C {
+  A a;
+  __device__ C() { a.a = 3; }
+  __device__ ~C() {}
+};
+
+// This cannot be a global var since it has a dynamic dtor.
+union D {
+  A a;
+  __device__ D() { }
+  __device__ ~D() { a.a = 4; }
+};
+
+__device__ B b;
+__device__ C c;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, 
__constant__, and __shared__ variables.}}
+__device__ D d;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, 
__constant__, and __shared__ variables.}}
+
+__device__ void foo() {
+  __shared__ B b;
+  __shared__ C c;
+  // expected-error@-1 {{initialization is not supported for __shared__ 
variables.}}
+  __shared__ D d;
+  // expected-error@-1 {{initialization is not supported for __shared__ 
variables.}}
+}
Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -426,6 +426,10 @@
   if (CD->getParent()->isDynamicClass())
 return false;
 
+  // Union ctor does not call ctors of its data members.
+  if (CD->getParent()->isUnion())
+return true;
+
   // The only form of initializer allowed is an empty constructor.
   // This will recursively check all base classes and member initializers
   if (!llvm::all_of(CD->inits(), [&](const CXXCtorInitializer *CI) {
@@ -465,6 +469,11 @@
   if (ClassDecl->isDynamicClass())
 return false;
 
+  // Union does not have base class and union dtor does not call dtors of its
+  // data members.
+  if (DD->getParent()->isUnion())
+return true;
+
   // Only empty destructors are allowed. This will recursively check
   // destructors for all base classes...
   if (!llvm::all_of(ClassDecl->bases(), [&](const CXXBaseSpecifier ) {


Index: clang/test/SemaCUDA/union-init.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/union-init.cu
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-linux-unknown -fsyntax-only -o - -verify
+
+#include "Inputs/cuda.h"
+
+struct A {
+  int a;
+  __device__ A() { a = 1; }
+  __device__ ~A() { a = 2; }
+};
+
+// This can be a global var since ctor/dtors of data members are not called.
+union B {
+  A a;
+  __device__ B() {}
+  __device__ ~B() {}
+};
+
+// This cannot be a global var since it has a dynamic ctor.
+union C {
+  A a;
+  __device__ C() { a.a = 3; }
+  __device__ ~C() {}
+};
+
+// This cannot be a global var since it has a dynamic dtor.
+union D {
+  A a;
+  __device__ D() { }
+  __device__ ~D() { a.a = 4; }
+};
+
+__device__ B b;
+__device__ C c;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+__device__ D d;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+__device__ void foo() {
+  __shared__ B b;
+  __shared__ C c;
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  __shared__ D d;
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+}
Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -426,6 +426,10 @@
   if (CD->getParent()->isDynamicClass())
 return false;
 
+  // Union ctor does not call ctors of its data members.
+  if (CD->getParent()->isUnion())
+return true;
+
   // The only form of initializer allowed is an empty constructor.
   // This will recursively check all base classes and member initializers
   if (!llvm::all_of(CD->inits(), [&](const CXXCtorInitializer *CI) {
@@ -465,6 +469,11 @@
   if (ClassDecl->isDynamicClass())
 return false;
 
+  // Union does not have base class and union dtor does not call dtors of its
+  // data members.
+  if (DD->getParent()->isUnion())
+return true;
+
   // Only empty destructors are 

[PATCH] D79236: [docs] Regenerate DiagnosticsReference.rst

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D79236#2019184 , @rsmith wrote:

> In D79236#2019005 , @rjmccall wrote:
>
> > Okay.  So it doesn't just run `ninja docs-clang-html`?  That's unfortunate.
>
>
> It didn't last time I looked into this. And 
> http://clang.llvm.org/docs/DiagnosticsReference.html shows the "before" state 
> of this patch, not the "after", so I think it still doesn't.


Okay.  And I can verify that it is in fact rebuilt by `docs-clang-html`.  So I 
agree that we should continue taking patches like this in the short term, and 
maybe we can restart that conversation about the website.  It's hard to imagine 
why we would be able to run `clang-tblgen` as part of the server build but not 
a full `docs-clang-html` build.  It's not like `clang-tblgen` wouldn't still 
need to be sandboxed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79236



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


[PATCH] D78655: [HIP] Add -fhip-lambda-host-device

2020-05-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

There are two behaviors that seem to make sense:

- Treat lambdas as implicitly HD (like constexpr functions) in all CUDA / HIP 
language modes. I don't think it makes sense for lambdas to become implicitly 
HD in C++17 simply because they become implicitly `constexpr`, nor for their 
HDness to depend on whether their parameter types happen to be literal types, 
etc. So in C++17, where lambdas are constexpr whenever they can be, the logical 
behavior would seem to be that lambdas are implicitly HD. And then for 
consistency with that, I'd expect them to be implicitly HD across all language 
modes.
- Implicitly give lambdas the same HD-ness as the enclosing function (if there 
is one).

I would think the best choice may be to do both of these things: if there is an 
enclosing function, inherit its host or device attributes. And if not, then 
treat the lambda as implicitly HD. A slight variation on that, that might be 
better: lambdas with no lambda-capture are implicitly HD; lambdas with any 
lambda-capture (which must therefore have an enclosing function) inherit the 
enclosing function's HDness.

(Note that if we go this way, it makes no difference if there are reference 
captures, because they're always references on the same "side".)




Comment at: clang/include/clang/Basic/LangOptions.def:247
 LANGOPT(HIPUseNewLaunchAPI, 1, 0, "Use new kernel launching API for HIP")
+LANGOPT(HIPLambdaHostDevice, 1, 0, "Let non-reference-capturing lambda be host 
device for HIP")
 

Is it really appropriate to have a flag for this? I would have expected that 
either the correct HIP behavior would be that lambdas are implicitly HD, or 
not, and Clang should just use whichever behavior is correct. (I don't know 
what authority decides what is and is not "correct" for HIP, but still.)

If this is a difference between versions of HIP, we generally model that by 
having a single "version" field rather than ways of turning on/off the 
individual changes.



Comment at: clang/include/clang/Driver/Options.td:628-632
+def fhip_lambda_host_device : Flag<["-"], "fhip-lambda-host-device">,
+  Flags<[CC1Option]>,
+  HelpText<"Let a lambda function without host/device attributes be a host "
+  "device function if it does not capture by reference (HIP only)">;
+def fno_hip_lambda_host_device : Flag<["-"], "fno-hip-lambda-host-device">;

You document these as "Let [lambdas be HD]" (which I would understand to mean 
"permit lambdas to be HD"), but the actual effect appears to be "make lambdas 
be HD by default".



Comment at: clang/lib/Sema/SemaCUDA.cpp:728-730
   FunctionDecl *CurFn = dyn_cast(CurContext);
   if (!CurFn)
 return;

This check appears to prevent lambdas appearing in any context outside a 
function from being implicitly HD. Is that what you want? Eg:

```
auto foo = [] {}; // not implicitly HD
```



Comment at: clang/lib/Sema/SemaCUDA.cpp:733
+  (Method->isConstexpr() ||
+   (getLangOpts().HIPLambdaHostDevice && !LambdaHasRefCaptures(LI {
+Method->addAttr(CUDADeviceAttr::CreateImplicit(Context));

The reference captures check seems quite strange to me. A copy capture of a 
pointer could have the same problem, as could a copy capture of a class that 
contains a reference or a pointer. As could an init-capture.

These kinds of quirky language rules are usually more trouble than they're 
worth.


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

https://reviews.llvm.org/D78655



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


[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-05-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

In D66831#2019125 , @vsapsai wrote:

> Agree that is a mistake in `NSItemProvider` API. The solution I offer is not 
> to revert the change but to add cc1 flag 
> `-fcompatibility-qualified-id-block-type-checking` and to make the driver for 
> Darwin platforms to add this flag. Thus developers using Apple SDKs will see 
> the old behavior and won't need to change their code. While everybody else 
> will use clang with correct type checking. If any other platforms provide 
> APIs relying on the old type checking, the solution for them is to tweak the 
> driver.
>
> The right `NSItemProvider` fix is to use `__kindof`, like `void (^)(__kindof 
> id item, NSError *error);` It's purpose is to flip 
> substitution principle, so that's the way to express API requirements in the 
> type system. When we have a fix available in SDK, we can change the driver 
> not to add the compatibility flag anymore and `NSItemProvider` should keep 
> working without developers changing their code.
>
> I'm going to work on the patch to add the compatibility flag, if you have any 
> scenarios not covered by the offered solution, please let me know.


Sounds like a fine plan.

I'm not sure whether there's anyone using objective-c blocks other than with 
the Apple SDKs on Darwin platforms, so your proposal seems basically equivalent 
to just entirely disabling the check, temporarily. But that's ok.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66831



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


[PATCH] D79265: Let normalize() for posix style convert backslash to slash unconditionally.

2020-05-04 Thread Nico Weber via Phabricator via cfe-commits
thakis updated this revision to Diff 261980.
thakis added a comment.

restore else


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

https://reviews.llvm.org/D79265

Files:
  clang/test/Lexer/case-insensitive-include-ms.c
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp


Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1182,9 +1182,10 @@
   Tests.emplace_back("a", "a", "a");
   Tests.emplace_back("a/b", "a\\b", "a/b");
   Tests.emplace_back("a\\b", "a\\b", "a/b");
-  Tests.emplace_back("ab", "ab", "ab");
+  Tests.emplace_back("ab", "ab", "a//b");
   Tests.emplace_back("\\a", "\\a", "/a");
   Tests.emplace_back("a\\", "a\\", "a/");
+  Tests.emplace_back("a\\t", "a\\t", "a/t");
 
   for (auto  : Tests) {
 SmallString<64> Win(std::get<0>(T));
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -540,15 +540,9 @@
   Path = PathHome;
 }
   } else {
-for (auto PI = Path.begin(), PE = Path.end(); PI < PE; ++PI) {
-  if (*PI == '\\') {
-auto PN = PI + 1;
-if (PN < PE && *PN == '\\')
-  ++PI; // increment once, the for loop will move over the escaped 
slash
-else
-  *PI = '/';
-  }
-}
+for (auto PI = Path.begin(), PE = Path.end(); PI < PE; ++PI)
+  if (*PI == '\\')
+*PI = '/';
   }
 }
 
Index: clang/test/Lexer/case-insensitive-include-ms.c
===
--- clang/test/Lexer/case-insensitive-include-ms.c
+++ clang/test/Lexer/case-insensitive-include-ms.c
@@ -6,15 +6,17 @@
 // RUN: %clang_cc1 -fsyntax-only -fms-compatibility %s -include %s -I 
%t/Output -verify
 // RUN: %clang_cc1 -fsyntax-only -fms-compatibility 
-fdiagnostics-parseable-fixits %s -include %s -I %t/Output 2>&1 | FileCheck %s
 
-// FIXME: Add a test with repeated backslashes once clang can handle that
-// in ms-compat mode on non-Windows hosts.
 #include "..\Output\.\case-insensitive-include.h"
 #include "..\Output\.\Case-Insensitive-Include.h" // expected-warning 
{{non-portable path}}
 // CHECK: 
fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\""
+#include "..\\Output\.\\Case-Insensitive-Include.h" // expected-warning 
{{non-portable path}}
+// CHECK: 
fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:52}:"\"..Output\\.case-insensitive-include.h\""
 #include "..\output\.\case-insensitive-include.h" // expected-warning 
{{non-portable path}}
 // CHECK: 
fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\""
 
 #include "apath\..\.\case-insensitive-include.h"
 #include "apath\..\.\Case-Insensitive-Include.h" // expected-warning 
{{non-portable path}}
 // CHECK: 
fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:49}:"\"apath\\..\\.\\case-insensitive-include.h\""
+#include "apath\\..\\.\\Case-Insensitive-Include.h" // expected-warning 
{{non-portable path}}
+// CHECK: 
fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:52}:"\"apath...case-insensitive-include.h\""
 #include "APath\..\.\case-insensitive-include.h" // For the sake of 
efficiency, this case is not diagnosed. :-(


Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1182,9 +1182,10 @@
   Tests.emplace_back("a", "a", "a");
   Tests.emplace_back("a/b", "a\\b", "a/b");
   Tests.emplace_back("a\\b", "a\\b", "a/b");
-  Tests.emplace_back("ab", "ab", "ab");
+  Tests.emplace_back("ab", "ab", "a//b");
   Tests.emplace_back("\\a", "\\a", "/a");
   Tests.emplace_back("a\\", "a\\", "a/");
+  Tests.emplace_back("a\\t", "a\\t", "a/t");
 
   for (auto  : Tests) {
 SmallString<64> Win(std::get<0>(T));
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -540,15 +540,9 @@
   Path = PathHome;
 }
   } else {
-for (auto PI = Path.begin(), PE = Path.end(); PI < PE; ++PI) {
-  if (*PI == '\\') {
-auto PN = PI + 1;
-if (PN < PE && *PN == '\\')
-  ++PI; // increment once, the for loop will move over the escaped slash
-else
-  *PI = '/';
-  }
-}
+for (auto PI = Path.begin(), PE = Path.end(); PI < PE; ++PI)
+  if (*PI == '\\')
+*PI = '/';
   }
 }
 
Index: clang/test/Lexer/case-insensitive-include-ms.c
===
--- clang/test/Lexer/case-insensitive-include-ms.c
+++ clang/test/Lexer/case-insensitive-include-ms.c
@@ -6,15 +6,17 @@
 // RUN: %clang_cc1 -fsyntax-only -fms-compatibility %s -include %s -I 

[PATCH] D79121: Add nomerge function attribute to clang

2020-05-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Sema/SemaStmtAttr.cpp:178
+return true;
+  return llvm::any_of(S->children(), hasCallExpr);
+}

This will recurse into too much (eg, the bodies of lambdas and blocks, and 
unevaluated operands). It would be better to make this a 
`ConstEvaluatedExprVisitor` instead (see 
include/clang/AST/EvaluatedExprVisitor.h).


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

https://reviews.llvm.org/D79121



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


[PATCH] D79265: Let normalize() for posix style convert backslash to slash unconditionally.

2020-05-04 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

In D79265#2018770 , @compnerd wrote:

> What happens if you encounter a `"\t"` as a string?  This would convert that 
> to a `"/t"` would it not?  Although, I suppose that in practice the treatment 
> of escaped characters is not important.  I think I still prefer the `} else 
> {` here over the early return.


`"\t"` is converted to `"/t"`. That's also the behavior before this change 
though – only '\' followed by '\' was handled specially before, not '\' 
followed by anything else.

I restored the else and removed the early return.


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

https://reviews.llvm.org/D79265



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


[PATCH] D74813: Function block naming - add hash of parameter type to end of block name

2020-05-04 Thread Alex Borcan via Phabricator via cfe-commits
alexbdv updated this revision to Diff 261978.
alexbdv added a comment.
Herald added subscribers: kerbowa, nhaehnle, jvesely.

Updating tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74813

Files:
  clang/lib/AST/Mangle.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks-irgen.mm
  clang/test/CodeGen/block-with-perdefinedexpr.cpp
  clang/test/CodeGen/blocks.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/func-in-block.c
  clang/test/CodeGen/mangle-blocks.c
  clang/test/CodeGenCXX/block-byref.cpp
  clang/test/CodeGenCXX/blocks.cpp
  clang/test/CodeGenCXX/debug-info-block-invocation-linkage-name.cpp
  clang/test/CodeGenCXX/predefined-expr-cxx14.cpp
  clang/test/CodeGenObjC/blocks-2.m
  clang/test/CodeGenObjC/blocks.m
  clang/test/CodeGenObjC/debug-info-block-line.m
  clang/test/CodeGenObjC/debug-info-nested-blocks.m
  clang/test/CodeGenObjC/mangle-blocks.m
  clang/test/CodeGenObjCXX/arc-forwarded-lambda-call.mm
  clang/test/CodeGenObjCXX/block-default-arg.mm
  clang/test/CodeGenObjCXX/lambda-expressions.mm
  clang/test/CodeGenObjCXX/mangle-blocks.mm
  clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
  clang/test/CodeGenOpenCL/blocks.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  clang/test/PCH/block-helpers.cpp
  clang/test/PCH/no-escaping-block-tail-calls.cpp
  clang/test/Profile/Inputs/objc-general.proftext
  clang/test/Profile/objc-general.m

Index: clang/test/Profile/objc-general.m
===
--- clang/test/Profile/objc-general.m
+++ clang/test/Profile/objc-general.m
@@ -34,7 +34,7 @@
 #endif
 
 // PGOGEN: @[[FRC:"__profc_objc_general.m_\+\[A foreach_\]"]] = private global [2 x i64] zeroinitializer
-// PGOGEN: @[[BLC:"__profc_objc_general.m___13\+\[A foreach_\]_block_invoke"]] = private global [2 x i64] zeroinitializer
+// PGOGEN: @[[BLC:"__profc_objc_general.m___13\+\[A foreach_\]_block_invoke_7636"]] = private global [2 x i64] zeroinitializer
 // PGOGEN: @[[MAC:__profc_main]] = private global [1 x i64] zeroinitializer
 
 @interface A : NSObject
@@ -52,8 +52,8 @@
   // PGOUSE: br {{.*}} !prof ![[FR1:[0-9]+]]
   // PGOUSE: br {{.*}} !prof ![[FR2:[0-9]+]]
   for (id x in array) {
-// PGOGEN: define {{.*}}_block_invoke
-// PGOUSE: define {{.*}}_block_invoke
+// PGOGEN: define {{.*}}_block_invoke_7636
+// PGOUSE: define {{.*}}_block_invoke_7636
 // PGOGEN: store {{.*}} @[[BLC]], i64 0, i64 0
 ^{ static int init = 0;
   // PGOGEN: store {{.*}} @[[BLC]], i64 0, i64 1
Index: clang/test/Profile/Inputs/objc-general.proftext
===
--- clang/test/Profile/Inputs/objc-general.proftext
+++ clang/test/Profile/Inputs/objc-general.proftext
@@ -1,4 +1,4 @@
-objc-general.m:__13+[A foreach:]_block_invoke
+objc-general.m:__13+[A foreach:]_block_invoke_7636
 42129
 2
 2
Index: clang/test/PCH/no-escaping-block-tail-calls.cpp
===
--- clang/test/PCH/no-escaping-block-tail-calls.cpp
+++ clang/test/PCH/no-escaping-block-tail-calls.cpp
@@ -4,7 +4,7 @@
 // Check that -fno-escaping-block-tail-calls doesn't disable tail-call
 // optimization if the block is non-escaping.
 
-// CHECK-LABEL: define internal i32 @___ZN1S1mEv_block_invoke(
+// CHECK-LABEL: define internal i32 @___ZN1S1mEv_block_invoke_7636(
 // CHECK: %[[CALL:.*]] = tail call i32 @_ZN1S3fooER2S0(
 // CHECK-NEXT: ret i32 %[[CALL]]
 
Index: clang/test/PCH/block-helpers.cpp
===
--- clang/test/PCH/block-helpers.cpp
+++ clang/test/PCH/block-helpers.cpp
@@ -19,7 +19,7 @@
 // CHECK: %[[V1:.*]] = bitcast %[[STRUCT_BLOCK_BYREF_Y]]* %[[Y]] to i8*
 // CHECK: store i8* %[[V1]], i8** %[[BLOCK_CAPTURED10]], align 8
 
-// CHECK-LABEL: define internal void @___ZN1S1mEv_block_invoke(
+// CHECK-LABEL: define internal void @___ZN1S1mEv_block_invoke_7636(
 
 // The second call to block_object_assign should be an invoke.
 
Index: clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -312,7 +312,7 @@
   };
 
   // Uses global block literal [[BLG8]] and invoke function [[INVG8]].
-  // COMMON: call spir_func void @__device_side_enqueue_block_invoke_11(i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
+  // COMMON: call spir_func void @__device_side_enqueue_block_invoke_7636.{{[0-9]+}}(i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32, i8 addrspace(4)* } addrspace(1)* [[BLG8]] to i8 addrspace(1)*) to i8 addrspace(4)*))
   block_A();
 
   // Emits global block literal [[BLG8]] and block 

[PATCH] D74668: [Clang][BPF] implement __builtin_btf_type_id() builtin function

2020-05-04 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 261976.
yonghong-song edited the summary of this revision.
yonghong-song added a comment.
Herald added subscribers: ormris, hiraditya, mgorny.

add second argument to __builtin_btf_type_id() to indicate whether a relocation 
should be generated or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74668

Files:
  clang/include/clang/Basic/BuiltinsBPF.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-bpf-btf-type-id.c
  clang/test/Sema/builtin-bpf-btf-type-id.c
  llvm/include/llvm/IR/IntrinsicsBPF.td
  llvm/lib/Target/BPF/BPF.h
  llvm/lib/Target/BPF/BPFCORE.h
  llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
  llvm/lib/Target/BPF/BPFPreserveDIType.cpp
  llvm/lib/Target/BPF/BPFTargetMachine.cpp
  llvm/lib/Target/BPF/BTFDebug.cpp
  llvm/lib/Target/BPF/BTFDebug.h
  llvm/lib/Target/BPF/CMakeLists.txt
  llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll

Index: llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
@@ -0,0 +1,144 @@
+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfel -mattr=+alu32 -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -mattr=+alu32 -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+;
+; Source code:
+;   static int (*bpf_log)(unsigned tid, void *data, int data_size) = (void *)999;
+;   struct {
+; char f1[100];
+; typeof(3) f2;
+;   } tmp__abc = {1, 3};
+;   void prog1() {
+; bpf_log(__builtin_btf_type_id(tmp__abc, 0), __abc, sizeof(tmp__abc));
+;   }
+;   void prog2() {
+; bpf_log(__builtin_btf_type_id(__abc, 1), __abc, sizeof(tmp__abc));
+;   }
+;   void prog3() {
+; bpf_log(__builtin_btf_type_id(tmp__abc.f1[3], 1), __abc, sizeof(tmp__abc));
+;   }
+; Compilation flag:
+;   clang -target bpf -O2 -g -S -emit-llvm test.c
+
+%struct.anon = type { [100 x i8], i32 }
+
+@tmp__abc = dso_local global { <{ i8, i8, [98 x i8] }>, i32 } { <{ i8, i8, [98 x i8] }> <{ i8 1, i8 3, [98 x i8] zeroinitializer }>, i32 0 }, align 4, !dbg !0
+
+; Function Attrs: nounwind
+define dso_local void @prog1() local_unnamed_addr #0 !dbg !28 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 1, i64 0), !dbg !31, !llvm.preserve.access.index !7
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !32
+  ret void, !dbg !33
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon*, i32, i64) #1
+
+; Function Attrs: nounwind
+define dso_local void @prog2() local_unnamed_addr #0 !dbg !34 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 0, i64 1), !dbg !35, !llvm.preserve.access.index !6
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !36
+  ret void, !dbg !37
+}
+
+; Function Attrs: nounwind
+define dso_local void @prog3() local_unnamed_addr #0 !dbg !38 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0i8.i32(i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 2, i64 1), i32 1, i64 1), !dbg !39, !llvm.preserve.access.index !11
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !40
+  ret void, !dbg !41
+}
+
+; CHECK-LABEL:   prog1
+; CHECK: r1 = 3
+; CHECK-LABEL:   prog2
+; CHECK: r1 = 10
+; CHECK-LABEL:   prog3
+; CHECK: r1 = 4
+;
+; CHECK: .long   0   # BTF_KIND_STRUCT(id = 3)
+; CHECK-NEXT:.long   67108866# 0x402
+; CHECK-NEXT:.long   104
+; CHECK-NEXT:.long   13
+; CHECK-NEXT:.long   5
+; CHECK-NEXT:.long   0   # 0x0
+; CHECK-NEXT:.long   16
+; CHECK-NEXT:.long   7
+; CHECK-NEXT:.long   800 # 0x320
+; CHECK-NEXT:.long   19  # BTF_KIND_INT(id = 4)
+; CHECK-NEXT:.long   16777216# 0x100
+; CHECK-NEXT:.long   

[PATCH] D74572: [BPF] preserve debuginfo types for builtin __builtin__btf_type_id()

2020-05-04 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 261977.
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

add second argument to __builtin_btf_type_id(var, flag) to indicate whether a 
.BTF.ext relocation should be generated or not.
flag == 0, no reloc, flag == 1, reloc, others, error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74572

Files:
  llvm/lib/Target/BPF/BPF.h
  llvm/lib/Target/BPF/BPFCORE.h
  llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
  llvm/lib/Target/BPF/BPFPreserveDIType.cpp
  llvm/lib/Target/BPF/BPFTargetMachine.cpp
  llvm/lib/Target/BPF/BTFDebug.cpp
  llvm/lib/Target/BPF/BTFDebug.h
  llvm/lib/Target/BPF/CMakeLists.txt
  llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll

Index: llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
@@ -0,0 +1,144 @@
+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfel -mattr=+alu32 -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -mattr=+alu32 -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+;
+; Source code:
+;   static int (*bpf_log)(unsigned tid, void *data, int data_size) = (void *)999;
+;   struct {
+; char f1[100];
+; typeof(3) f2;
+;   } tmp__abc = {1, 3};
+;   void prog1() {
+; bpf_log(__builtin_btf_type_id(tmp__abc, 0), __abc, sizeof(tmp__abc));
+;   }
+;   void prog2() {
+; bpf_log(__builtin_btf_type_id(__abc, 1), __abc, sizeof(tmp__abc));
+;   }
+;   void prog3() {
+; bpf_log(__builtin_btf_type_id(tmp__abc.f1[3], 1), __abc, sizeof(tmp__abc));
+;   }
+; Compilation flag:
+;   clang -target bpf -O2 -g -S -emit-llvm test.c
+
+%struct.anon = type { [100 x i8], i32 }
+
+@tmp__abc = dso_local global { <{ i8, i8, [98 x i8] }>, i32 } { <{ i8, i8, [98 x i8] }> <{ i8 1, i8 3, [98 x i8] zeroinitializer }>, i32 0 }, align 4, !dbg !0
+
+; Function Attrs: nounwind
+define dso_local void @prog1() local_unnamed_addr #0 !dbg !28 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 1, i64 0), !dbg !31, !llvm.preserve.access.index !7
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !32
+  ret void, !dbg !33
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon*, i32, i64) #1
+
+; Function Attrs: nounwind
+define dso_local void @prog2() local_unnamed_addr #0 !dbg !34 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 0, i64 1), !dbg !35, !llvm.preserve.access.index !6
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !36
+  ret void, !dbg !37
+}
+
+; Function Attrs: nounwind
+define dso_local void @prog3() local_unnamed_addr #0 !dbg !38 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0i8.i32(i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 2, i64 1), i32 1, i64 1), !dbg !39, !llvm.preserve.access.index !11
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !40
+  ret void, !dbg !41
+}
+
+; CHECK-LABEL:   prog1
+; CHECK: r1 = 3
+; CHECK-LABEL:   prog2
+; CHECK: r1 = 10
+; CHECK-LABEL:   prog3
+; CHECK: r1 = 4
+;
+; CHECK: .long   0   # BTF_KIND_STRUCT(id = 3)
+; CHECK-NEXT:.long   67108866# 0x402
+; CHECK-NEXT:.long   104
+; CHECK-NEXT:.long   13
+; CHECK-NEXT:.long   5
+; CHECK-NEXT:.long   0   # 0x0
+; CHECK-NEXT:.long   16
+; CHECK-NEXT:.long   7
+; CHECK-NEXT:.long   800 # 0x320
+; CHECK-NEXT:.long   19  # BTF_KIND_INT(id = 4)
+; CHECK-NEXT:.long   16777216# 0x100
+; CHECK-NEXT:.long   1
+; CHECK-NEXT:.long   16777224# 0x108
+; CHECK: .long   0   # BTF_KIND_PTR(id = 10)
+; CHECK-NEXT:.long   33554432# 0x200
+; CHECK-NEXT:.long   3
+
+; CHECK: .long   16  

[PATCH] D79236: [docs] Regenerate DiagnosticsReference.rst

2020-05-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D79236#2019005 , @rjmccall wrote:

> Okay.  So it doesn't just run `ninja docs-clang-html`?  That's unfortunate.


It didn't last time I looked into this. And 
http://clang.llvm.org/docs/DiagnosticsReference.html shows the "before" state 
of this patch, not the "after", so I think it still doesn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79236



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


[PATCH] D79121: Add nomerge function attribute to clang

2020-05-04 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:611
 void CodeGenFunction::EmitAttributedStmt(const AttributedStmt ) {
+  for (const auto *A: S.getAttrs())
+if (A->getKind() == attr::NoMerge) {

rnk wrote:
> Can we use S.hasAttr, or does that not exist for statements?
Not exist for statements.


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

https://reviews.llvm.org/D79121



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


[PATCH] D79121: Add nomerge function attribute to clang

2020-05-04 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 261973.
zequanwu marked 10 inline comments as done.
zequanwu added a comment.

To keep it simple, `nomerge` attribute should not be applied to decl statement. 
Since label statement calls `ProcessDeclAttributeList` to handle attributes, 
label statement should not have `nomerge` attribute.


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

https://reviews.llvm.org/D79121

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-nomerge.cpp
  clang/test/Sema/attr-nomerge.cpp

Index: clang/test/Sema/attr-nomerge.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-nomerge.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+void bar();
+
+void foo() {
+  [[clang::nomerge]] bar();
+  [[clang::nomerge(1, 2)]] bar(); // expected-error {{'nomerge' attribute takes no arguments}}
+  int x;
+  [[clang::nomerge]] x = 10; // expected-warning {{nomerge attribute is ignored because there exists no call expression inside the statement}}
+
+  [[clang::nomerge]] label: bar(); // expected-error {{'nomerge' attribute cannot be applied to a declaration}}
+
+}
+
+int f();
+
+[[clang::nomerge]] static int i = f(); // expected-error {{'nomerge' attribute cannot be applied to a declaration}}
Index: clang/test/CodeGen/attr-nomerge.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-nomerge.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s
+
+bool bar();
+void f(bool, bool);
+
+void foo(int i) {
+  [[clang::nomerge]] bar();
+  [[clang::nomerge]] (i = 4, bar());
+  [[clang::nomerge]] (void)(bar());
+  [[clang::nomerge]] f(bar(), bar());
+  [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the anonymous function call
+  [[clang::nomerge]] for (bar(); bar(); bar()) {}
+  bar();
+}
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR:[0-9]+]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call void @_Z1fbb({{.*}}) #[[NOMERGEATTR]]
+// CHECK: call void @"_ZZ3fooiENK3$_0clEv"(%class.anon* %ref.tmp) #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call zeroext i1 @_Z3barv()
+// CHECK: attributes #[[NOMERGEATTR]] = { nomerge }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -16,6 +16,7 @@
 #include "clang/Sema/DelayedDiagnostic.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ScopeInfo.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 
 using namespace clang;
@@ -170,6 +171,26 @@
   return LoopHintAttr::CreateImplicit(S.Context, Option, State, ValueExpr, A);
 }
 
+static bool hasCallExpr(const Stmt *S) {
+  if (!S) return false;
+  if (S->getStmtClass() == Stmt::CallExprClass)
+return true;
+  return llvm::any_of(S->children(), hasCallExpr);
+}
+
+static Attr *handleNoMergeAttr(Sema , Stmt *St, const ParsedAttr ,
+   SourceRange Range) {
+  NoMergeAttr NMA(S.Context, A);
+  if (S.CheckAttrNoArgs(A))
+return nullptr;
+  if (!hasCallExpr(St)) {
+S.Diag(St->getBeginLoc(), diag::warn_nomerge_attribute_ignored_in_stmt)
+<< NMA.getSpelling();
+return nullptr;
+  }
+  return ::new (S.Context) NoMergeAttr(S.Context, A);
+}
+
 static void
 CheckForIncompatibleAttributes(Sema ,
const SmallVectorImpl ) {
@@ -335,6 +356,8 @@
 return handleOpenCLUnrollHint(S, St, A, Range);
   case ParsedAttr::AT_Suppress:
 return handleSuppressAttr(S, St, A, Range);
+  case ParsedAttr::AT_NoMerge:
+return handleNoMergeAttr(S, St, A, Range);
   default:
 // if we're here, then we parsed a known attribute, but didn't recognize
 // it as a statement attribute => it is declaration attribute
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -595,6 +595,9 @@
   /// region.
   bool IsInPreservedAIRegion = false;
 
+  /// True if the current statement has nomerge attribute.
+  bool InNoMergeAttributedStmt = false;
+
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
Index: clang/lib/CodeGen/CGStmt.cpp

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-05-04 Thread Scott Constable via Phabricator via cfe-commits
sconstab updated this revision to Diff 261968.
sconstab marked 9 inline comments as done.
sconstab added a comment.
Herald added a subscriber: mgrang.

Addressed the previously unaddressed comments, as pointed out by @craig.topper.


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

https://reviews.llvm.org/D75936

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/test/Driver/x86-target-features.c
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/ImmutableGraph.h
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/O3-pipeline.ll
  llvm/test/CodeGen/X86/lvi-hardening-gadget-graph.ll

Index: llvm/test/CodeGen/X86/lvi-hardening-gadget-graph.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/lvi-hardening-gadget-graph.ll
@@ -0,0 +1,129 @@
+; RUN: llc -verify-machineinstrs -mtriple=x86_64-unknown -x86-lvi-load-dot-verify -o %t < %s | FileCheck %s
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @test(i32* %untrusted_user_ptr, i32* %secret, i32 %secret_size) #0 {
+entry:
+  %untrusted_user_ptr.addr = alloca i32*, align 8
+  %secret.addr = alloca i32*, align 8
+  %secret_size.addr = alloca i32, align 4
+  %ret_val = alloca i32, align 4
+  %i = alloca i32, align 4
+  store i32* %untrusted_user_ptr, i32** %untrusted_user_ptr.addr, align 8
+  store i32* %secret, i32** %secret.addr, align 8
+  store i32 %secret_size, i32* %secret_size.addr, align 4
+  store i32 0, i32* %ret_val, align 4
+  call void @llvm.x86.sse2.lfence()
+  store i32 0, i32* %i, align 4
+  br label %for.cond
+
+for.cond: ; preds = %for.inc, %entry
+  %0 = load i32, i32* %i, align 4
+  %1 = load i32, i32* %secret_size.addr, align 4
+  %cmp = icmp slt i32 %0, %1
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body: ; preds = %for.cond
+  %2 = load i32, i32* %i, align 4
+  %rem = srem i32 %2, 2
+  %cmp1 = icmp eq i32 %rem, 0
+  br i1 %cmp1, label %if.then, label %if.else
+
+if.then:  ; preds = %for.body
+  %3 = load i32*, i32** %secret.addr, align 8
+  %4 = load i32, i32* %ret_val, align 4
+  %idxprom = sext i32 %4 to i64
+  %arrayidx = getelementptr inbounds i32, i32* %3, i64 %idxprom
+  %5 = load i32, i32* %arrayidx, align 4
+  %6 = load i32*, i32** %untrusted_user_ptr.addr, align 8
+  store i32 %5, i32* %6, align 4
+  br label %if.end
+
+if.else:  ; preds = %for.body
+  %7 = load i32*, i32** %secret.addr, align 8
+  %8 = load i32, i32* %ret_val, align 4
+  %idxprom2 = sext i32 %8 to i64
+  %arrayidx3 = getelementptr inbounds i32, i32* %7, i64 %idxprom2
+  store i32 42, i32* %arrayidx3, align 4
+  br label %if.end
+
+if.end:   ; preds = %if.else, %if.then
+  %9 = load i32*, i32** %untrusted_user_ptr.addr, align 8
+  %10 = load i32, i32* %9, align 4
+  store i32 %10, i32* %ret_val, align 4
+  br label %for.inc
+
+for.inc:  ; preds = %if.end
+  %11 = load i32, i32* %i, align 4
+  %inc = add nsw i32 %11, 1
+  store i32 %inc, i32* %i, align 4
+  br label %for.cond
+
+for.end:  ; preds = %for.cond
+  %12 = load i32, i32* %ret_val, align 4
+  ret i32 %12
+}
+
+; CHECK:  digraph "Speculative gadgets for \"test\" function" {
+; CHECK-NEXT: label="Speculative gadgets for \"test\" function";
+; CHECK:  Node0x{{[0-9a-f]+}} [shape=record,color = green,label="{LFENCE\n}"];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} -> Node0x{{[0-9a-f]+}}[label = 0];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} [shape=record,label="{renamable $eax = MOV32rm %stack.4.i, 1, $noreg, 0, $noreg :: (dereferenceable load 4 from %ir.i)\n}"];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} -> Node0x{{[0-9a-f]+}}[color = red, style = "dashed"];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} -> Node0x{{[0-9a-f]+}}[label = 1];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} [shape=record,label="{JCC_1 %bb.6, 13, implicit killed $eflags\n}"];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} -> Node0x{{[0-9a-f]+}}[label = 1];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} -> Node0x{{[0-9a-f]+}}[label = 1];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} [shape=record,label="{CMP32rm killed renamable $eax, %stack.2.secret_size.addr, 1, $noreg, 0, $noreg, implicit-def $eflags :: (dereferenceable load 4 from %ir.secret_size.addr)\n}"];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} -> Node0x{{[0-9a-f]+}}[color = red, style = "dashed"];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} -> Node0x{{[0-9a-f]+}}[label = 1];
+; CHECK-NEXT: Node0x{{[0-9a-f]+}} [shape=record,label="{renamable $eax = MOV32rm %stack.4.i, 1, $noreg, 0, $noreg :: (dereferenceable load 4 from 

[PATCH] D75936: Add a Pass to X86 that builds a Condensed CFG for Load Value Injection (LVI) Gadgets [4/6]

2020-05-04 Thread Scott Constable via Phabricator via cfe-commits
sconstab added inline comments.



Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:322
+  DenseSet> GadgetEdgeSet;
+  auto AnalyzeUse = [&](NodeAddr Use, MachineInstr *MI) {
+assert(!(Use.Addr->getFlags() & NodeAttrs::PhiRef));

craig.topper wrote:
> mattdr wrote:
> > Please add a comment explaining the semantics of the boolean return here. I 
> > //think// it's: `true` if we need to consider defs of this instruction 
> > tainted by this use (and therefore add them for analysis); `false` if this 
> > instruction consumes its use
> Was this comment addressed?
It had not been addressed, so thank you for pointing this out. That lambda was 
doing too many things at once, which made it more confusing than it needed to 
be. So I just inlined it in the
```
for (auto N : Uses) {
…
}
```
loop, and I added some additional clarifying comments.



Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:341
+GadgetBegin.first, GadgetEnd.first);
+  if (UseMI.mayLoad()) // FIXME: This should be more precise
+return false;  // stop traversing further uses of `Reg`

craig.topper wrote:
> mattdr wrote:
> > Some more detail would be useful here: precise about what? What are the 
> > likely errors?
> > 
> Was this answered somewhere?
This was referring to the use of `mayLoad()`. At the time I wrote that comment, 
I wasn't sure that `mayLoad()` was exactly what was needed there, but I now 
think that it does suffice (SLH also uses `MachineInstr::mayLoad()`).



Comment at: llvm/lib/Target/X86/X86LoadValueInjectionLoadHardening.cpp:409
+NodeList Defs = SA.Addr->members_if(DataFlowGraph::IsDef, DFG);
+llvm::for_each(Defs, AnalyzeDef);
+  }

craig.topper wrote:
> mattdr wrote:
> > We analyze every def from every instruction in the function, but then 
> > //also// in `AnalyzeDefUseChain` analyze every def of every instruction 
> > with an interesting use. Are we doing a lot of extra work?
> Was this answered somewhere?
Wow, big oversight on my part. @mattdr was correct that this was doing a LOT of 
extra work. I added a memoization scheme that remembers the instructions that 
may transmit for each def. The getGadgetGraph() routine now runs about 75% 
faster.


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

https://reviews.llvm.org/D75936



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


[PATCH] D79380: [clang-tidy] In TransformerClangTidyCheck, support option SourceNamingStyle.

2020-05-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision.
ymandel added a reviewer: gribozavr2.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

The new option allows the user to specify which file naming convention is used
by the source code ('llvm' or 'google').


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79380

Files:
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -221,6 +221,84 @@
 test::runCheckOnCode>(Input));
 }
 
+class IncludeOrderCheck : public TransformerClangTidyCheck {
+  static RewriteRule rule() {
+using namespace ::clang::ast_matchers;
+RewriteRule Rule = transformer::makeRule(integerLiteral(), change(cat("5")),
+ cat("no message"));
+addInclude(Rule, "bar.h", IncludeFormat::Quoted);
+return Rule;
+  }
+
+public:
+  IncludeOrderCheck(StringRef Name, ClangTidyContext *Context)
+  : TransformerClangTidyCheck(rule(), Name, Context) {}
+};
+
+TEST(TransformerClangTidyCheckTest, AddIncludeObeysSortStyleLocalOption) {
+  std::string Input = R"cc(#include "input.h"
+int h(int x) { return 3; })cc";
+
+  std::string TreatsAsLibraryHeader = R"cc(#include "input.h"
+
+#include "bar.h"
+int h(int x) { return 5; })cc";
+
+  std::string TreatsAsNormalHeader = R"cc(#include "bar.h"
+#include "input.h"
+int h(int x) { return 5; })cc";
+
+  ClangTidyOptions Options;
+  std::map PathsToContent = {{"input.h", "\n"}};
+  Options.CheckOptions["test-check-0.SourceNamingStyle"] = "llvm";
+  EXPECT_EQ(TreatsAsLibraryHeader, test::runCheckOnCode(
+   Input, nullptr, "inputTest.cpp", None,
+   Options, PathsToContent));
+  EXPECT_EQ(TreatsAsNormalHeader, test::runCheckOnCode(
+  Input, nullptr, "input_test.cpp", None,
+  Options, PathsToContent));
+
+  Options.CheckOptions["test-check-0.SourceNamingStyle"] = "google";
+  EXPECT_EQ(TreatsAsNormalHeader,
+test::runCheckOnCode(
+Input, nullptr, "inputTest.cc", None, Options, PathsToContent));
+  EXPECT_EQ(TreatsAsLibraryHeader, test::runCheckOnCode(
+   Input, nullptr, "input_test.cc", None,
+   Options, PathsToContent));
+}
+
+TEST(TransformerClangTidyCheckTest, AddIncludeObeysSortStyleGlobalOption) {
+  std::string Input = R"cc(#include "input.h"
+int h(int x) { return 3; })cc";
+
+  std::string TreatsAsLibraryHeader = R"cc(#include "input.h"
+
+#include "bar.h"
+int h(int x) { return 5; })cc";
+
+  std::string TreatsAsNormalHeader = R"cc(#include "bar.h"
+#include "input.h"
+int h(int x) { return 5; })cc";
+
+  ClangTidyOptions Options;
+  std::map PathsToContent = {{"input.h", "\n"}};
+  Options.CheckOptions["SourceNamingStyle"] = "llvm";
+  EXPECT_EQ(TreatsAsLibraryHeader, test::runCheckOnCode(
+   Input, nullptr, "inputTest.cpp", None,
+   Options, PathsToContent));
+  EXPECT_EQ(TreatsAsNormalHeader, test::runCheckOnCode(
+  Input, nullptr, "input_test.cpp", None,
+  Options, PathsToContent));
+
+  Options.CheckOptions["SourceNamingStyle"] = "google";
+  EXPECT_EQ(TreatsAsNormalHeader,
+test::runCheckOnCode(
+Input, nullptr, "inputTest.cc", None, Options, PathsToContent));
+  EXPECT_EQ(TreatsAsLibraryHeader, test::runCheckOnCode(
+   Input, nullptr, "input_test.cc", None,
+   Options, PathsToContent));
+}
+
 } // namespace
 } // namespace utils
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
@@ -10,7 +10,8 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_TRANSFORMER_CLANG_TIDY_CHECK_H
 
 #include "../ClangTidy.h"
-#include "../utils/IncludeInserter.h"
+#include "IncludeInserter.h"
+#include "IncludeSorter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Tooling/Transformer/Transformer.h"
@@ -31,6 +32,11 @@
 //   MyCheck(StringRef Name, ClangTidyContext *Context)
 //   : TransformerClangTidyCheck(MyCheckAsRewriteRule, Name, Context) 

[PATCH] D79378: PR34581: Don't remove an 'if (p)' guarding a call to 'operator delete(p)' under -Oz.

2020-05-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision.
rsmith added reviewers: davide, dnsampaio, rjmccall.
Herald added a subscriber: hiraditya.
Herald added projects: clang, LLVM.

This transformation is correct for a builtin call to 'free(p)', but not
for 'operator delete(p)'. There is no guarantee that a user replacement
'operator delete' has no effect when called on a null pointer.

However, the principle behind the transformation *is* correct, and can
be applied more broadly: a 'delete p' expression is permitted to
unconditionally call 'operator delete(p)'. So do that in Clang under
-Oz where possible. We do this whether or not 'p' has trivial
destruction, since the destruction might turn out to be trivial after
inlining, and even for a class-specific (but non-virtual,
non-destroying, non-array) 'operator delete'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79378

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/test/CodeGenCXX/delete.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/malloc-free-delete.ll

Index: llvm/test/Transforms/InstCombine/malloc-free-delete.ll
===
--- llvm/test/Transforms/InstCombine/malloc-free-delete.ll
+++ llvm/test/Transforms/InstCombine/malloc-free-delete.ll
@@ -140,6 +140,31 @@
   ret void
 }
 
+; Same optimization with even a builtin 'operator delete' would be
+; incorrect in general.
+; 'if (p) delete p;' cannot result in a call to 'operator delete(0)'.
+define void @test6a(i8* %foo) minsize {
+; CHECK-LABEL: @test6a(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[TOBOOL:%.*]] = icmp eq i8* [[FOO:%.*]], null
+; CHECK-NEXT:br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK:   if.then:
+; CHECK-NEXT:tail call void @_ZdlPv(i8* [[FOO]])
+; CHECK-NEXT:br label [[IF_END]]
+; CHECK:   if.end:
+; CHECK-NEXT:ret void
+entry:
+  %tobool = icmp eq i8* %foo, null
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:  ; preds = %entry
+  tail call void @_ZdlPv(i8* %foo) builtin
+  br label %if.end
+
+if.end:   ; preds = %entry, %if.then
+  ret void
+}
+
 declare i8* @_ZnwmRKSt9nothrow_t(i64, i8*) nobuiltin
 declare void @_ZdlPvRKSt9nothrow_t(i8*, i8*) nobuiltin
 declare i32 @__gxx_personality_v0(...)
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2672,9 +2672,16 @@
   // if (foo) free(foo);
   // into
   // free(foo);
-  if (MinimizeSize)
-if (Instruction *I = tryToMoveFreeBeforeNullTest(FI, DL))
-  return I;
+  //
+  // Note that we can only do this for 'free' and not for any flavor of
+  // 'operator delete'; there is no 'operator delete' symbol for which we are
+  // permitted to invent a call, even if we're passing in a null pointer.
+  if (MinimizeSize) {
+LibFunc Func;
+if (TLI.getLibFunc(FI, Func) && TLI.has(Func) && Func == LibFunc_free)
+  if (Instruction *I = tryToMoveFreeBeforeNullTest(FI, DL))
+return I;
+  }
 
   return nullptr;
 }
Index: clang/test/CodeGenCXX/delete.cpp
===
--- clang/test/CodeGenCXX/delete.cpp
+++ clang/test/CodeGenCXX/delete.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOSIZE
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -emit-llvm -o - -Oz -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,CHECK-SIZE
 
 void t1(int *a) {
   delete a;
@@ -9,7 +10,19 @@
 };
 
 // POD types.
+
+// CHECK-LABEL: define void @_Z2t3P1S
 void t3(S *s) {
+  // CHECK: icmp {{.*}} null
+  // CHECK: br i1
+
+  // CHECK: bitcast
+  // CHECK-NEXT: call void @_ZdlPv
+
+  // Check the delete is inside the 'if !null' check unless we're optimizing
+  // for size. FIXME: We could omit the branch entirely in this case.
+  // CHECK-NOSIZE-NEXT: br
+  // CHECK-SIZE-NEXT: ret
   delete s;
 }
 
@@ -22,7 +35,9 @@
 // CHECK-LABEL: define void @_Z2t4P1T
 void t4(T *t) {
   // CHECK: call void @_ZN1TD1Ev
-  // CHECK-NEXT: bitcast
+  // CHECK-NOSIZE-NEXT: bitcast
+  // CHECK-SIZE-NEXT: br
+  // CHECK-SIZE: bitcast
   // CHECK-NEXT: call void @_ZdlPv
   delete t;
 }
@@ -49,7 +64,9 @@
   // CHECK-LABEL: define void @_ZN5test04testEPNS_1AE(
   void test(A *a) {
 // CHECK: call void @_ZN5test01AD1Ev
-// CHECK-NEXT: bitcast
+// CHECK-NOSIZE-NEXT: bitcast
+// CHECK-SIZE-NEXT: br
+// CHECK-SIZE: bitcast
 // CHECK-NEXT: call void @_ZN5test01AdlEPv
 delete a;
   }
Index: clang/lib/CodeGen/CGExprCXX.cpp

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

2020-05-04 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment.

Agree that is a mistake in `NSItemProvider` API. The solution I offer is not to 
revert the change but to add cc1 flag 
`-fcompatibility-qualified-id-block-type-checking` and to make the driver for 
Darwin platforms to add this flag. Thus developers using Apple SDKs will see 
the old behavior and won't need to change their code. While everybody else will 
use clang with correct type checking. If any other platforms provide APIs 
relying on the old type checking, the solution for them is to tweak the driver.

The right `NSItemProvider` fix is to use `__kindof`, like `void (^)(__kindof 
id item, NSError *error);` It's purpose is to flip substitution 
principle, so that's the way to express API requirements in the type system. 
When we have a fix available in SDK, we can change the driver not to add the 
compatibility flag anymore and `NSItemProvider` should keep working without 
developers changing their code.

I'm going to work on the patch to add the compatibility flag, if you have any 
scenarios not covered by the offered solution, please let me know.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66831



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


[PATCH] D79276: [FileCheck] Support comment directives

2020-05-04 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added inline comments.



Comment at: llvm/docs/CommandGuide/FileCheck.rst:57
+ By default, FileCheck ignores any occurrence in ``match-filename`` of any 
check
+ prefix if it is preceded on the same line by "``COM:``" or "``RUN:``".  See 
the
+ section `The "COM:" directive`_ for usage details.

jhenderson wrote:
> Nit: the rest of this document uses single spaces after full stops. Please do 
> that in the new text too.
It doesn't matter to me which we use, especially given that it doesn't seem to 
affect how the HTML renders.  However, the majority of periods (ignoring `..` 
marking blocks) in this document that are followed by at least one space are 
followed by two spaces. Does that change your suggestion?



Comment at: llvm/lib/Support/FileCheck.cpp:1930
 
+bool FileCheck::ValidateCheckPrefixes() {
+  StringSet<> PrefixSet;

jhenderson wrote:
> It looks to me like the changes related to this function could be done 
> separately (perhaps first). Is that the case, and if so, could you do so? 
> It's a little hard to follow what's just refactoring of the existing stuff 
> and what is new, with the extra comment stuff thrown in.
Good idea: D79375



Comment at: llvm/test/FileCheck/comment.txt:38-43
+// RUN: echo 'foo COM: bar'> %t.in
+// RUN: echo 'CHECK: foo COM: bar' > %t.chk
+// RUN: %ProtectFileCheckOutput FileCheck -vv %t.chk < %t.in 2>&1 \
+// RUN: | FileCheck -check-prefix=WITHIN-CHECKS %s
+//
+// WITHIN-CHECKS: .chk:1:8: remark: CHECK: expected string found in input

jhenderson wrote:
> I'm struggling a little with this case. Firstly, why the '8' in the column 
> count in the remark? Secondly, if COM: was being treated as a genuine comment 
> here, then the check directive would become `CHECK: foo` which would still be 
> a match of the input, if I'm not mistaken? I guess the two might somehow be 
> related, but I don't know how if so.
> Firstly, why the '8' in the column count in the remark?

Are you asking why it's not `1`?  Here, FileCheck gives the location of the 
first character of the pattern from the check file.

> Secondly, if COM: was being treated as a genuine comment here, then the check 
> directive would become CHECK: foo which would still be a match of the input, 
> if I'm not mistaken?

Ah, thanks.  Fixed.




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

https://reviews.llvm.org/D79276



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


[PATCH] D79276: [FileCheck] Support comment directives

2020-05-04 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny updated this revision to Diff 261962.
jdenny marked 11 inline comments as done.
jdenny added a comment.

Addressed most reviewer comments.


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

https://reviews.llvm.org/D79276

Files:
  clang/test/CodeGen/default-address-space.c
  clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
  clang/test/Driver/hip-device-libs.hip
  llvm/docs/CommandGuide/FileCheck.rst
  llvm/include/llvm/Support/FileCheck.h
  llvm/lib/Support/FileCheck.cpp
  llvm/test/Assembler/drop-debug-info-nonzero-alloca.ll
  llvm/test/FileCheck/comment.txt
  llvm/test/FileCheck/first-character-match.txt
  llvm/test/FileCheck/validate-check-prefix.txt
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -44,6 +44,14 @@
 cl::desc(
 "Alias for -check-prefix permitting multiple comma separated values"));
 
+static cl::list CommentPrefixes(
+"comment-prefixes", cl::CommaSeparated, cl::Hidden,
+cl::desc("Comma-separated list of comment prefixes to use from check file\n"
+ "(defaults to 'COM,RUN'). Please avoid using this feature in\n"
+ "LLVM's LIT-based test suites, which should be easier to\n"
+ "maintain if they all follow a consistent comment style. This\n"
+ "feature is meant for non-LIT test suites using FileCheck."));
+
 static cl::opt NoCanonicalizeWhiteSpace(
 "strict-whitespace",
 cl::desc("Do not treat all horizontal whitespace as equivalent"));
@@ -279,6 +287,8 @@
 return "label";
   case Check::CheckEmpty:
 return "empty";
+  case Check::CheckComment:
+return "com";
   case Check::CheckEOF:
 return "eof";
   case Check::CheckBadNot:
@@ -565,6 +575,9 @@
   for (StringRef Prefix : CheckPrefixes)
 Req.CheckPrefixes.push_back(Prefix);
 
+  for (StringRef Prefix : CommentPrefixes)
+Req.CommentPrefixes.push_back(Prefix);
+
   for (StringRef CheckNot : ImplicitCheckNot)
 Req.ImplicitCheckNot.push_back(CheckNot);
 
Index: llvm/test/FileCheck/validate-check-prefix.txt
===
--- llvm/test/FileCheck/validate-check-prefix.txt
+++ llvm/test/FileCheck/validate-check-prefix.txt
@@ -8,6 +8,6 @@
 
 ; BAD_PREFIX: supplied check prefix must start with a letter and contain only alphanumeric characters, hyphens, and underscores: 'A!'
 
-; DUPLICATE_PREFIX: error: supplied check prefix must be unique among check prefixes: 'REPEAT'
+; DUPLICATE_PREFIX: error: supplied check prefix must be unique among check prefixes and comment prefixes: 'REPEAT'
 
 ; EMPTY_PREFIX: error: supplied check prefix must not be the empty string
Index: llvm/test/FileCheck/first-character-match.txt
===
--- llvm/test/FileCheck/first-character-match.txt
+++ llvm/test/FileCheck/first-character-match.txt
@@ -1,2 +1,2 @@
-RUN: FileCheck -check-prefix=RUN -input-file %s %s
+RUN: FileCheck -check-prefix=RUN --comment-prefixes=COM -input-file %s %s
 // Prefix is at the first character in the file. The run line then matches itself.
Index: llvm/test/FileCheck/comment.txt
===
--- /dev/null
+++ llvm/test/FileCheck/comment.txt
@@ -0,0 +1,175 @@
+// Comment directives successfully comment out check directives.
+//
+// Check all default comment prefixes.
+// Check that a comment directive at the beginning/end of the file is handled.
+// Check that the preceding/following line's check directive is not affected.
+// RUN: echo 'foo'>  %t-supresses1.in
+// RUN: echo 'COM: CHECK: bar'>  %t-supresses1.chk
+// RUN: echo 'CHECK: foo' >> %t-supresses1.chk
+// RUN: echo 'RUN: echo "CHECK: baz"' >> %t-supresses1.chk
+// RUN: %ProtectFileCheckOutput \
+// RUN: FileCheck -vv %t-supresses1.chk < %t-supresses1.in 2>&1 \
+// RUN: | FileCheck -check-prefix=SUPPRESSES-CHECKS -DCHECK_LINE=2 %s
+//
+// Check the case of one user-specified comment prefix.
+// Check that a comment directive not at the beginning of a line is handled.
+// RUN: echo 'foo'  >  %t-suppresses2.in
+// RUN: echo 'CHECK: foo'   >  %t-suppresses2.chk
+// RUN: echo 'letters then space MY-PREFIX: CHECK: bar' >> %t-suppresses2.chk
+// RUN: %ProtectFileCheckOutput \
+// RUN: FileCheck -vv %t-suppresses2.chk -comment-prefixes=MY-PREFIX \
+// RUN:   < %t-suppresses2.in 2>&1 \
+// RUN: | FileCheck -check-prefix=SUPPRESSES-CHECKS -DCHECK_LINE=1 %s
+//
+// Check the case of multiple user-specified comment prefixes.
+// RUN: echo 'foo'   >  %t-suppresses3.in
+// RUN: echo 'Bar_2: CHECK: Bar' >  %t-suppresses3.chk
+// RUN: echo 'CHECK: foo'>> %t-suppresses3.chk
+// RUN: echo 'Foo_1: CHECK: Foo' >> 

[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX

2020-05-04 Thread Jason Liu via Phabricator via cfe-commits
jasonliu marked 2 inline comments as done.
jasonliu added inline comments.



Comment at: clang/test/CodeGen/ppc32-dwarf.c:2
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm %s -o - | FileCheck 
%s
+// RUN: %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s --check-prefixes=CHECK,PPC32
+static unsigned char dwarf_reg_size_table[1024];

Xiangling_L wrote:
> Minor comment:
> Would `PPC32SVR4` compared to `PPC32` make the checking content clearer since 
> PPC32 actually includes AIX target?
Technically, it's PPC32 target except AIX (not restrict to SVR4). So PPC32SVR4 
is not that accurate either. 



Comment at: clang/test/CodeGen/ppc64-dwarf.c:2
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s --check-prefixes=CHECK,PPC64
 static unsigned char dwarf_reg_size_table[1024];

Xiangling_L wrote:
> Same comment as above.
> s/PPC64/PPC64SVR4?
Same above, and for PPC64 we have Darwin that's actually not SVR4.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79035



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


[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX

2020-05-04 Thread Jason Liu via Phabricator via cfe-commits
jasonliu marked 2 inline comments as done.
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4317
+  if (isAggregateTypeForABI(RetTy))
+return getNaturalAlignIndirect(RetTy);
+

Xiangling_L wrote:
> This method uses the ABI alignment of the given aggregate type which I think 
> is not ideal due to our AIX special alignment rule. We need to use preferred 
> alignment in this case.
> Btw also I think it's not necessary for you to rebase your patch on the power 
> alignment patch, I can refresh the testcase when I am dealing with that one.
As it is right now in master, there is no difference between natural alignment 
and preferred alignment for AIX. The tentative direction is to use preferred 
alignment to record the actual alignment on AIX, but it is not finalized yet. I 
would rather leave this part of the work for the patch that's going to 
implement the power alignment rule for AIX.



Comment at: clang/test/CodeGen/aix-vaargs.c:3
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -o - %s | FileCheck 
%s --check-prefixes=AIX-COM,AIX-M32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -o - %s | 
FileCheck %s --check-prefixes=AIX-COM,AIX-M64

Xiangling_L wrote:
> Consistent with other testcases to use `AIX32/AIX64`?
I chose AIX-M32/AIX-M64 mainly because the length is the same as AIX-COM so we 
don't need to worry about aligning the space. I would prefer to keep it that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79035



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


[PATCH] D79244: [Sema] Put existing warning under -Wexcess-initializers

2020-05-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

Should we stick ext_excess_initializers_in_char_array_initializer and 
ext_initializer_string_for_char_array_too_long in the same warning group?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79244



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


[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

2020-05-04 Thread Alina Sbirlea via Phabricator via cfe-commits
asbirlea accepted this revision.
asbirlea added a comment.
This revision is now accepted and ready to land.

Thank you for adding this!

Please update first sentence in the description to:

  We want to add a way to avoid merging identical calls so as to keep the 
separate debug-information for those calls. There is also an asan usecase where 
having this attribute would be beneficial to avoid alternative work-arounds.




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

https://reviews.llvm.org/D78659



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


[PATCH] D79236: [docs] Regenerate DiagnosticsReference.rst

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D79236#2018891 , @rsmith wrote:

> In D79236#2018812 , @rjmccall wrote:
>
> > In D79236#2018661 , @rsmith wrote:
> >
> > > We do generate this document with tblgen already. The problem is that 
> > > it's not automatically regenerated as part of the docs build on the 
> > > website, unlike some of our other similar tblgen-generated documentation. 
> > > I've asked for that to happen repeatedly for years but so far I have 
> > > achieved zero traction. So the best we have right now is to manually 
> > > regenerate this every once in a while and check it in. :-(
> >
> >
> > Do we need support from the website ops people on this, or do we just need 
> > to set up the build system properly to do it?
>
>
> My understanding is that there's a custom script on the website that runs 
> some `clang-tblgen` invocations as part of the docs build process, and it's 
> not something that we can modify by changing anything that's checked into 
> git. (I mean, we could hack around it by detecting one of the tablegen 
> invocations that's already performed and doing more work on the side, but 
> that's clearly the wrong way to get this to work...)


Okay.  So it doesn't just run `ninja docs-clang-html`?  That's unfortunate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79236



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


[PATCH] D79336: [analyzer] Generalize bitwise OR rules for ranges

2020-05-04 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:77
+  assert(!isEmpty());
+  // NOTE: It's a shame that we can't implement 'getMaxValue' without scanning
+  //   the whole tree to get to the last element.

vsavchenko wrote:
> xazax.hun wrote:
> > Yeah, this is quite unfortunate. But you might end up calling this a lot 
> > for bitwise operations. I wonder if it is worth to solve this problem 
> > before commiting this patch. I do not insist though. 
> I was even thinking about fixing it myself, this shouldn't be very hard, Also 
> addition of `lower_bound` and `upper_bound` can really help implementing 
> `Includes` for `RangeSet`.
Be aware as D77792 also implements such a method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79336



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


[PATCH] D79367: [CUDA][HIP] Fix empty ctor/dtor check for union

2020-05-04 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

Nice! Thank you for the patch.


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

https://reviews.llvm.org/D79367



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


[PATCH] D79372: [clang-format] [PR45126] Help text is missing all available formats

2020-05-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, sammccall, mitchell-stellar.
MyDeveloperDay added projects: clang, clang-format.

https://bugs.llvm.org/show_bug.cgi?id=45126

GNU and Microsoft styles are built in supported styles but are not displayed in 
the help text


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79372

Files:
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/clang-format-diff.py


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -52,8 +52,8 @@
   parser.add_argument('-v', '--verbose', action='store_true',
   help='be more verbose, ineffective without -i')
   parser.add_argument('-style',
-  help='formatting style to apply (LLVM, Google, Chromium, 
'
-  'Mozilla, WebKit)')
+  help='formatting style to apply (LLVM, GNU, Google, 
Chromium, '
+  'Microsoft, Mozilla, WebKit)')
   parser.add_argument('-binary', default='clang-format',
   help='location of binary to use for clang-format')
   args = parser.parse_args()
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2642,7 +2642,7 @@
 
 const char *StyleOptionHelpDescription =
 "Coding style, currently supports:\n"
-"  LLVM, Google, Chromium, Mozilla, WebKit.\n"
+"  LLVM, GNU, Google, Chromium, Microsoft, Mozilla, WebKit.\n"
 "Use -style=file to load style configuration from\n"
 ".clang-format file located in one of the parent\n"
 "directories of the source file (or current\n"


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -52,8 +52,8 @@
   parser.add_argument('-v', '--verbose', action='store_true',
   help='be more verbose, ineffective without -i')
   parser.add_argument('-style',
-  help='formatting style to apply (LLVM, Google, Chromium, '
-  'Mozilla, WebKit)')
+  help='formatting style to apply (LLVM, GNU, Google, Chromium, '
+  'Microsoft, Mozilla, WebKit)')
   parser.add_argument('-binary', default='clang-format',
   help='location of binary to use for clang-format')
   args = parser.parse_args()
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2642,7 +2642,7 @@
 
 const char *StyleOptionHelpDescription =
 "Coding style, currently supports:\n"
-"  LLVM, Google, Chromium, Mozilla, WebKit.\n"
+"  LLVM, GNU, Google, Chromium, Microsoft, Mozilla, WebKit.\n"
 "Use -style=file to load style configuration from\n"
 ".clang-format file located in one of the parent\n"
 "directories of the source file (or current\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D79213: [hip] Add noalias on restrict qualified coerced hip pointers

2020-05-04 Thread Austin Kerbow via Phabricator via cfe-commits
kerbowa added a comment.

In D79213#2018820 , @hliao wrote:

> Any more comments? As this should be a performance-critical issue, shall we 
> get conclusion and make progress for the next step?


We applied this current version of the patch internally for testing which is 
the reason for the lack of urgency, but hopefully we can get the final generic 
solution submitted upstream early this week. I believe I understand your 
comments and it makes sense to me that it should be safe for pointers whose 
coerced type is also a single pointer to retain the same attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79213



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


[PATCH] D79236: [docs] Regenerate DiagnosticsReference.rst

2020-05-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D79236#2018812 , @rjmccall wrote:

> In D79236#2018661 , @rsmith wrote:
>
> > We do generate this document with tblgen already. The problem is that it's 
> > not automatically regenerated as part of the docs build on the website, 
> > unlike some of our other similar tblgen-generated documentation. I've asked 
> > for that to happen repeatedly for years but so far I have achieved zero 
> > traction. So the best we have right now is to manually regenerate this 
> > every once in a while and check it in. :-(
>
>
> Do we need support from the website ops people on this, or do we just need to 
> set up the build system properly to do it?


My understanding is that there's a custom script on the website that runs some 
`clang-tblgen` invocations as part of the docs build process, and it's not 
something that we can modify by changing anything that's checked into git. (I 
mean, we could hack around it by detecting one of the tablegen invocations 
that's already performed and doing more work on the side, but that's clearly 
the wrong way to get this to work...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79236



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


[PATCH] D79054: [NFC] Improve documentation for -i and update example git one liner

2020-05-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

We should probably encourage people to use git-clang-format with git 
repositories.  It naturally doesn't have this sort of fragility because it 
integrates with the repository more tightly.




Comment at: clang/tools/clang-format/clang-format-diff.py:41
+  ' The filename contained in the diff is used unmodified'
+  ' to determine the source file to update')
   parser.add_argument('-p', metavar='NUM', default=0,

That isn't specific to -i. If you look more carefully at what the command is 
doing, it always goes back to the original source file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79054



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D79344#2018693 , @hliao wrote:

> OK, I will add one option, But, do we turn it on by default or off?


As a rule of thumb, if it's an experimental feature, then the default would be 
off. For a change which should be the default, but is risky, the default is on. 
This patch looks like the latter.

If you can wait, I can try patching this change into our clang tree and then 
see if it breaks anything obvious. If nothing falls apart, I'll be fine with 
the patch as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D79367: [CUDA][HIP] Fix empty ctor/dtor check for union

2020-05-04 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.

union ctor does not call ctors of its data members. union dtor does not call 
dtors of its data members.
Also union does not have base class.

https://godbolt.org/z/8RxZeG

Currently when clang checks whether union has an empty ctor/dtor, it checks the 
ctors/dtors of its
data members. This causes incorrectly diagnose device side global variables and 
shared variables as
having non-empty ctors/dtors.

This patch fixes that.


https://reviews.llvm.org/D79367

Files:
  clang/lib/Sema/SemaCUDA.cpp
  clang/test/SemaCUDA/union-init.cu


Index: clang/test/SemaCUDA/union-init.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/union-init.cu
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-linux-unknown -fsyntax-only 
-o - -verify
+
+#include "Inputs/cuda.h"
+
+struct A {
+  int a;
+  __device__ A() { a = 1; }
+  __device__ ~A() { a = 2; }
+};
+
+// This can be a global var since ctor/dtors of data members are not called.
+union B {
+  A a;
+  __device__ B() {}
+  __device__ ~B() {}
+};
+
+// This cannot be a global var since it has a dynamic ctor.
+union C {
+  A a;
+  __device__ C() { a.a = 3; }
+  __device__ ~C() {}
+};
+
+// This cannot be a global var since it has a dynamic dtor.
+union D {
+  A a;
+  __device__ D() { }
+  __device__ ~D() { a.a = 4; }
+};
+
+__device__ B b;
+__device__ C c;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, 
__constant__, and __shared__ variables.}}
+__device__ D d;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, 
__constant__, and __shared__ variables.}}
+
+__device__ void foo() {
+  __shared__ B b;
+  __shared__ C c;
+  // expected-error@-1 {{initialization is not supported for __shared__ 
variables.}}
+  __shared__ D d;
+  // expected-error@-1 {{initialization is not supported for __shared__ 
variables.}}
+}
Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -426,6 +426,10 @@
   if (CD->getParent()->isDynamicClass())
 return false;
 
+  // Union ctor does not call ctors of its data members.
+  if (CD->getParent()->isUnion())
+return true;
+
   // The only form of initializer allowed is an empty constructor.
   // This will recursively check all base classes and member initializers
   if (!llvm::all_of(CD->inits(), [&](const CXXCtorInitializer *CI) {
@@ -465,6 +469,11 @@
   if (ClassDecl->isDynamicClass())
 return false;
 
+  // Union does not have base class and union dtor does not call dtors of its
+  // data members.
+  if (DD->getParent()->isUnion())
+return true;
+
   // Only empty destructors are allowed. This will recursively check
   // destructors for all base classes...
   if (!llvm::all_of(ClassDecl->bases(), [&](const CXXBaseSpecifier ) {


Index: clang/test/SemaCUDA/union-init.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/union-init.cu
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-linux-unknown -fsyntax-only -o - -verify
+
+#include "Inputs/cuda.h"
+
+struct A {
+  int a;
+  __device__ A() { a = 1; }
+  __device__ ~A() { a = 2; }
+};
+
+// This can be a global var since ctor/dtors of data members are not called.
+union B {
+  A a;
+  __device__ B() {}
+  __device__ ~B() {}
+};
+
+// This cannot be a global var since it has a dynamic ctor.
+union C {
+  A a;
+  __device__ C() { a.a = 3; }
+  __device__ ~C() {}
+};
+
+// This cannot be a global var since it has a dynamic dtor.
+union D {
+  A a;
+  __device__ D() { }
+  __device__ ~D() { a.a = 4; }
+};
+
+__device__ B b;
+__device__ C c;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+__device__ D d;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+__device__ void foo() {
+  __shared__ B b;
+  __shared__ C c;
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+  __shared__ D d;
+  // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+}
Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -426,6 +426,10 @@
   if (CD->getParent()->isDynamicClass())
 return false;
 
+  // Union ctor does not call ctors of its data members.
+  if (CD->getParent()->isUnion())
+return true;
+
   // The only form of initializer allowed is an empty constructor.
   // This will recursively check all base classes and member initializers
   if (!llvm::all_of(CD->inits(), [&](const CXXCtorInitializer *CI) {
@@ -465,6 +469,11 @@
   if (ClassDecl->isDynamicClass())
 return false;
 
+  // 

[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX

2020-05-04 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4179
+  // This is calculated from the LLVM and GCC tables and verified
+  // against gcc output.  AFAIK all ABIs use the same encoding.
+

Minor comment about comment style:
Though I noticed that "AFAIK all ABIs use the same encoding." is from original 
code, could we adjust it to something like "All PPC ABIs use the same encoding."



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4194
+
+  // 64-76 are various 4-byte or 8-byte special-purpose registers:
+  // 64: mq

s/64-76/64-67?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4317
+  if (isAggregateTypeForABI(RetTy))
+return getNaturalAlignIndirect(RetTy);
+

This method uses the ABI alignment of the given aggregate type which I think is 
not ideal due to our AIX special alignment rule. We need to use preferred 
alignment in this case.
Btw also I think it's not necessary for you to rebase your patch on the power 
alignment patch, I can refresh the testcase when I am dealing with that one.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4336
+if (CGCXXABI::RecordArgABI RAA = getRecordArgABI(Ty, getCXXABI()))
+  return getNaturalAlignIndirect(Ty, RAA == CGCXXABI::RAA_DirectInMemory);
+

Same comment like above.



Comment at: clang/test/CodeGen/aix-vaargs.c:3
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -o - %s | FileCheck 
%s --check-prefixes=AIX-COM,AIX-M32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -o - %s | 
FileCheck %s --check-prefixes=AIX-COM,AIX-M64

Consistent with other testcases to use `AIX32/AIX64`?



Comment at: clang/test/CodeGen/ppc32-and-aix-struct-return.c:8
+// RUN:   -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
+// RUN: %clang_cc1 -triple powerpc-unknown-linux \
+// RUN:   -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX

Do you mean to check AIX or SVR4?



Comment at: clang/test/CodeGen/ppc32-dwarf.c:2
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm %s -o - | FileCheck 
%s
+// RUN: %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s --check-prefixes=CHECK,PPC32
+static unsigned char dwarf_reg_size_table[1024];

Minor comment:
Would `PPC32SVR4` compared to `PPC32` make the checking content clearer since 
PPC32 actually includes AIX target?



Comment at: clang/test/CodeGen/ppc64-dwarf.c:2
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s --check-prefixes=CHECK,PPC64
 static unsigned char dwarf_reg_size_table[1024];

Same comment as above.
s/PPC64/PPC64SVR4?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79035



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


[PATCH] D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

2020-05-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.
Herald added a reviewer: aaron.ballman.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76646



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


[PATCH] D79121: Add nomerge function attribute to clang

2020-05-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:611
 void CodeGenFunction::EmitAttributedStmt(const AttributedStmt ) {
+  for (const auto *A: S.getAttrs())
+if (A->getKind() == attr::NoMerge) {

Can we use S.hasAttr, or does that not exist for statements?



Comment at: clang/lib/CodeGen/CGStmt.cpp:615
+  break;
+}
   EmitStmt(S.getSubStmt(), S.getAttrs());

Please use SaveAndRestore, since this could be re-entrant. Consider cases like:
  [[clang::nomerge]] foo(1, ({bar(); [[clang::nomerge]] baz(); 2}), qux());

The inner nomerge will "unset" the outer one too early.

This is similar to what will happen if we allow nomerge on compound statements 
as in:
  [[clang::nomerge]] if (cond) {
 foo();
 [[clang::nomerge}] foo();
 foo(); // will not carry nomerge unless we restore to old value
  }



Comment at: clang/lib/CodeGen/CodeGenFunction.h:599
   /// region.
   bool IsInPreservedAIRegion = false;
 

I would put your field after this one, and name it more specific, something 
like `InNoMergeAttributedStmt`. This field tracks a similar concept of being 
within a region



Comment at: clang/test/CodeGen/attr-nomerge.cpp:6
+
+void foo(int i) {
+  [[clang::nomerge]] bar();

aaron.ballman wrote:
> I'd appreciate seeing some more complex tests showing the behavior of the 
> attribute. As some examples:
> ```
> // Does this work on dynamically initialized global statics?
> [[clang::nomerge]] static int i = foo() + bar();
> 
> void func() {
>   // Is the lambda function call operator nomerge, the contained calls within 
> the lambda, or none of the above?
>   [[clang::nomerge]] [] { foo(); bar(); }(); 
> 
>   // Do we mark the implicit function calls to begin()/end() as nomerge?
>   [[clang::nomerge]] for (auto foo : some_container) {}
> 
>   // Does it apply to builtins as well?
>   [[clang::nomerge]] foo(__builtin_strlen("bar"), __builtin_strlen("bar"));
> 
>   // Does it apply to the substatement of a label?
>   [[clang::nomerge]] how_about_this: f(bar(), bar());
> 
>   // Does it apply across the components of a for statement?
>   for (foo(); bar(); baz()) {}
> }
> ```
I think the emergent behavior from the current implementation is reasonable. 
For lambdas, it applies to the outer call, not the inner lambda body.

I don't see any reason to make this work for global variables currently.

We could try to ban the application of this attribute to compound statements 
and statments containing them (`{}`, `if`, `for`), but that's a lot of effort. 
I could go either way.


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

https://reviews.llvm.org/D79121



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


[PATCH] D79336: [analyzer] Generalize bitwise OR rules for ranges

2020-05-04 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko marked 3 inline comments as done.
vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:77
+  assert(!isEmpty());
+  // NOTE: It's a shame that we can't implement 'getMaxValue' without scanning
+  //   the whole tree to get to the last element.

xazax.hun wrote:
> Yeah, this is quite unfortunate. But you might end up calling this a lot for 
> bitwise operations. I wonder if it is worth to solve this problem before 
> commiting this patch. I do not insist though. 
I was even thinking about fixing it myself, this shouldn't be very hard, Also 
addition of `lower_bound` and `upper_bound` can really help implementing 
`Includes` for `RangeSet`.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:388
+  /// it will return the range [x_0, y_N].
+  static Range roughen(RangeSet Origin) {
+assert(!Origin.isEmpty());

xazax.hun wrote:
> Is `roughen` part of a nomenclature? If not, what about something like 
> `cover`?
Yeah  I spend quite some time thinking about a proper name for that one. It is 
used to be `coarsen` (yep, yuck!), but I don't really feel "cover" either. Mb 
something like "unify" will work? What do you think?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:418
+
+auto ConvertedCoarseLHS = convert(CoarseLHS, ResultType);
+auto ConvertedCoarseRHS = convert(CoarseRHS, ResultType);

xazax.hun wrote:
> Why do we need this conversion?
> Do we want to model a cast in the code somewhere?
> If so, I think this is more like a workaround and in the future we would need 
> an explicit way to represent those cast operations. It might be worth to have 
> a TODO.
Those ranges that we inferred have nothing to do with upper operations.  I 
mean, in theory, `VisitBinaryOperator` can take care of conversions, but I 
don't know how it can be done in the most generic way.  In this particular 
situation, we really care that `From` and `To` don't change signs or something 
like that after conversion, but for other operators it might be not so 
important.

Answering the question of why we even need those, putting it simple - there is 
no other way, this is how `APSInt` works, couldn't do anything with values of 
different types (e.g. comparison).

I am totally on board with you on the inconvenience of the whole thing.  Every 
implementer of the `Visit*` method should take care of conversions on their 
own.  I would say that `SValBuilder` adding special symbolic values for this 
kind of casts, can be a solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79336



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


[PATCH] D79213: [hip] Add noalias on restrict qualified coerced hip pointers

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

Any more comments? As this should be a performance-critical issue, shall we get 
conclusion and make progress for the next step?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79213



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


[PATCH] D79236: [docs] Regenerate DiagnosticsReference.rst

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D79236#2018661 , @rsmith wrote:

> In D79236#2015982 , @rjmccall wrote:
>
> > There's no good reason for this file to exist; documenting the warning 
> > options is useful, but the file should be generated as an intermediate 
> > output of the documentation build and then ultimately fed into Sphinx.
> >
> > There's even a TODO in the docs about it:
> >
> > > TODO: Generate this from tblgen. Define one anchor per warning group.
>
>
> We do generate this document with tblgen already. The problem is that it's 
> not automatically regenerated as part of the docs build on the website, 
> unlike some of our other similar tblgen-generated documentation. I've asked 
> for that to happen repeatedly for years but so far I have achieved zero 
> traction. So the best we have right now is to manually regenerate this every 
> once in a while and check it in. :-(


Do we need support from the website ops people on this, or do we just need to 
set up the build system properly to do it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79236



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


[PATCH] D79337: Silence warnings when compiling x86 with latest MSVC

2020-05-04 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

Will do, thank you for your time!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79337



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


[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2020-05-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 261928.
echristo edited the summary of this revision.
echristo added a comment.
Herald added a subscriber: zzheng.

Add a testcase with opt and command line option so we can enable it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71687

Files:
  clang/test/Misc/loop-opt-setup.c
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Transforms/LoopUnroll/FullUnroll.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -100,6 +100,11 @@
  "the OptimizerLast extension point into default pipelines"),
 cl::Hidden);
 
+// Individual pipeline tuning options.
+static cl::opt DisableLoopUnrolling(
+"new-pm-disable-loop-unrolling",
+cl::desc("Disable loop unrolling in all relevant passes"), cl::init(false));
+
 extern cl::opt PGOKindFlag;
 extern cl::opt ProfileFile;
 extern cl::opt CSPGOKindFlag;
@@ -260,6 +265,10 @@
   SI.registerCallbacks(PIC);
 
   PipelineTuningOptions PTO;
+  // LoopUnrolling defaults on to true and DisableLoopUnrolling is initialized
+  // to false above so we shouldn't necessarily need to check whether or not the
+  // option has been enabled.
+  PTO.LoopUnrolling = !DisableLoopUnrolling;
   PTO.Coroutines = Coroutines;
   PassBuilder PB(TM, PTO, P, );
   registerEPCallbacks(PB, VerifyEachPass, DebugPM);
Index: llvm/test/Transforms/LoopUnroll/FullUnroll.ll
===
--- /dev/null
+++ llvm/test/Transforms/LoopUnroll/FullUnroll.ll
@@ -0,0 +1,81 @@
+; RUN: opt -passes='default' -disable-verify --mtriple x86_64-pc-linux-gnu -new-pm-disable-loop-unrolling=true \
+; RUN: -S -o - %s | FileCheck %s
+
+; We don't end up deleting the loop, but we remove everything inside of it so checking for any
+; reasonable instruction from the original loop will work.
+; CHECK-NOT: br i1
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+$_Z6Helperv = comdat any
+
+; Function Attrs: noinline optnone uwtable
+define dso_local void @_Z3Runv() #0 {
+entry:
+  call void @_Z6Helperv()
+  ret void
+}
+
+; Function Attrs: noinline nounwind optnone uwtable
+define linkonce_odr dso_local void @_Z6Helperv() #1 comdat {
+entry:
+  %nodes = alloca [5 x i32*], align 16
+  %num_active = alloca i32, align 4
+  %i = alloca i32, align 4
+  store i32 5, i32* %num_active, align 4
+  br label %while.cond
+
+while.cond:   ; preds = %for.end, %entry
+  %0 = load i32, i32* %num_active, align 4
+  %tobool = icmp ne i32 %0, 0
+  br i1 %tobool, label %while.body, label %while.end
+
+while.body:   ; preds = %while.cond
+  store i32 0, i32* %i, align 4
+  br label %for.cond
+
+for.cond: ; preds = %for.inc, %while.body
+  %1 = load i32, i32* %i, align 4
+  %cmp = icmp slt i32 %1, 5
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body: ; preds = %for.cond
+  %2 = load i32, i32* %i, align 4
+  %idxprom = sext i32 %2 to i64
+  %arrayidx = getelementptr inbounds [5 x i32*], [5 x i32*]* %nodes, i64 0, i64 %idxprom
+  %3 = load i32*, i32** %arrayidx, align 8
+  %tobool1 = icmp ne i32* %3, null
+  br i1 %tobool1, label %if.then, label %if.end
+
+if.then:  ; preds = %for.body
+  %4 = load i32, i32* %num_active, align 4
+  %dec = add nsw i32 %4, -1
+  store i32 %dec, i32* %num_active, align 4
+  br label %if.end
+
+if.end:   ; preds = %if.then, %for.body
+  br label %for.inc
+
+for.inc:  ; preds = %if.end
+  %5 = load i32, i32* %i, align 4
+  %inc = add nsw i32 %5, 1
+  store i32 %inc, i32* %i, align 4
+  br label %for.cond, !llvm.loop !2
+
+for.end:  ; preds = %for.cond
+  br label %while.cond
+
+while.end:; preds = %while.cond
+  ret void
+}
+
+attributes #0 = { noinline optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" 

[PATCH] D79265: Let normalize() for posix style convert backslash to slash unconditionally.

2020-05-04 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment.

What happens if you encounter a `"\t"` as a string?  This would convert that to 
a `"/t"` would it not?  Although, I suppose that in practice the treatment of 
escaped characters is not important.  I think I still prefer the `} else {` 
here over the early return.


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

https://reviews.llvm.org/D79265



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


[PATCH] D79337: Silence warnings when compiling x86 with latest MSVC

2020-05-04 Thread Vedant Kumar via Phabricator via cfe-commits
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks, this lgtm. If you could split this up into two commits before landing 
it, I'd appreciate it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79337



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

In D79344#2018683 , @tra wrote:

> In D79344#2018628 , @hliao wrote:
>
> > In D79344#2018561 , @tra wrote:
> >
> > > This has a good chance of breaking existing code. It would be great to 
> > > add an escape hatch option to revert to the old behavior if we run into 
> > > problems. The change is relatively simple, so reverting it in case 
> > > something goes wrong should work, too. Up to you.
> >
> >
> > Why? for the cases addressed in this patch, if there is existing code, it 
> > won't be compiled to generate module file due to the missing symbol. 
> > Anything missing?
>
>
> Logistics, mostly.
>
> Overloading is a rather fragile area of CUDA. This is the area where clang 
> and NVCC behave differently. Combined with the existing code that needs to 
> work with both compilers, even minor changes in compiler behavior can result 
> in unexpected issues. Stricter checks tend to expose existing code which 
> happens to work (or to compile) when it should not have, but it's not always 
> trivial to fix those quickly. Having an escape hatch allows us to deal with 
> those issues. It allows the owner of the code to reproduce the problem while 
> the rest of the world continues to work. Reverting is suboptimal as the end 
> user is often not in a good position to build a compiler with your patch 
> plumbed in and then plumb the patched compiler into their build system. 
> Adding another compiler option to enable/disable the new behavior is much 
> more manageable.


OK, I will add one option, But, do we turn it on by default or off?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations

2020-05-04 Thread Melanie Blower via Phabricator via cfe-commits
mibintc updated this revision to Diff 261921.
mibintc added a comment.

I fixed the issues that @rjmccall mentioned.  I don't yet have an answer for 
@scanon, need to get back to you about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/fp-reassoc-pragma.cpp
  clang/test/Parser/pragma-fp-contract.c
  clang/test/Parser/pragma-fp.cpp

Index: clang/test/Parser/pragma-fp.cpp
===
--- clang/test/Parser/pragma-fp.cpp
+++ clang/test/Parser/pragma-fp.cpp
@@ -1,14 +1,14 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
 
 void test_0(int *List, int Length) {
-/* expected-error@+1 {{missing option; expected contract}} */
+/* expected-error@+1 {{missing option; expected 'contract' or 'reassociate'}} */
 #pragma clang fp
   for (int i = 0; i < Length; i++) {
 List[i] = i;
   }
 }
 void test_1(int *List, int Length) {
-/* expected-error@+1 {{invalid option 'blah'; expected contract}} */
+/* expected-error@+1 {{invalid option 'blah'; expected 'contract' or 'reassociate'}} */
 #pragma clang fp blah
   for (int i = 0; i < Length; i++) {
 List[i] = i;
@@ -24,7 +24,7 @@
 }
 
 void test_4(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
 #pragma clang fp contract(while)
   for (int i = 0; i < Length; i++) {
 List[i] = i;
@@ -32,7 +32,7 @@
 }
 
 void test_5(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
 #pragma clang fp contract(maybe)
   for (int i = 0; i < Length; i++) {
 List[i] = i;
Index: clang/test/Parser/pragma-fp-contract.c
===
--- clang/test/Parser/pragma-fp-contract.c
+++ clang/test/Parser/pragma-fp-contract.c
@@ -23,3 +23,18 @@
 // expected-error@+1 {{this pragma cannot appear in union declaration}}
 #pragma STDC FP_CONTRACT ON
 };
+
+float fp_reassoc_fail(float a, float b) {
+  // CHECK-LABEL: fp_reassoc_fail
+  // expected-error@+2{{'#pragma clang fp' can only appear at file scope or at the start of a compound statement}}
+  float c = a + b;
+#pragma clang fp reassociate(on)
+  return c - b;
+}
+
+float fp_reassoc_no_fast(float a, float b) {
+// CHECK-LABEL: fp_reassoc_no_fast
+// expected-error@+1{{unexpected argument 'fast' to '#pragma clang fp reassociate'; expected 'on' or 'off'}}
+#pragma clang fp reassociate(fast)
+  return a - b;
+}
Index: clang/test/CodeGen/fp-reassoc-pragma.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-reassoc-pragma.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+// Simple case
+float fp_reassoc_simple(float a, float b, float c) {
+// CHECK: _Z17fp_reassoc_simplefff
+// CHECK: %[[M:.+]] = fmul reassoc float %a, %b
+// CHECK-NEXT: fadd reassoc float %[[M]], %c
+#pragma clang fp reassociate(on)
+  return a * b + c;
+}
+
+// Reassoc pragma should only apply to its scope
+float fp_reassoc_scoped(float a, float b, float c) {
+  // CHECK: _Z17fp_reassoc_scopedfff
+  // CHECK: %[[M:.+]] = fmul float %a, %b
+  // CHECK-NEXT: fadd float %[[M]], %c
+  {
+#pragma clang fp reassociate(on)
+  }
+  return a * b + c;
+}
+
+// Reassoc pragma should apply to templates as well
+class Foo {};
+Foo operator+(Foo, Foo);
+template 
+T template_reassoc(T a, T b, T c) {
+#pragma clang fp reassociate(on)
+  return ((a + b) - c) + c;
+}
+
+float fp_reassoc_template(float a, float b, float c) {
+  // CHECK: _Z19fp_reassoc_templatefff
+  // CHECK: %[[A1:.+]] = fadd reassoc float %a, %b
+  // CHECK-NEXT: %[[A2:.+]] = fsub reassoc float %[[A1]], %c
+  // CHECK-NEXT: fadd reassoc float %[[A2]], %c
+  return template_reassoc(a, b, c);
+}
+
+// File Scoping should work across functions
+#pragma clang fp reassociate(on)
+float fp_file_scope_on(float a, float b, float c) {
+  // CHECK: _Z16fp_file_scope_onfff
+  // CHECK: %[[M1:.+]] = fmul reassoc float %a, %c
+  // CHECK-NEXT: %[[M2:.+]] = fmul reassoc float %b, %c
+  // CHECK-NEXT: fadd reassoc float %[[M1]], %[[M2]]
+  return (a * c) + (b * c);
+}
+
+// Inner pragma has precedence
+float 

[PATCH] D78750: [SveEmitter] Add builtins for svdupq and svdupq_lane

2020-05-04 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added a comment.

I have reverted the patch in the meantime


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78750



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


[PATCH] D78750: [SveEmitter] Add builtins for svdupq and svdupq_lane

2020-05-04 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added a comment.

In D78750#2018673 , @thakis wrote:

> Looks like this breaks check-clang on Windows:
>  http://45.33.8.238/win/14483/step_7.txt
>
> And some other bots:
>  
> http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/37883/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Aacle_sve_dupq.c
>  
> http://lab.llvm.org:8011/builders/llvm-avr-linux/builds/993/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Aacle_sve_dupq.c
>
> Please take a look, and revert for now if fixing takes a while.


I suspect some 'CHECK' lines should be CHECK-DAG lines, but I'll investigate 
that tomorrow. Thanks for pointing out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78750



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


[clang] 90f3f62 - Revert "[SveEmitter] Add builtins for svdupq and svdupq_lane"

2020-05-04 Thread Sander de Smalen via cfe-commits

Author: Sander de Smalen
Date: 2020-05-04T21:31:55+01:00
New Revision: 90f3f62cb087782fe2608e95d686c29067281b6e

URL: 
https://github.com/llvm/llvm-project/commit/90f3f62cb087782fe2608e95d686c29067281b6e
DIFF: 
https://github.com/llvm/llvm-project/commit/90f3f62cb087782fe2608e95d686c29067281b6e.diff

LOG: Revert "[SveEmitter] Add builtins for svdupq and svdupq_lane"

It seems this patch broke some buildbots, so reverting until I
have had a chance to investigate.

This reverts commit 6b90a6887d25e3375bb916a3ed09f7ccec819d0c.

Added: 


Modified: 
clang/include/clang/Basic/arm_sve.td
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/utils/TableGen/SveEmitter.cpp

Removed: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c



diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index 2d2a09d4524d..bde26aed43f6 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -610,13 +610,6 @@ def SVPRFW_GATHER_BASES_OFFSET : 
MInst<"svprfw_gather[_{2}base]_index",  "vPdlJ"
 def SVPRFD_GATHER_BASES_OFFSET : MInst<"svprfd_gather[_{2}base]_index",  
"vPdlJ", "UiUl", [IsGatherPrefetch], MemEltTyInt64, 
"aarch64_sve_prfd_gather_scalar_offset">;
 
 

-// Scalar to vector
-
-def SVDUPQ_8  : SInst<"svdupq[_n]_{d}", "d",  "cUc", 
MergeNone>;
-def SVDUPQ_16 : SInst<"svdupq[_n]_{d}", "d",  "sUsh", MergeNone>;
-def SVDUPQ_32 : SInst<"svdupq[_n]_{d}", "d",  "iUif", MergeNone>;
-def SVDUPQ_64 : SInst<"svdupq[_n]_{d}", "dss",  "lUld", MergeNone>;
-
 // Integer arithmetic
 
 multiclass SInstZPZ flags=[]> {
@@ -1041,7 +1034,7 @@ def SVCLASTB : SInst<"svclastb[_{d}]","dPdd", 
"csilUcUsUiUlhfd", MergeNo
 def SVCLASTB_N   : SInst<"svclastb[_n_{d}]",  "sPsd", "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_clastb_n">;
 def SVCOMPACT: SInst<"svcompact[_{d}]",   "dPd",  "ilUiUlfd",
MergeNone, "aarch64_sve_compact">;
 //  SVDUP_LANE(to land in D78750)
-def SVDUPQ_LANE  : SInst<"svdupq_lane[_{d}]", "ddn",  "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_dupq_lane">;
+//  SVDUPQ_LANE   (to land in D78750)
 def SVEXT: SInst<"svext[_{d}]",   "dddi", "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_ext", [], [ImmCheck<2, ImmCheckExtract, 1>]>;
 def SVLASTA  : SInst<"svlasta[_{d}]", "sPd",  "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_lasta">;
 def SVLASTB  : SInst<"svlastb[_{d}]", "sPd",  "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_lastb">;
@@ -1079,12 +1072,6 @@ def SVPFALSE : SInst<"svpfalse[_b]", "P", "", MergeNone, 
"", [IsOverloadNone]>;
 def SVPTRUE_PAT : SInst<"svptrue_pat_{d}", "PI", "PcPsPiPl", MergeNone, 
"aarch64_sve_ptrue">;
 def SVPTRUE : SInst<"svptrue_{d}", "P",  "PcPsPiPl", MergeNone, 
"aarch64_sve_ptrue", [IsAppendSVALL]>;
 
-def SVDUPQ_B8  : SInst<"svdupq[_n]_{d}",  "P",  "Pc", 
MergeNone>;
-def SVDUPQ_B16 : SInst<"svdupq[_n]_{d}", "P",  "Ps", MergeNone>;
-def SVDUPQ_B32 : SInst<"svdupq[_n]_{d}", "P",  "Pi", MergeNone>;
-def SVDUPQ_B64 : SInst<"svdupq[_n]_{d}", "Pss",  "Pl", MergeNone>;
-
-
 

 // Predicate operations
 

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 797fcc6deea3..94c0adfdf4af 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -7562,15 +7562,6 @@ CodeGenFunction::getSVEPredType(SVETypeFlags TypeFlags) {
 return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 4);
   case SVETypeFlags::EltTyFloat64:
 return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 2);
-
-  case SVETypeFlags::EltTyBool8:
-return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 16);
-  case SVETypeFlags::EltTyBool16:
-return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 8);
-  case SVETypeFlags::EltTyBool32:
-return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 4);
-  case SVETypeFlags::EltTyBool64:
-return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 2);
   }
 }
 
@@ -7608,12 +7599,6 @@ CodeGenFunction::getSVEType(const SVETypeFlags 
) {
   }
 }
 
-llvm::Value *CodeGenFunction::EmitSVEAllTruePred(SVETypeFlags TypeFlags) {
-  Function *Ptrue =
-  CGM.getIntrinsic(Intrinsic::aarch64_sve_ptrue, 
getSVEPredType(TypeFlags));
-  return Builder.CreateCall(Ptrue, {Builder.getInt32(/*SV_ALL*/ 31)});
-}
-
 constexpr unsigned SVEBitsPerBlock = 128;
 
 static llvm::ScalableVectorType *getSVEVectorForElementType(llvm::Type *EltTy) 
{
@@ -8022,64 +8007,6 @@ Value 
*CodeGenFunction::EmitAArch64SVEBuiltinExpr(unsigned BuiltinID,
 return Builder.CreateCall(F, {Ops[0], Ops[1], Ops[0]});
   }
 
-  case SVE::BI__builtin_sve_svdupq_n_b8:
-  case 

[PATCH] D78750: [SveEmitter] Add builtins for svdupq and svdupq_lane

2020-05-04 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Looks like this breaks check-clang on Windows:
http://45.33.8.238/win/14483/step_7.txt

And some other bots:
http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/37883/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Aacle_sve_dupq.c
http://lab.llvm.org:8011/builders/llvm-avr-linux/builds/993/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Aacle_sve_dupq.c

Please take a look, and revert for now if fixing takes a while.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78750



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


[PATCH] D78190: Add Bfloat IR type

2020-05-04 Thread Francesco Petrogalli via Phabricator via cfe-commits
fpetrogalli added a comment.

Hi @stuij,

thank you for working on this!

Admittedly, I don't know much about the Asm parser, but I have left some 
comments anyway.

1. Shouldn't we test also that the parser is happy with the following 
expressions?

  bfloat *
  %... = fadd  %..., %... 
  similar for 

Or is this not needed, or left to be done in a separate patch?

2. Would it make sense to to split this patch into 2 separate patches? One that 
defines the enums and interfaces for `bfloat`, and one that does the actual 
parsing/emission in the IR? I suspect there is much intertwine going on, so 
probably not - in that case, I am happy for everything to go via a single patch.

3. Do you need those changes in the Hexagon and x86 backend? Could they be 
submitted separately, with some testing?

Kind regards,

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78190



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


[PATCH] D79274: Fix template class debug info for Visual Studio visualizers

2020-05-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

I think testing it is a matter of adding later C++ standard versions to the 
existing test here: 
`../clang/test/CodeGenCXX/debug-info-codeview-display-name.cpp`


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

https://reviews.llvm.org/D79274



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D79344#2018628 , @hliao wrote:

> In D79344#2018561 , @tra wrote:
>
> > This has a good chance of breaking existing code. It would be great to add 
> > an escape hatch option to revert to the old behavior if we run into 
> > problems. The change is relatively simple, so reverting it in case 
> > something goes wrong should work, too. Up to you.
>
>
> Why? for the cases addressed in this patch, if there is existing code, it 
> won't be compiled to generate module file due to the missing symbol. Anything 
> missing?


Logistics, mostly.

Overloading is a rather fragile area of CUDA. This is the area where clang and 
NVCC behave differently. Combined with the existing code that needs to work 
with both compilers, even minor changes in compiler behavior can result in 
unexpected issues. Stricter checks tend to expose existing code which happens 
to work (or to compile) when it should not have, but it's not always trivial to 
fix those quickly. Having an escape hatch allows us to deal with those issues. 
It allows the owner of the code to reproduce the problem while the rest of the 
world continues to work. Reverting is suboptimal as the end user is often not 
in a good position to build a compiler with your patch plumbed in and then 
plumb the patched compiler into their build system. Adding another compiler 
option to enable/disable the new behavior is much more manageable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D79357: [SveEmitter] Add builtins for svdup and svindex

2020-05-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: clang/include/clang/Basic/arm_sve.td:1050
 def SVCOMPACT: SInst<"svcompact[_{d}]",   "dPd",  "ilUiUlfd",
MergeNone, "aarch64_sve_compact">;
-//  SVDUP_LANE(to land in D78750)
+def SVDUP_LANE   : SInst<"svdup_lane[_{d}]",  "ddL",  "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_tbl">; // Implemented using tbl
 def SVDUPQ_LANE  : SInst<"svdupq_lane[_{d}]", "ddn",  "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_dupq_lane">;

"Implemented using tbl" isn't really a helpful comment.

I guess you're doing it this way so the lane doesn't have to be a constant?  
(And then I guess you can optimize it in the backend.)



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:8047
+Value *PFalse = Constant::getNullValue(PTrue->getType());
+Value *Sel = Builder.CreateSelect(CmpNE, PTrue, PFalse);
+return EmitSVEPredicateCast(Sel, cast(Ty));

The select might not be the best approach. I mean, it's semantically correct, 
but the backend currently doesn't know how to lower a select like that.  And 
even if it did, it's probably better to avoid generating it for now.

Maybe this should use whilelo directly, since that's what the backend would 
eventually use anyway.


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

https://reviews.llvm.org/D79357



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao marked an inline comment as done.
hliao added inline comments.



Comment at: clang/lib/Sema/SemaCUDA.cpp:156
+  if (VD->getType()->isCUDADeviceBuiltinSurfaceType() ||
+  VD->getType()->isCUDADeviceBuiltinTextureType())
+return CFT_HostDevice;

yaxunl wrote:
> We may need to mark constexpr variables as host device too. In practice such 
> usage has exist for long time.
`cosntexpr` variable is a little bit tricky as it's still possible for that 
variable to be finally emitted as a variable. For example, if its address is 
taken, it won't be optimized away and still needs emitting somewhere. But, like 
other non-local variables, CUDA forbids their initializers. Any suggestion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/test/SemaCUDA/variable-target.cu:42
+  return 0;
+}

hliao wrote:
> yaxunl wrote:
> > we need to have a test to check captured local host variable is allowed in 
> > device lambda.
> > 
> > we need to have some test for constexpr variables used in device function.
> This patch just addresses the direct address of variables. For capture, it 
> would be better to start with another patch.
but there are chances that this patch may break valid usage of captured 
variables in device lambda. At least we should add test to avoid that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D78232: [OPENMP50]Codegen for scan directive in simd loops.

2020-05-04 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev updated this revision to Diff 261917.
ABataev added a comment.

Rebase and simplify


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78232

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/scan_codegen.cpp
  clang/test/OpenMP/scan_messages.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2374,6 +2374,14 @@
   for (auto *E : C->reduction_ops()) {
 Visitor->AddStmt(E);
   }
+  if (C->getModifier() == clang::OMPC_REDUCTION_inscan) {
+for (auto *E : C->copy_temps()) {
+  Visitor->AddStmt(E);
+}
+for (auto *E : C->copy_ops()) {
+  Visitor->AddStmt(E);
+}
+  }
 }
 void OMPClauseEnqueue::VisitOMPTaskReductionClause(
 const OMPTaskReductionClause *C) {
Index: clang/test/OpenMP/scan_messages.cpp
===
--- clang/test/OpenMP/scan_messages.cpp
+++ clang/test/OpenMP/scan_messages.cpp
@@ -19,32 +19,32 @@
 #pragma omp for simd reduction(inscan, +: argc)
   for (int i = 0; i < 10; ++i)
 if (argc)
-#pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}}
+#pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}} expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
 if (argc) {
 #pragma omp scan inclusive(argc) // expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
 }
 #pragma omp simd reduction(inscan, +: argc)
   for (int i = 0; i < 10; ++i)
   while (argc)
-#pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}}
+#pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}} expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
 while (argc) {
 #pragma omp scan inclusive(argc) // expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
 }
 #pragma omp simd reduction(inscan, +: argc)
   for (int i = 0; i < 10; ++i)
   do
-#pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}}
+#pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}} expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
 while (argc)
   ;
-#pragma omp simd reduction(inscan, +: argc)
+#pragma omp simd reduction(inscan, +: argc) // expected-error {{the inscan reduction list item must appear as a list item in an 'inclusive' or 'exclusive' clause on an inner 'omp scan' directive}}
   for (int i = 0; i < 10; ++i)
   do {
-#pragma omp scan inclusive(argc)
+#pragma omp scan inclusive(argc) // expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
   } while (argc);
 #pragma omp simd reduction(inscan, +: argc)
   for (int i = 0; i < 10; ++i)
   switch (argc)
-#pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}}
+#pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}} expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
 switch (argc)
 case 1:
 #pragma omp scan inclusive(argc) // expected-error {{'#pragma omp scan' cannot be an immediate substatement}} expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for 

[PATCH] D79236: [docs] Regenerate DiagnosticsReference.rst

2020-05-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D79236#2015982 , @rjmccall wrote:

> There's no good reason for this file to exist; documenting the warning 
> options is useful, but the file should be generated as an intermediate output 
> of the documentation build and then ultimately fed into Sphinx.
>
> There's even a TODO in the docs about it:
>
> > TODO: Generate this from tblgen. Define one anchor per warning group.


We do generate this document with tblgen already. The problem is that it's not 
automatically regenerated as part of the docs build on the website, unlike some 
of our other similar tblgen-generated documentation. I've asked for that to 
happen repeatedly for years but so far I have achieved zero traction. So the 
best we have right now is to manually regenerate this every once in a while and 
check it in. :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79236



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao marked an inline comment as done.
hliao added inline comments.



Comment at: clang/test/SemaCUDA/variable-target.cu:6
+
+static int gvar;
+// expected-note@-1{{'gvar' declared here}}

tra wrote:
> The current set of tests only verifies access of host variable from device 
> side. We need to check that things work in other direction (i.e. device 
> veriable is not accessible from host). A bit of it is covered in 
> function-overload.cu, but it would make sense to deal with all 
> variable-related things here.
> 
> It would be great to add more test cases:
> * access of device variable from various host functions.
> * test cases to verify that  and sizeof(var) works for device vars in 
> host functions.
> 
> 
yeah, as noted in both the message and some sources, that direction diagnosing 
is more complicated because the host code still be able to access shadow 
variables. We need to issue warnings on improper usage, such as variable direct 
read/write. I want to address that in another patch as more change is required 
to check how a variable is being used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[clang] 9fbf998 - Reject operations between vectors and enum types.

2020-05-04 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2020-05-04T13:11:24-07:00
New Revision: 9fbf9989a2bf0edfbe1b482de470dcccd1108e24

URL: 
https://github.com/llvm/llvm-project/commit/9fbf9989a2bf0edfbe1b482de470dcccd1108e24
DIFF: 
https://github.com/llvm/llvm-project/commit/9fbf9989a2bf0edfbe1b482de470dcccd1108e24.diff

LOG: Reject operations between vectors and enum types.

There are some lookup oddities with these as reported in PR45780, and
GCC doesn't support these behaviors at all.  To be more consistent with
GCC and prevent the crashes caused by our lookup issues, nip the problem
in the bud and prohibit enums here.

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/vector-conditional.cpp
clang/test/SemaCXX/vector.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7e8446c6f1ab..871f3703e6d8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -1483,7 +1483,7 @@ QualType Sema::UsualArithmeticConversions(ExprResult 
, ExprResult ,
 return LHSType;
 
   // ExtInt types aren't subject to conversions between them or normal 
integers,
-  // so this fails. 
+  // so this fails.
   if(LHSType->isExtIntType() || RHSType->isExtIntType())
 return QualType();
 
@@ -9621,7 +9621,8 @@ static bool tryGCCVectorConvertAndSplat(Sema , 
ExprResult *Scalar,
   ScalarCast = CK_IntegralToFloating;
 } else
   return true;
-  }
+  } else if (ScalarTy->isEnumeralType())
+return true;
 
   // Adjust scalar if desired.
   if (Scalar) {

diff  --git a/clang/test/SemaCXX/vector-conditional.cpp 
b/clang/test/SemaCXX/vector-conditional.cpp
index 5676d7a3880d..1b360e4fa832 100644
--- a/clang/test/SemaCXX/vector-conditional.cpp
+++ b/clang/test/SemaCXX/vector-conditional.cpp
@@ -97,7 +97,7 @@ void Operands() {
 
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to 
floats.
-  (void)(four_ints ? four_uints : e);  // should work, non-scoped enum can 
convert to uint.
+  (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert 
between scalar type 'E' and vector type 'FourUInts'}}
   (void)(four_ints ? four_uints : se); // expected-error {{cannot convert 
between vector and non-scalar values ('FourUInts' (vector of 4 'unsigned int' 
values) and 'SE'}}
   // GCC permits this, but our conversion rules reject this for truncation.
   (void)(two_ints ? two_ints : us);// expected-error {{cannot convert 
between scalar type 'unsigned int' and vector type 'TwoInts'}}

diff  --git a/clang/test/SemaCXX/vector.cpp b/clang/test/SemaCXX/vector.cpp
index 0c143babbe3b..724ccece0c42 100644
--- a/clang/test/SemaCXX/vector.cpp
+++ b/clang/test/SemaCXX/vector.cpp
@@ -484,3 +484,15 @@ template  void f() {
   second_type st;
 }
 }
+
+namespace PR45780 {
+enum E { Value = 15 };
+void use(char16 c) {
+  E e;
+  c// expected-error{{cannot convert between scalar type 
'PR45780::E' and vector type 'char16'}}
+  c == Value; // expected-error{{cannot convert between scalar type 
'PR45780::E' and vector type 'char16'}}
+  e | c;  // expected-error{{cannot convert between scalar type 
'PR45780::E' and vector type 'char16'}}
+  e != c; // expected-error{{cannot convert between scalar type 
'PR45780::E' and vector type 'char16'}}
+}
+
+} // namespace PR45780



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao marked an inline comment as done.
hliao added inline comments.



Comment at: clang/test/SemaCUDA/variable-target.cu:42
+  return 0;
+}

yaxunl wrote:
> we need to have a test to check captured local host variable is allowed in 
> device lambda.
> 
> we need to have some test for constexpr variables used in device function.
This patch just addresses the direct address of variables. For capture, it 
would be better to start with another patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D72281: [Matrix] Add matrix type to Clang.

2020-05-04 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 261915.
fhahn added a comment.

Add missing early exit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72281

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGen/debug-info-matrix-types.c
  clang/test/CodeGen/matrix-type.c
  clang/test/CodeGenCXX/matrix-type.cpp
  clang/test/Parser/matrix-type-disabled.c
  clang/test/SemaCXX/matrix-type.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1795,6 +1795,8 @@
 DEFAULT_TYPELOC_IMPL(DependentSizedExtVector, Type)
 DEFAULT_TYPELOC_IMPL(Vector, Type)
 DEFAULT_TYPELOC_IMPL(ExtVector, VectorType)
+DEFAULT_TYPELOC_IMPL(ConstantMatrix, MatrixType)
+DEFAULT_TYPELOC_IMPL(DependentSizedMatrix, MatrixType)
 DEFAULT_TYPELOC_IMPL(FunctionProto, FunctionType)
 DEFAULT_TYPELOC_IMPL(FunctionNoProto, FunctionType)
 DEFAULT_TYPELOC_IMPL(Record, TagType)
Index: clang/test/SemaCXX/matrix-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/matrix-type.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -fsyntax-only -pedantic -fenable-matrix -std=c++11 -verify -triple x86_64-apple-darwin %s
+
+using matrix_double_t = double __attribute__((matrix_type(6, 6)));
+using matrix_float_t = float __attribute__((matrix_type(6, 6)));
+using matrix_int_t = int __attribute__((matrix_type(6, 6)));
+
+void matrix_var_dimensions(int Rows, unsigned Columns, char C) {
+  using matrix1_t = int __attribute__((matrix_type(Rows, 1)));// expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix2_t = int __attribute__((matrix_type(1, Columns))); // expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix3_t = int __attribute__((matrix_type(C, C)));   // expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix4_t = int __attribute__((matrix_type(-1, 1)));  // expected-error{{matrix row size too large}}
+  using matrix5_t = int __attribute__((matrix_type(1, -1)));  // expected-error{{matrix column size too large}}
+  using matrix6_t = int __attribute__((matrix_type(0, 1)));   // expected-error{{zero matrix size}}
+  using matrix7_t = int __attribute__((matrix_type(1, 0)));   // expected-error{{zero matrix size}}
+  using matrix7_t = int __attribute__((matrix_type(char, 0)));// expected-error{{expected '(' for function-style cast or type construction}}
+  using matrix8_t = int __attribute__((matrix_type(1048576, 1))); // expected-error{{matrix row size too large}}
+}
+
+struct S1 {};
+
+enum TestEnum {
+  A,
+  B
+};
+
+void matrix_unsupported_element_type() {
+  using matrix1_t = char *__attribute__((matrix_type(1, 1)));// expected-error{{invalid matrix element type 'char *'}}
+  using matrix2_t = S1 __attribute__((matrix_type(1, 1)));   // expected-error{{invalid matrix element type 'S1'}}
+  using matrix3_t = bool __attribute__((matrix_type(1, 1))); // expected-error{{invalid matrix element type 'bool'}}
+  using matrix4_t = TestEnum __attribute__((matrix_type(1, 1))); // expected-error{{invalid matrix element type 'TestEnum'}}
+}
+
+template  // expected-note{{declared here}}
+void matrix_template_1() {
+  using matrix1_t = float __attribute__((matrix_type(T, T))); // expected-error{{'T' does not refer to a value}}
+}
+
+template  // expected-note{{declared here}}
+void matrix_template_2() {
+  using matrix1_t = float 

[PATCH] D78750: [SveEmitter] Add builtins for svdupq and svdupq_lane

2020-05-04 Thread Sander de Smalen via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
sdesmalen marked an inline comment as done.
Closed by commit rG6b90a6887d25: [SveEmitter] Add builtins for svdupq and 
svdupq_lane (authored by sdesmalen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78750

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -94,7 +94,9 @@
   bool isDefault() const { return DefaultType; }
   bool isFloat() const { return Float; }
   bool isInteger() const { return !Float && !Predicate; }
-  bool isScalarPredicate() const { return !Float && ElementBitwidth == 1; }
+  bool isScalarPredicate() const {
+return !Float && Predicate && NumVectors == 0;
+  }
   bool isPredicateVector() const { return Predicate; }
   bool isPredicatePattern() const { return PredicatePattern; }
   bool isPrefetchOp() const { return PrefetchOp; }
@@ -407,12 +409,12 @@
 
 if (Float)
   S += "float";
-else if (isScalarPredicate())
+else if (isScalarPredicate() || isPredicateVector())
   S += "bool";
 else
   S += "int";
 
-if (!isScalarPredicate())
+if (!isScalarPredicate() && !isPredicateVector())
   S += utostr(ElementBitwidth);
 if (!isScalableVector() && isVector())
   S += "x" + utostr(getNumElements());
@@ -433,7 +435,6 @@
 switch (I) {
 case 'P':
   Predicate = true;
-  ElementBitwidth = 1;
   break;
 case 'U':
   Signed = false;
Index: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c
@@ -0,0 +1,389 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+
+#include 
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+svint8_t test_svdupq_lane_s8(svint8_t data, uint64_t index)
+{
+  // CHECK-LABEL: test_svdupq_lane_s8
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dupq.lane.nxv16i8( %data, i64 %index)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdupq_lane,_s8,,)(data, index);
+}
+
+svint16_t test_svdupq_lane_s16(svint16_t data, uint64_t index)
+{
+  // CHECK-LABEL: test_svdupq_lane_s16
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dupq.lane.nxv8i16( %data, i64 %index)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdupq_lane,_s16,,)(data, index);
+}
+
+svint32_t test_svdupq_lane_s32(svint32_t data, uint64_t index)
+{
+  // CHECK-LABEL: test_svdupq_lane_s32
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dupq.lane.nxv4i32( %data, i64 %index)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdupq_lane,_s32,,)(data, index);
+}
+
+svint64_t test_svdupq_lane_s64(svint64_t data, uint64_t index)
+{
+  // CHECK-LABEL: test_svdupq_lane_s64
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dupq.lane.nxv2i64( %data, i64 %index)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdupq_lane,_s64,,)(data, index);
+}
+
+svuint8_t test_svdupq_lane_u8(svuint8_t data, uint64_t index)
+{
+  // CHECK-LABEL: test_svdupq_lane_u8
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dupq.lane.nxv16i8( %data, i64 %index)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdupq_lane,_u8,,)(data, index);
+}
+
+svuint16_t test_svdupq_lane_u16(svuint16_t data, uint64_t index)
+{
+  // CHECK-LABEL: test_svdupq_lane_u16
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dupq.lane.nxv8i16( %data, i64 %index)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdupq_lane,_u16,,)(data, index);
+}
+
+svuint32_t test_svdupq_lane_u32(svuint32_t data, uint64_t index)
+{
+  // CHECK-LABEL: test_svdupq_lane_u32
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dupq.lane.nxv4i32( %data, i64 %index)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdupq_lane,_u32,,)(data, index);
+}
+
+svuint64_t test_svdupq_lane_u64(svuint64_t data, uint64_t index)
+{
+  // CHECK-LABEL: test_svdupq_lane_u64
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dupq.lane.nxv2i64( %data, 

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-04 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 261820.
erichkeane marked 5 inline comments as done.
Herald added a reviewer: jdoerfert.

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

https://reviews.llvm.org/D79118

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/Sema/ext-int-not-supported.c

Index: clang/test/Sema/ext-int-not-supported.c
===
--- clang/test/Sema/ext-int-not-supported.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
-
-void foo() {
-  _ExtInt(33) a; // expected-error{{_ExtInt is not supported on this target}}
-}
Index: clang/test/CodeGenCXX/ext-int.cpp
===
--- clang/test/CodeGenCXX/ext-int.cpp
+++ clang/test/CodeGenCXX/ext-int.cpp
@@ -98,7 +98,7 @@
 };
 
 void UnderlyingTypeUsage(AsEnumUnderlyingType Param) {
-  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i16 %
+  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i9 signext %
   // WIN: define dso_local void @"?UnderlyingTypeUsage@@YAXW4AsEnumUnderlyingType@@@Z"(i9 %
   AsEnumUnderlyingType Var;
   // CHECK: alloca i9, align 2
Index: clang/test/CodeGen/ext-int-sanitizer.cpp
===
--- clang/test/CodeGen/ext-int-sanitizer.cpp
+++ clang/test/CodeGen/ext-int-sanitizer.cpp
@@ -145,8 +145,7 @@
 
   // Also triggers signed integer overflow.
   E / E;
-  // CHECK: %[[E1LOAD:.+]] = load i11
-  // CHECK: store i11 %[[E1LOAD]], i11* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i11
   // CHECK: %[[E:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[E2:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[NEZERO:.+]] = icmp ne i11 %[[E2]], 0
@@ -163,8 +162,7 @@
 // CHECK: define void @_Z6ShiftsU7_ExtIntILi9EEi
 void Shifts(_ExtInt(9) E) {
   E >> E;
-  // CHECK: %[[E1LOAD:.+]] = load i9, i9*
-  // CHECK: store i9 %[[E1LOAD]], i9* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i9
   // CHECK: %[[LHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[RHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[CMP:.+]] = icmp ule i9 %[[RHSE]], 8
@@ -227,8 +225,7 @@
  unsigned _ExtInt(23) SmallE,
  unsigned _ExtInt(35) BigE) {
   u = SmallE + SmallE;
-  // CHECK: %[[LOADBIGGESTE2:.+]] = load i23
-  // CHECK: store i23 %[[LOADBIGGESTE2]], i23* %[[BIGGESTEADDR:.+]]
+  // CHECK: %[[BIGGESTEADDR:.+]] = alloca i23
   // CHECK: %[[LOADE1:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[LOADE2:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[OFCALL:.+]] = call { i23, i1 } @llvm.uadd.with.overflow.i23(i23 %[[LOADE1]], i23 %[[LOADE2]])
Index: clang/test/CodeGen/ext-int-cc.c
===
--- clang/test/CodeGen/ext-int-cc.c
+++ clang/test/CodeGen/ext-int-cc.c
@@ -2,6 +2,32 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN64
 // RUN: %clang_cc1 -triple i386-gnu-linux -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=LIN32
 // RUN: %clang_cc1 -triple i386-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -triple le32-nacl -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NACL
+// RUN: %clang_cc1 -triple nvptx64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX64
+// RUN: %clang_cc1 -triple nvptx -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX
+// RUN: %clang_cc1 -triple sparcv9 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARCV9
+// RUN: %clang_cc1 -triple sparc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARC
+// RUN: %clang_cc1 -triple mips64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS64
+// RUN: %clang_cc1 -triple mips -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS
+// RUN: %clang_cc1 -triple spir64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPIR64
+// RUN: 

[PATCH] D72281: [Matrix] Add matrix type to Clang.

2020-05-04 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 261911.
fhahn marked 9 inline comments as done.
fhahn added a comment.

Address comments, thanks:

- Use MatrixType as base class for ConstantMatrixType and 
DependentSizedMatrixType.
- Handle both ConstantMatrixType and DependentSizedMatrixType arguments in 
deduction.
- Try to further deduce row and column exprs.
- Return canonical type directly, if it matches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72281

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGen/debug-info-matrix-types.c
  clang/test/CodeGen/matrix-type.c
  clang/test/CodeGenCXX/matrix-type.cpp
  clang/test/Parser/matrix-type-disabled.c
  clang/test/SemaCXX/matrix-type.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1795,6 +1795,8 @@
 DEFAULT_TYPELOC_IMPL(DependentSizedExtVector, Type)
 DEFAULT_TYPELOC_IMPL(Vector, Type)
 DEFAULT_TYPELOC_IMPL(ExtVector, VectorType)
+DEFAULT_TYPELOC_IMPL(ConstantMatrix, MatrixType)
+DEFAULT_TYPELOC_IMPL(DependentSizedMatrix, MatrixType)
 DEFAULT_TYPELOC_IMPL(FunctionProto, FunctionType)
 DEFAULT_TYPELOC_IMPL(FunctionNoProto, FunctionType)
 DEFAULT_TYPELOC_IMPL(Record, TagType)
Index: clang/test/SemaCXX/matrix-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/matrix-type.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -fsyntax-only -pedantic -fenable-matrix -std=c++11 -verify -triple x86_64-apple-darwin %s
+
+using matrix_double_t = double __attribute__((matrix_type(6, 6)));
+using matrix_float_t = float __attribute__((matrix_type(6, 6)));
+using matrix_int_t = int __attribute__((matrix_type(6, 6)));
+
+void matrix_var_dimensions(int Rows, unsigned Columns, char C) {
+  using matrix1_t = int __attribute__((matrix_type(Rows, 1)));// expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix2_t = int __attribute__((matrix_type(1, Columns))); // expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix3_t = int __attribute__((matrix_type(C, C)));   // expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix4_t = int __attribute__((matrix_type(-1, 1)));  // expected-error{{matrix row size too large}}
+  using matrix5_t = int __attribute__((matrix_type(1, -1)));  // expected-error{{matrix column size too large}}
+  using matrix6_t = int __attribute__((matrix_type(0, 1)));   // expected-error{{zero matrix size}}
+  using matrix7_t = int __attribute__((matrix_type(1, 0)));   // expected-error{{zero matrix size}}
+  using matrix7_t = int __attribute__((matrix_type(char, 0)));// expected-error{{expected '(' for function-style cast or type construction}}
+  using matrix8_t = int __attribute__((matrix_type(1048576, 1))); // expected-error{{matrix row size too large}}
+}
+
+struct S1 {};
+
+enum TestEnum {
+  A,
+  B
+};
+
+void matrix_unsupported_element_type() {
+  using matrix1_t = char *__attribute__((matrix_type(1, 1)));// expected-error{{invalid matrix element type 'char *'}}
+  using matrix2_t = S1 __attribute__((matrix_type(1, 1)));   // expected-error{{invalid matrix element type 'S1'}}
+  using matrix3_t = bool __attribute__((matrix_type(1, 1))); // expected-error{{invalid matrix element type 'bool'}}
+  using matrix4_t = TestEnum __attribute__((matrix_type(1, 1))); // expected-error{{invalid matrix element type 'TestEnum'}}
+}
+

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+

erichkeane wrote:
> rjmccall wrote:
> > Does this need to consider the aggregate-as-array logic below?
> I'm not sure what you mean by this?  Are you suggesting I could/should pass 
> this as an array instead of indirectly?
My interpretation is that in general you're lowering large a `_ExtInt` like a 
struct with a bunch of integer members in it.  My understanding is that, for 
this target, that would make it a homogeneous aggregate eligible for the 
special treatment given to certain aggregate types below.  I don't know what 
that corresponds to at the code-emission level.


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

https://reviews.llvm.org/D79118



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

In D79344#2018561 , @tra wrote:

> This has a good chance of breaking existing code. It would be great to add an 
> escape hatch option to revert to the old behavior if we run into problems. 
> The change is relatively simple, so reverting it in case something goes wrong 
> should work, too. Up to you.


Why? for the cases addressed in this patch, if there is existing code, it won't 
be compiled to generate module file due to the missing symbol. Anything missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D72281: [Matrix] Add matrix type to Clang.

2020-05-04 Thread Florian Hahn via Phabricator via cfe-commits
fhahn marked an inline comment as done.
fhahn added inline comments.



Comment at: clang/include/clang/Basic/TypeNodes.td:73
 def ExtVectorType : TypeNode;
+def MatrixType : TypeNode;
 def FunctionType : TypeNode;

rjmccall wrote:
> I think some parts of your life might be easier if there were a common base 
> class here.  You can either call the abstract superclass something like 
> `AbstractMatrixType`, or you can call it `MatrixType` and then make this the 
> concrete subclass `ConstantMatrixType`.
> 
> This is something we've occasionally regretted with vector types.
I've added ConstantMatrixType and DependentSizedMatrixType, which are both 
subclasses of MatrixType.



Comment at: clang/lib/AST/ASTContext.cpp:3840
+ RowExpr, ColumnExpr, AttrLoc);
+  } else {
+QualType CanonicalMatrixElementType = getCanonicalType(MatrixElementType);

rjmccall wrote:
> Is this logic copied from the dependent vector code?  It's actually wrong in 
> a sense — it'll end up creating a non-canonical matrix type wrapping Canon 
> even if all the components are exactly the same.  Probably the only impact is 
> that we waste a little memory, although it's also possible that type-printing 
> in diagnostics will be a little weird.  It'd be better to recognize that the 
> components match Canon exactly and avoid creating a redundant node.
> 
> Please cache the result of calling `getCanonicalType(MatrixElementType)` 
> above.
> Is this logic copied from the dependent vector code? It's actually wrong in a 
> sense — it'll end up creating a non-canonical matrix type wrapping Canon even 
> if all the components are exactly the same. Probably the only impact is that 
> we waste a little memory, although it's also possible that type-printing in 
> diagnostics will be a little weird. It'd be better to recognize that the 
> components match Canon exactly and avoid creating a redundant node.

Yes I think it is similar to what the code for dependent vectors does. I've 
updated it to use the Canonical type, it the components match exactly. If that 
properly addresses your comment I can submit a patch to do the same for the 
vector case.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3349
+void CXXNameMangler::mangleType(const MatrixType *T) {
+  Out << "Dm" << T->getNumRows() << "_" << T->getNumColumns() << '_';
+  mangleType(T->getElementType());

rjmccall wrote:
> fhahn wrote:
> > rjmccall wrote:
> > > fhahn wrote:
> > > > rjmccall wrote:
> > > > > You need to ask the Itanium C++ ABI for this mangling, since it's not 
> > > > > using a vendor-extension prefix.  I know that wasn't done for vector 
> > > > > types, but we're trying to do the right thing.
> > > > That basically means reaching out to 
> > > > https://github.com/itanium-cxx-abi/cxx-abi/, right?
> > > > 
> > > > Do you think it would be feasible to initially go with a 
> > > > vendor-extensions mangling (like 
> > > > `uMatrixxx`)? I've updated 
> > > > the mangling to this.
> > > Yeah, you can open an issue there.
> > > 
> > > That approach doesn't quite work because mangling the element type can 
> > > use or introduce substitutions, but the demangler will naturally skip the 
> > > whole thing.  I think there are two possible approaches here: we could 
> > > either treat the entire matrix type as a vendor-extended qualifier on the 
> > > element type (i.e. `U11matrix_typeILi3ELi4EEi`), or we could extend the 
> > > vendor-extended type mangling to allow template-args (i.e. 
> > > `u11matrix_typeIiLi3ELi4EE`).  The latter is probably better and should 
> > > fit cleanly into the mangling grammar.
> > Thanks for those suggestions. For now I went with the vendor-extended 
> > qualifier, because IIUC that would fit in the existing mangling scheme 
> > without any changes, while the second option would require changes to the 
> > grammar, right?
> > 
> Yes, but it's a very modest change; please raise this with the Itanium 
> committee (which is largely just me again, but wearing a different hat).
> 
> In the meantime, the qualifier approach is fine as a hack, but it's not 
> technically correct unless you do annoying things with the mangling of 
> actually-qualified matrix types.  But the proper qualifier approach is to 
> provide the arguments as template-arguments, not one big unstructured string.
> 
> Also you should do the same thing with DependentSizedMatrixType.
> Yes, but it's a very modest change; please raise this with the Itanium 
> committee (which is largely just me again, but wearing a different hat).

Great, I will do so shortly (probably tomorrow).

> Also you should do the same thing with DependentSizedMatrixType.

I am not sure if it would make sense for DependentSizedMatrixType, as we would 
have to mangle both the row and column expressions and add them to the 
qualifier IIUC. I've disabled mangling for DependentSizedMatrixType 

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

2020-05-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

If we are concerned about affecting other workflows about if we added a new 
command line option to trigger your behavior?

I'd also like to see some lit tests in clang/test/Format to exercise the -lines 
so we really understand its usage

I also wonder if anyone else would notice? if they do I sure wish their usage 
was captured in the unit tests


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

https://reviews.llvm.org/D54881



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


[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations

2020-05-04 Thread Steve Canon via Phabricator via cfe-commits
scanon added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3182
+enabled for the translation unit with the ``-fassociative-math`` flag.
+The pragma can take two values: ``on`` and ``off``.
+

mibintc wrote:
> scanon wrote:
> > rjmccall wrote:
> > > Do you want to add an example here?
> > Is the intention that this allows reassociation only within statements 
> > (like fp contract on)? Based on the ir in the tests, I think the answer is 
> > "no".
> > 
> > If so, should "on" be called "fast" instead, since its semantics match the 
> > "fast" semantics for contract, and reserve "on" for reassociation within a 
> > statement (that seems like it would also be useful, potentially)?
> > 
> > Otherwise, please add some tests with multiple statements.
> > 
> > I agree with John that `pragma clang fp reassociate` is a reasonable 
> > spelling.
> The intention is that the pragma can be placed at either file scope or at the 
> start of a compound statement, if at file scope it affects the functions 
> following the pragma.  If at compound statement it is effective for the 
> compound statement, i can modify the test to have multiple statements.  I 
> disagree with the suggestion of "fast" instead of on/off.  at the command 
> line you can use -f[no-]associative-math; of course that command line option 
> isn't specific to floating point, but the point is it's on/off ; i can't 
> speak to whether "fast" would be a useful setting.  Thanks for your review
I don't think you understood my comment, so I'll try to explain more clearly: I 
am not asking if it applies to multiple statements or a single statement; I am 
asking within what scope reassociation is allowed.

An example: `fp contract on` allows:
```
double r = a*b + c;
```
to be contracted to an fma, but does not allow:
```
double t = a*b;
double r = t + c;
```
to be contracted. `fp contract fast` allows both to be contracted.

Your reassociate pragma, if I am understanding correctly, allows:
```
double t = a + b;
double r = t + c;
```
to be reassociated; this matches the behavior of `fp contract fast`. I am 
asking if, for the sake of understandability and orthogonality of the 
floating-point model, it would make more sense for this option to be named 
`fast`, reserving `on` for a mode that only allows reassociation within 
expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

This has a good chance of breaking existing code. It would be great to add an 
escape hatch option to revert to the old behavior if we run into problems. The 
change is relatively simple, so reverting it in case something goes wrong 
should work, too. Up to you.




Comment at: clang/test/SemaCUDA/variable-target.cu:6
+
+static int gvar;
+// expected-note@-1{{'gvar' declared here}}

The current set of tests only verifies access of host variable from device 
side. We need to check that things work in other direction (i.e. device 
veriable is not accessible from host). A bit of it is covered in 
function-overload.cu, but it would make sense to deal with all variable-related 
things here.

It would be great to add more test cases:
* access of device variable from various host functions.
* test cases to verify that  and sizeof(var) works for device vars in host 
functions.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D79357: [SveEmitter] Add builtins for svdup and svindex

2020-05-04 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen created this revision.
sdesmalen added reviewers: efriedma, SjoerdMeijer, kmclaughlin.
Herald added subscribers: arphaman, tschuett.
sdesmalen added a parent revision: D79356: [CodeGen][SVE] Add patterns for 
whole vector predicate select.

https://reviews.llvm.org/D79357

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup.c

Index: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup.c
@@ -0,0 +1,528 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+
+#include 
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+svint8_t test_svdup_n_s8(int8_t op)
+{
+  // CHECK-LABEL: test_svdup_n_s8
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv16i8(i8 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_s8,)(op);
+}
+
+svint16_t test_svdup_n_s16(int16_t op)
+{
+  // CHECK-LABEL: test_svdup_n_s16
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv8i16(i16 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_s16,)(op);
+}
+
+svint32_t test_svdup_n_s32(int32_t op)
+{
+  // CHECK-LABEL: test_svdup_n_s32
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv4i32(i32 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_s32,)(op);
+}
+
+svint64_t test_svdup_n_s64(int64_t op)
+{
+  // CHECK-LABEL: test_svdup_n_s64
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv2i64(i64 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_s64,)(op);
+}
+
+svuint8_t test_svdup_n_u8(uint8_t op)
+{
+  // CHECK-LABEL: test_svdup_n_u8
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv16i8(i8 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_u8,)(op);
+}
+
+svuint16_t test_svdup_n_u16(uint16_t op)
+{
+  // CHECK-LABEL: test_svdup_n_u16
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv8i16(i16 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_u16,)(op);
+}
+
+svuint32_t test_svdup_n_u32(uint32_t op)
+{
+  // CHECK-LABEL: test_svdup_n_u32
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv4i32(i32 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_u32,)(op);
+}
+
+svuint64_t test_svdup_n_u64(uint64_t op)
+{
+  // CHECK-LABEL: test_svdup_n_u64
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv2i64(i64 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_u64,)(op);
+}
+
+svfloat16_t test_svdup_n_f16(float16_t op)
+{
+  // CHECK-LABEL: test_svdup_n_f16
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv8f16(half %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_f16,)(op);
+}
+
+svfloat32_t test_svdup_n_f32(float32_t op)
+{
+  // CHECK-LABEL: test_svdup_n_f32
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv4f32(float %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_f32,)(op);
+}
+
+svfloat64_t test_svdup_n_f64(float64_t op)
+{
+  // CHECK-LABEL: test_svdup_n_f64
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.x.nxv2f64(double %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_f64,)(op);
+}
+
+svint8_t test_svdup_n_s8_z(svbool_t pg, int8_t op)
+{
+  // CHECK-LABEL: test_svdup_n_s8_z
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.nxv16i8( zeroinitializer,  %pg, i8 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_s8_z,)(pg, op);
+}
+
+svint16_t test_svdup_n_s16_z(svbool_t pg, int16_t op)
+{
+  // CHECK-LABEL: test_svdup_n_s16_z
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv8i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.nxv8i16( zeroinitializer,  %[[PG]], i16 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_s16_z,)(pg, op);
+}
+
+svint32_t test_svdup_n_s32_z(svbool_t pg, int32_t op)
+{
+  // CHECK-LABEL: test_svdup_n_s32_z
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv4i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.dup.nxv4i32( zeroinitializer,  %[[PG]], i32 %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svdup,_n,_s32_z,)(pg, op);
+}
+
+svint64_t 

[PATCH] D70073: [ConstExprPreter] Implemented function calls and if statements

2020-05-04 Thread Nandor Licker via Phabricator via cfe-commits
nand added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70073



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


[PATCH] D79358: [analyzer] CERT: STR37-C

2020-05-04 Thread Zurab Tsinadze via Phabricator via cfe-commits
zukatsinadze created this revision.
zukatsinadze added reviewers: NoQ, Charusso, Szelethus.
zukatsinadze added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, dkrupp, 
donat.nagy, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, 
mgorny.

This patch introduces a new checker:
`alpha.security.cert.str.37c`

This checker is implemented based on the following rule:
https://wiki.sei.cmu.edu/confluence/x/BNcxBQ
The check warns if the argument of a character handling 
function is not representable as unsigned char.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79358

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/cert/Str37Checker.cpp
  clang/test/Analysis/cert/str37-c-fp-suppression.cpp
  clang/test/Analysis/cert/str37-c.cpp

Index: clang/test/Analysis/cert/str37-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str37-c.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.str.37c\
+// RUN:  -verify %s
+
+// Examples from the CERT rule's page.
+// https://wiki.sei.cmu.edu/confluence/x/BNcxBQ
+
+#include "../Inputs/system-header-simulator.h"
+
+int isspace(int c);
+
+size_t bad_usage(const char *s) {
+  const char *t = s;
+  size_t length = strlen(s) + 1;
+  while (isspace(*t) && (t - s < length)) {
+// expected-warning@-1 {{arguments to character-handling function `isspace` must be representable as an unsigned char}}
+++t;
+  }
+  return t - s;
+}
+
+size_t good_usage(const char *s) {
+  const char *t = s;
+  size_t length = strlen(s) + 1;
+  while (isspace((unsigned char)*t) && (t - s < length)) { // no warning
+++t;
+  }
+  return t - s;
+}
Index: clang/test/Analysis/cert/str37-c-fp-suppression.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str37-c-fp-suppression.cpp
@@ -0,0 +1,200 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.str.37c\
+// RUN:  -verify %s
+
+#include "../Inputs/system-header-simulator.h"
+
+int isspace(int c);
+int toupper(int c);
+int isalnum(int c);
+int isalpha(int c);
+int isascii(int c);
+int isblank(int c);
+int iscntrl(int c);
+int isdigit(int c);
+int isgraph(int c);
+int islower(int c);
+int isprint(int c);
+int ispunct(int c);
+int isupper(int c);
+int isxdigit(int c);
+int toascii(int c);
+int tolower(int c);
+
+namespace correct_use {
+
+int test_tolower(const char *c) {
+  char lowA = tolower('A'); // no warning
+  return tolower(97);   // no warning
+}
+
+int test_toascii(const char *c) {
+  return toascii((unsigned char)*c); // no warning
+}
+
+void test_toupper(const char *s) {
+  char c = 98;
+  c = toupper(c); // no warning
+  char b = -2;
+  b = toupper((unsigned char)b); // no warning
+}
+
+int test_isalnum() {
+  int a = 50;
+  int testEOF = isalnum(EOF); // no warning
+  return isalnum(a);  // no warning
+}
+
+int test_isalpha() {
+  char c = 'a';
+  int b = isalpha(255); // no warning
+  return isalpha(c);// no warning
+}
+
+int test_isascii() {
+  char c = 'a';
+  int b = isascii(200); // no warning
+  int A = isascii(65);  // no warning
+  return isascii(c);// no warning
+}
+
+int test_isblank() {
+  char c = ' ';
+  bool isEOFBlank = isblank(EOF); // no warning
+  return isblank(c);  // no warning
+}
+
+int test_iscntrl() {
+  char c = 2;
+  return iscntrl(c); // no warning
+}
+
+int test_isdigit() {
+  char c = '2';
+  return isdigit(c); // no warning
+}
+
+int test_isgraph() {
+  char c = '9';
+  return isgraph(c); // no warning
+}
+
+int test_islower() {
+  char c = 'a';
+  return islower(c); // no warning
+}
+
+int test_isprint(char c) {
+  return isprint((unsigned char)c); // no warning
+}
+
+int test_ispunct() {
+  char c = 'a';
+  char b = -2;
+  bool test_unsigned = ispunct((unsigned char)b); // no warning
+  bool not_true = ispunct(c); // no warning
+  return ispunct(2);  // no warning
+}
+
+int test_isupper(const char *b) {
+  char c = 'A';
+  bool A_is_upper = isupper(c);  // no warning
+  return isupper((unsigned char)*b); // no warning
+}
+
+int test_isxdigit() {
+  char hexa = '9';
+  char not_hexa = 'M';
+  return isxdigit(hexa) || isxdigit(not_hexa); // no warning
+}
+} // namespace correct_use
+
+namespace incorrect_use {
+
+int test_tolower() {
+  int a = 5;
+  int b = 7;
+  char c = a - b;
+  return tolower(c);
+  // expected-warning@-1 {{arguments to character-handling function `tolower` must be representable as an unsigned char}}
+}
+
+int test_toascii(const char *c) {
+  return toascii(*c);
+  // expected-warning@-1 {{arguments to character-handling function `toascii` must be representable as an unsigned char}}
+}
+
+void 

[PATCH] D79334: [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse.

2020-05-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:33
+  CharTypdefsToIgnoreList(Options.get("CharTypdefsToIgnore", "")),
+  CertSTR34C(Options.get("CertSTR34C", false)) {}
 

Same request for the user-facing option name as above.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h:41
   const std::string CharTypdefsToIgnoreList;
+  const bool CertSTR34C;
 };

I'd prefer a more descriptive name than this -- most people reading the code 
won't be familiar with that coding standard. How about 
`DiagnoseSignedUnsignedCharComparisons` or something along those lines?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst:117-119
+  When non-zero, the check will warn only in the cases described by the
+  STR34-C SEI cert rule. This check covers more use cases, which can be
+  limited with this option.

The documentation should describe the actual differences rather than defer 
directly to other documentation. It's fine to make the description light on 
content and say "see  for more information" if the differences are hard 
to spell out concisely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79334



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/lib/Sema/SemaCUDA.cpp:156
+  if (VD->getType()->isCUDADeviceBuiltinSurfaceType() ||
+  VD->getType()->isCUDADeviceBuiltinTextureType())
+return CFT_HostDevice;

We may need to mark constexpr variables as host device too. In practice such 
usage has exist for long time.



Comment at: clang/test/SemaCUDA/variable-target.cu:42
+  return 0;
+}

we need to have a test to check captured local host variable is allowed in 
device lambda.

we need to have some test for constexpr variables used in device function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao updated this revision to Diff 261904.
hliao added a comment.

Reformatting test code following pre-merge checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaInternal.h
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCUDA/function-overload.cu
  clang/test/SemaCUDA/variable-target.cu

Index: clang/test/SemaCUDA/variable-target.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/variable-target.cu
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcuda-is-device -verify %s
+
+#include "Inputs/cuda.h"
+
+static int gvar;
+// expected-note@-1{{'gvar' declared here}}
+// expected-note@-2{{'gvar' declared here}}
+// expected-note@-3{{'gvar' declared here}}
+// expected-note@-4{{'gvar' declared here}}
+// expected-note@-5{{'gvar' declared here}}
+// expected-note@-6{{'gvar' declared here}}
+
+__device__ int d0() {
+  // expected-error@+1{{reference to __host__ variable 'gvar' in __device__ function}}
+  return gvar;
+}
+__device__ int d1() {
+  // expected-error@+1{{reference to __host__ variable 'gvar' in __device__ function}}
+  return []() -> int { return gvar; }();
+}
+
+// expected-warning@+1{{reference to __host__ variable 'gvar' as default argument in __device__ function}}
+__device__ int d2(int arg = gvar) {
+  return arg;
+}
+__device__ int d3() {
+  // expected-error@+1{{reference to __host__ variable 'gvar' in __device__ function}}
+  return d2();
+}
+
+template 
+__global__ void g0(F f) {
+  // expected-error@+1{{reference to __host__ variable 'gvar' in __global__ function}}
+  f();
+}
+int h0() {
+  // expected-warning@+1{{reference to __host__ variable 'gvar' as default argument in __device__ function}}
+  g0<<<1, 1>>>([] __device__(int arg = gvar) -> int { return arg; });
+  // expected-note-re@-1{{in instantiation of function template specialization 'g0<(lambda at {{.*}})>' requested here}}
+  return 0;
+}
Index: clang/test/CodeGenCUDA/function-overload.cu
===
--- clang/test/CodeGenCUDA/function-overload.cu
+++ clang/test/CodeGenCUDA/function-overload.cu
@@ -12,13 +12,15 @@
 #include "Inputs/cuda.h"
 
 // Check constructors/destructors for D/H functions
-int x;
+__device__ int x;
 struct s_cd_dh {
+  // TODO: Need to generate warning on direct accesses on shadow variables.
   __host__ s_cd_dh() { x = 11; }
   __device__ s_cd_dh() { x = 12; }
 };
 
 struct s_cd_hd {
+  // TODO: Need to generate warning on direct accesses on shadow variables.
   __host__ __device__ s_cd_hd() { x = 31; }
   __host__ __device__ ~s_cd_hd() { x = 32; }
 };
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -976,8 +976,6 @@
   startLambdaDefinition(Class, Intro.Range, MethodTyInfo, EndLoc, Params,
 ParamInfo.getDeclSpec().getConstexprSpecifier(),
 ParamInfo.getTrailingRequiresClause());
-  if (ExplicitParams)
-CheckCXXDefaultArguments(Method);
 
   // This represents the function body for the lambda function, check if we
   // have to apply optnone due to a pragma.
@@ -995,6 +993,10 @@
   if (getLangOpts().CUDA)
 CUDASetLambdaAttrs(Method);
 
+  // Check parameters with default arguments.
+  if (ExplicitParams)
+CheckCXXDefaultArguments(Method);
+
   // Number the lambda for linkage purposes if necessary.
   handleLambdaNumbering(Class, Method);
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -345,6 +345,11 @@
 return true;
   }
 
+  if (LangOpts.CUDA && isNonLocalVariable(D) &&
+  !CheckCUDAAccess(Loc, dyn_cast(CurContext),
+   cast(D)))
+return true;
+
   DiagnoseAvailabilityOfDecl(D, Locs, UnknownObjCClass, ObjCPropertyAccess,
  AvoidPartialAvailabilityChecks, ClassReceiver);
 
@@ -5480,6 +5485,13 @@
"default argument expression has capturing blocks?");
   }
 
+  // TODO: Add CUDA check on the default argument and issue warning if any
+  // invalid target reference from the function.
+  if (getLangOpts().CUDA &&
+  checkCUDAInvalidDefaultArgument(
+  CallLoc, dyn_cast(CurContext), Param->getDefaultArg()))
+return true;
+
   // We already type-checked the argument, so we know it works.
   // Just mark all of the declarations in this potentially-evaluated expression
   

[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

2020-05-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D78756



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


[clang] 6b90a68 - [SveEmitter] Add builtins for svdupq and svdupq_lane

2020-05-04 Thread Sander de Smalen via cfe-commits

Author: Sander de Smalen
Date: 2020-05-04T20:38:47+01:00
New Revision: 6b90a6887d25e3375bb916a3ed09f7ccec819d0c

URL: 
https://github.com/llvm/llvm-project/commit/6b90a6887d25e3375bb916a3ed09f7ccec819d0c
DIFF: 
https://github.com/llvm/llvm-project/commit/6b90a6887d25e3375bb916a3ed09f7ccec819d0c.diff

LOG: [SveEmitter] Add builtins for svdupq and svdupq_lane

* svdupq builtins that duplicate scalars to every quadword of a vector
  are defined using builtins for svld1rq (load and replicate quadword).
* svdupq builtins that duplicate boolean values to fill a predicate vector
  are defined using `svcmpne`.

Reviewers: SjoerdMeijer, efriedma, ctetreau

Reviewed By: efriedma

Tags: #clang

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

Added: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c

Modified: 
clang/include/clang/Basic/arm_sve.td
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/utils/TableGen/SveEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index bde26aed43f6..2d2a09d4524d 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -610,6 +610,13 @@ def SVPRFW_GATHER_BASES_OFFSET : 
MInst<"svprfw_gather[_{2}base]_index",  "vPdlJ"
 def SVPRFD_GATHER_BASES_OFFSET : MInst<"svprfd_gather[_{2}base]_index",  
"vPdlJ", "UiUl", [IsGatherPrefetch], MemEltTyInt64, 
"aarch64_sve_prfd_gather_scalar_offset">;
 
 

+// Scalar to vector
+
+def SVDUPQ_8  : SInst<"svdupq[_n]_{d}", "d",  "cUc", 
MergeNone>;
+def SVDUPQ_16 : SInst<"svdupq[_n]_{d}", "d",  "sUsh", MergeNone>;
+def SVDUPQ_32 : SInst<"svdupq[_n]_{d}", "d",  "iUif", MergeNone>;
+def SVDUPQ_64 : SInst<"svdupq[_n]_{d}", "dss",  "lUld", MergeNone>;
+
 // Integer arithmetic
 
 multiclass SInstZPZ flags=[]> {
@@ -1034,7 +1041,7 @@ def SVCLASTB : SInst<"svclastb[_{d}]","dPdd", 
"csilUcUsUiUlhfd", MergeNo
 def SVCLASTB_N   : SInst<"svclastb[_n_{d}]",  "sPsd", "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_clastb_n">;
 def SVCOMPACT: SInst<"svcompact[_{d}]",   "dPd",  "ilUiUlfd",
MergeNone, "aarch64_sve_compact">;
 //  SVDUP_LANE(to land in D78750)
-//  SVDUPQ_LANE   (to land in D78750)
+def SVDUPQ_LANE  : SInst<"svdupq_lane[_{d}]", "ddn",  "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_dupq_lane">;
 def SVEXT: SInst<"svext[_{d}]",   "dddi", "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_ext", [], [ImmCheck<2, ImmCheckExtract, 1>]>;
 def SVLASTA  : SInst<"svlasta[_{d}]", "sPd",  "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_lasta">;
 def SVLASTB  : SInst<"svlastb[_{d}]", "sPd",  "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_lastb">;
@@ -1072,6 +1079,12 @@ def SVPFALSE : SInst<"svpfalse[_b]", "P", "", MergeNone, 
"", [IsOverloadNone]>;
 def SVPTRUE_PAT : SInst<"svptrue_pat_{d}", "PI", "PcPsPiPl", MergeNone, 
"aarch64_sve_ptrue">;
 def SVPTRUE : SInst<"svptrue_{d}", "P",  "PcPsPiPl", MergeNone, 
"aarch64_sve_ptrue", [IsAppendSVALL]>;
 
+def SVDUPQ_B8  : SInst<"svdupq[_n]_{d}",  "P",  "Pc", 
MergeNone>;
+def SVDUPQ_B16 : SInst<"svdupq[_n]_{d}", "P",  "Ps", MergeNone>;
+def SVDUPQ_B32 : SInst<"svdupq[_n]_{d}", "P",  "Pi", MergeNone>;
+def SVDUPQ_B64 : SInst<"svdupq[_n]_{d}", "Pss",  "Pl", MergeNone>;
+
+
 

 // Predicate operations
 

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 94c0adfdf4af..797fcc6deea3 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -7562,6 +7562,15 @@ CodeGenFunction::getSVEPredType(SVETypeFlags TypeFlags) {
 return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 4);
   case SVETypeFlags::EltTyFloat64:
 return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 2);
+
+  case SVETypeFlags::EltTyBool8:
+return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 16);
+  case SVETypeFlags::EltTyBool16:
+return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 8);
+  case SVETypeFlags::EltTyBool32:
+return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 4);
+  case SVETypeFlags::EltTyBool64:
+return llvm::ScalableVectorType::get(Builder.getInt1Ty(), 2);
   }
 }
 
@@ -7599,6 +7608,12 @@ CodeGenFunction::getSVEType(const SVETypeFlags 
) {
   }
 }
 
+llvm::Value *CodeGenFunction::EmitSVEAllTruePred(SVETypeFlags TypeFlags) {
+  Function *Ptrue =
+  CGM.getIntrinsic(Intrinsic::aarch64_sve_ptrue, 
getSVEPredType(TypeFlags));
+  return Builder.CreateCall(Ptrue, {Builder.getInt32(/*SV_ALL*/ 31)});
+}
+
 constexpr unsigned SVEBitsPerBlock = 128;
 
 static llvm::ScalableVectorType *getSVEVectorForElementType(llvm::Type *EltTy) 
{

[PATCH] D79354: [clang-format] [PR34574] Handle [[nodiscard]] attribute in class declaration

2020-05-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, sammccall, marejde, mitchell-stellar.
MyDeveloperDay added projects: clang, clang-format.

https://bugs.llvm.org/show_bug.cgi?id=34574
https://bugs.llvm.org/show_bug.cgi?id=38401

  template 
  class [[nodiscard]] result
  {
public:
  result(T&&)
  {
  }
  };

formats incorrectly to

  template 
  class [[nodiscard]] result{public : result(T &&){}};


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79354

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7636,6 +7636,10 @@
   verifyFormat("void f() [[deprecated(\"so sorry\")]];");
   verifyFormat("aa\n"
"[[unused]] aaa(int i);");
+  verifyFormat("[[nodiscard]] bool f() { return false; }");
+  verifyFormat("class [[nodiscard]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[deprecated(\"so sorry\")]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[gnu::unused]] f {\npublic:\n  f() {}\n}");
 
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2401,10 +2401,14 @@
   // it is often token-pasted.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas) ||
+tok::kw_alignas, tok::l_square) ||
  ((Style.Language == FormatStyle::LK_Java ||
Style.Language == FormatStyle::LK_JavaScript) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
+
+if (FormatTok->is(tok::l_square)) {
+  parseSquare();
+}
 if (Style.Language == FormatStyle::LK_JavaScript &&
 FormatTok->isOneOf(Keywords.kw_extends, Keywords.kw_implements)) {
   // JavaScript/TypeScript supports inline object types in


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7636,6 +7636,10 @@
   verifyFormat("void f() [[deprecated(\"so sorry\")]];");
   verifyFormat("aa\n"
"[[unused]] aaa(int i);");
+  verifyFormat("[[nodiscard]] bool f() { return false; }");
+  verifyFormat("class [[nodiscard]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[deprecated(\"so sorry\")]] f {\npublic:\n  f() {}\n}");
+  verifyFormat("class [[gnu::unused]] f {\npublic:\n  f() {}\n}");
 
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2401,10 +2401,14 @@
   // it is often token-pasted.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas) ||
+tok::kw_alignas, tok::l_square) ||
  ((Style.Language == FormatStyle::LK_Java ||
Style.Language == FormatStyle::LK_JavaScript) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
+
+if (FormatTok->is(tok::l_square)) {
+  parseSquare();
+}
 if (Style.Language == FormatStyle::LK_JavaScript &&
 FormatTok->isOneOf(Keywords.kw_extends, Keywords.kw_implements)) {
   // JavaScript/TypeScript supports inline object types in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

2020-05-04 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen updated this revision to Diff 261892.
sdesmalen added a comment.

- Added FIXME in CGBuiltins for big-endian svreinterpret.
- Added diagnostic in arm_sve.h that big-endian is not yet supported.


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

https://reviews.llvm.org/D78756

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_reinterpret.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/big_endian.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -237,6 +237,23 @@
 
 class SVEEmitter {
 private:
+  // The reinterpret builtins are generated separately because they
+  // need the cross product of all types (121 functions in total),
+  // which is inconvenient to specify in the arm_sve.td file or
+  // generate in CGBuiltin.cpp.
+  struct ReinterpretTypeInfo {
+const char *Suffix;
+const char *Type;
+const char *BuiltinType;
+  };
+  SmallVector Reinterprets = {
+  {"s8", "svint8_t", "q16Sc"},   {"s16", "svint16_t", "q8Ss"},
+  {"s32", "svint32_t", "q4Si"},  {"s64", "svint64_t", "q2SWi"},
+  {"u8", "svuint8_t", "q16Uc"},  {"u16", "svuint16_t", "q8Us"},
+  {"u32", "svuint32_t", "q4Ui"}, {"u64", "svuint64_t", "q2UWi"},
+  {"f16", "svfloat16_t", "q8h"}, {"f32", "svfloat32_t", "q4f"},
+  {"f64", "svfloat64_t", "q2d"}};
+
   RecordKeeper 
   llvm::StringMap EltTypes;
   llvm::StringMap MemEltTypes;
@@ -1008,6 +1025,10 @@
   OS << "#error \"SVE support not enabled\"\n";
   OS << "#else\n\n";
 
+  OS << "#if !defined(__LITTLE_ENDIAN__)\n";
+  OS << "#error \"Big endian is currently not supported for arm_sve.h\"\n";
+  OS << "#endif\n";
+
   OS << "#include \n\n";
   OS << "#ifdef  __cplusplus\n";
   OS << "extern \"C\" {\n";
@@ -1074,6 +1095,22 @@
   OS << "#define __aio static inline __attribute__((__always_inline__, "
 "__nodebug__, __overloadable__))\n\n";
 
+  // Add reinterpret functions.
+  for (auto ShortForm : { false, true } )
+for (const ReinterpretTypeInfo  : Reinterprets)
+  for (const ReinterpretTypeInfo  : Reinterprets) {
+if (ShortForm) {
+  OS << "__aio " << From.Type << " svreinterpret_" << From.Suffix;
+  OS << "(" << To.Type << " op) {\n";
+  OS << "  return __builtin_sve_reinterpret_" << From.Suffix << "_"
+ << To.Suffix << "(op);\n";
+  OS << "}\n\n";
+} else
+  OS << "#define svreinterpret_" << From.Suffix << "_" << To.Suffix
+ << "(...) __builtin_sve_reinterpret_" << From.Suffix << "_"
+ << To.Suffix << "(__VA_ARGS__)\n";
+  }
+
   SmallVector, 128> Defs;
   std::vector RV = Records.getAllDerivedDefinitions("Inst");
   for (auto *R : RV)
@@ -1148,8 +1185,16 @@
   OS << "BUILTIN(__builtin_sve_" << Def->getMangledName() << ", \""
  << Def->getBuiltinTypeStr() << "\", \"n\")\n";
   }
+
+  // Add reinterpret builtins
+  for (const ReinterpretTypeInfo  : Reinterprets)
+for (const ReinterpretTypeInfo  : Reinterprets)
+  OS << "BUILTIN(__builtin_sve_reinterpret_" << From.Suffix << "_"
+ << To.Suffix << +", \"" << From.BuiltinType << To.BuiltinType
+ << "\", \"n\")\n";
+
   OS << "#endif\n\n";
-}
+  }
 
 void SVEEmitter::createCodeGenMap(raw_ostream ) {
   std::vector RV = Records.getAllDerivedDefinitions("Inst");
Index: clang/test/CodeGen/aarch64-sve-intrinsics/negative/big_endian.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/negative/big_endian.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64_be-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+// expected-error@* {{Big endian is currently not supported for arm_sve.h}}
+#include 
Index: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_reinterpret.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_reinterpret.c
@@ -0,0 +1,960 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+
+#include 
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+svint8_t test_svreinterpret_s8_s8(svint8_t op)
+{
+  // CHECK-LABEL: test_svreinterpret_s8_s8
+  // CHECK: ret  %op
+  

[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations

2020-05-04 Thread Melanie Blower via Phabricator via cfe-commits
mibintc marked an inline comment as done.
mibintc added a comment.

A reply to @scanon




Comment at: clang/docs/LanguageExtensions.rst:3182
+enabled for the translation unit with the ``-fassociative-math`` flag.
+The pragma can take two values: ``on`` and ``off``.
+

scanon wrote:
> rjmccall wrote:
> > Do you want to add an example here?
> Is the intention that this allows reassociation only within statements (like 
> fp contract on)? Based on the ir in the tests, I think the answer is "no".
> 
> If so, should "on" be called "fast" instead, since its semantics match the 
> "fast" semantics for contract, and reserve "on" for reassociation within a 
> statement (that seems like it would also be useful, potentially)?
> 
> Otherwise, please add some tests with multiple statements.
> 
> I agree with John that `pragma clang fp reassociate` is a reasonable spelling.
The intention is that the pragma can be placed at either file scope or at the 
start of a compound statement, if at file scope it affects the functions 
following the pragma.  If at compound statement it is effective for the 
compound statement, i can modify the test to have multiple statements.  I 
disagree with the suggestion of "fast" instead of on/off.  at the command line 
you can use -f[no-]associative-math; of course that command line option isn't 
specific to floating point, but the point is it's on/off ; i can't speak to 
whether "fast" would be a useful setting.  Thanks for your review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827



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


[PATCH] D79113: Revert "Remove false positive in AvoidNonConstGlobalVariables."

2020-05-04 Thread Kim Viggedal via Phabricator via cfe-commits
vingeldal added a comment.

In D79113#2016294 , @aaron.ballman 
wrote:

> I only see a comment on the issue, but not that the issue was resolved in 
> favor of that comment (unless I've missed something). If the issue gets 
> resolved in the direction that comment is going, then the revert makes sense 
> (though we should add an explicit test case showing that we want the 
> diagnostic).


No, your not missing anything. The issue isn't actually closed yet. I'll 
remember to update tests as appropriate if the issue is resolved in favor of 
this revert. Until the issue is resolved I'll just wait.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79113



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


[clang] 54fa46a - [SveEmitter] Add builtins for Int & FP reductions

2020-05-04 Thread Sander de Smalen via cfe-commits

Author: Sander de Smalen
Date: 2020-05-04T19:50:16+01:00
New Revision: 54fa46aa0a82bd281d0ba31fad69a227de4a622c

URL: 
https://github.com/llvm/llvm-project/commit/54fa46aa0a82bd281d0ba31fad69a227de4a622c
DIFF: 
https://github.com/llvm/llvm-project/commit/54fa46aa0a82bd281d0ba31fad69a227de4a622c.diff

LOG: [SveEmitter] Add builtins for Int & FP reductions

This patch adds integer builtins for:
- svaddv, svandv, sveorv,
  svmaxv, svminv, svorv.

And FP builtins for:
- svadda, svaddv, svmaxv, svmaxnmv,
  svminv, svminnmv

Added: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_addv.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_andv.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_eorv.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_maxnmv.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_maxv.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_minnmv.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_minv.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_orv.c

Modified: 
clang/include/clang/Basic/arm_sve.td

Removed: 




diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index 013357c3de9b..bde26aed43f6 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -705,6 +705,19 @@ defm SVLSR : SInst_SHIFT<"svlsr", "aarch64_sve_lsr", 
"UcUsUiUl", "UcUsUi">;
 
 def SVASRD_M : SInst<"svasrd[_n_{d}]", "dPdi", "csil",MergeOp1,  
"aarch64_sve_asrd", [], [ImmCheck<2, ImmCheckShiftRight, 1>]>;
 
+
+// Integer reductions
+
+def SVADDV_S : SInst<"svaddv[_{d}]", "lPd", "csil", MergeNone, 
"aarch64_sve_saddv">;
+def SVADDV_U : SInst<"svaddv[_{d}]", "nPd", "UcUsUiUl", MergeNone, 
"aarch64_sve_uaddv">;
+def SVANDV   : SInst<"svandv[_{d}]", "sPd", "csilUcUsUiUl", MergeNone, 
"aarch64_sve_andv">;
+def SVEORV   : SInst<"sveorv[_{d}]", "sPd", "csilUcUsUiUl", MergeNone, 
"aarch64_sve_eorv">;
+def SVMAXV_S : SInst<"svmaxv[_{d}]", "sPd", "csil", MergeNone, 
"aarch64_sve_smaxv">;
+def SVMAXV_U : SInst<"svmaxv[_{d}]", "sPd", "UcUsUiUl", MergeNone, 
"aarch64_sve_umaxv">;
+def SVMINV_S : SInst<"svminv[_{d}]", "sPd", "csil", MergeNone, 
"aarch64_sve_sminv">;
+def SVMINV_U : SInst<"svminv[_{d}]", "sPd", "UcUsUiUl", MergeNone, 
"aarch64_sve_uminv">;
+def SVORV: SInst<"svorv[_{d}]",  "sPd", "csilUcUsUiUl", MergeNone, 
"aarch64_sve_orv">;
+
 

 // Integer comparisons
 
@@ -876,6 +889,15 @@ def SVRECPS  : SInst<"svrecps[_{d}]",  "ddd", "hfd", 
MergeNone, "aarch64_sve_fre
 def SVRSQRTE : SInst<"svrsqrte[_{d}]", "dd",  "hfd", MergeNone, 
"aarch64_sve_frsqrte_x">;
 def SVRSQRTS : SInst<"svrsqrts[_{d}]", "ddd", "hfd", MergeNone, 
"aarch64_sve_frsqrts_x">;
 
+
+// Floating-point reductions
+
+def SVFADDA   : SInst<"svadda[_{d}]",   "sPsd", "hfd", MergeNone, 
"aarch64_sve_fadda">;
+def SVFADDV   : SInst<"svaddv[_{d}]",   "sPd",  "hfd", MergeNone, 
"aarch64_sve_faddv">;
+def SVFMAXV   : SInst<"svmaxv[_{d}]",   "sPd",  "hfd", MergeNone, 
"aarch64_sve_fmaxv">;
+def SVFMAXNMV : SInst<"svmaxnmv[_{d}]", "sPd",  "hfd", MergeNone, 
"aarch64_sve_fmaxnmv">;
+def SVFMINV   : SInst<"svminv[_{d}]",   "sPd",  "hfd", MergeNone, 
"aarch64_sve_fminv">;
+def SVFMINNMV : SInst<"svminnmv[_{d}]", "sPd",  "hfd", MergeNone, 
"aarch64_sve_fminnmv">;
 
 

 // Floating-point comparisons

diff  --git a/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c 
b/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c
new file mode 100644
index ..6ac6e5d0d618
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu 
-target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple 
aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns 
-S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+
+#include 
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+float16_t test_svadda_f16(svbool_t pg, float16_t initial, svfloat16_t op)
+{
+  // CHECK-LABEL: test_svadda_f16
+  // CHECK: %[[PG:.*]] = call  
@llvm.aarch64.sve.convert.from.svbool.nxv8i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call 

[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint

2020-05-04 Thread Lei Huang via Phabricator via cfe-commits
lei accepted this revision as: lei.
lei 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/D77542/new/

https://reviews.llvm.org/D77542



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


[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations

2020-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Just minor requests now.




Comment at: clang/docs/LanguageExtensions.rst:3177
+Both floating point reassociation and floating point contraction can be
+controlled with this pragma.
+``#pragma clang fp reassoc`` allows control over the reassociation

rjmccall wrote:
> Let's go ahead and word this as if arbitrary things will be controllable in 
> the future.  So:
> 
> > Currently, the following things can be controlled by this pragma:
Thanks.  Please put the first sentence in its own paragraph and end it with a 
colon so that it logically leads into both of the following blocks.



Comment at: clang/docs/LanguageExtensions.rst:3182
+enabled for the translation unit with the ``-fassociative-math`` flag.
+The pragma can take two values: ``on`` and ``off``.
+

Do you want to add an example here?



Comment at: clang/docs/LanguageExtensions.rst:3192
 in cases when the language standard does not make this possible (e.g. across
 statements in C)
 

Oh, and there's a missing period here.



Comment at: clang/include/clang/Basic/LangOptions.h:186
+FPM_Fast
   };
 

mibintc wrote:
> rjmccall wrote:
> > I'm not sure I think this fusion was an improvement; the net effect was to 
> > remove a few lines from this header and make a bunch of switches 
> > unnecessarily non-exhaustive.
> I dropped the on/off enumeration and just using boolean, where needed, to 
> show the setting, do you like this better? 
Yeah, thanks.



Comment at: clang/include/clang/Sema/Sema.h:9624
+  /// \#pragma clang fp reassociate
+  void ActOnPragmaFPAllowReassociation(bool IsEnabled);
 

Please name this the same as the language feature.



Comment at: clang/lib/Parse/ParsePragma.cpp:2900
   return;
 }
 PP.Lex(Tok);

Please make this a single block by making the condition something like `if 
(!FlagValue || (FlagKind == TokFPAnnotValue::Reassociate && FlagValue == 
TokFPAnnotValue::Fast))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

That test code just passed compilation on clang trunk if only assembly code is 
generated, https://godbolt.org/z/XYjRcT. But NVCC generates errors on all cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79344



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


[PATCH] D79344: [cuda] Start diagnosing variables with bad target.

2020-05-04 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added reviewers: tra, rjmccall, yaxunl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
hliao added a comment.

That test code just passed compilation on clang trunk if only assembly code is 
generated, https://godbolt.org/z/XYjRcT. But NVCC generates errors on all cases.


- Non-local variables on the host side are generally not accessible from the 
device side. Without proper diagnostic messages, the compilation may pass until 
the final linking stage. That link error may not be intuitive enough for 
developers, especially for relocatable code compilation. For certain cases like 
assembly output only, it is even worse that the compilation just passes.
- This patch addresses that issue by checking the use of non-local variables 
and issuing errors on bad target references. For references through default 
argumennts, a warning is generated on the function declaration as, at that 
point, that variables are just bound. No real code would be generated if that 
function won't be used.
- The oppose direction, i.e. accessing device variables from the host side, is 
NOT addressed in this patch as the host code allows the access those device 
variables by using runtime interface on their shadow variables. It needs more 
support to identify how that variable is used on the host side for simple 
cases. The comprehensive diagnosing would be so expensive that alternative 
analysis tools like clang-tidy should be used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79344

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaInternal.h
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCUDA/function-overload.cu
  clang/test/SemaCUDA/variable-target.cu

Index: clang/test/SemaCUDA/variable-target.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/variable-target.cu
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcuda-is-device -verify %s
+
+#include "Inputs/cuda.h"
+
+static int gvar;
+// expected-note@-1{{'gvar' declared here}}
+// expected-note@-2{{'gvar' declared here}}
+// expected-note@-3{{'gvar' declared here}}
+// expected-note@-4{{'gvar' declared here}}
+// expected-note@-5{{'gvar' declared here}}
+// expected-note@-6{{'gvar' declared here}}
+
+__device__ int d0() {
+  // expected-error@+1{{reference to __host__ variable 'gvar' in __device__ function}}
+  return gvar;
+}
+__device__ int d1() {
+  // expected-error@+1{{reference to __host__ variable 'gvar' in __device__ function}}
+  return []() -> int { return gvar; }();
+}
+
+// expected-warning@+1{{reference to __host__ variable 'gvar' as default argument in __device__ function}}
+__device__ int d2(int arg = gvar) {
+  return arg;
+}
+__device__ int d3() {
+  // expected-error@+1{{reference to __host__ variable 'gvar' in __device__ function}}
+  return d2();
+}
+
+template
+__global__ void g0(F f) {
+  // expected-error@+1{{reference to __host__ variable 'gvar' in __global__ function}}
+  f();
+}
+int h0() {
+  // expected-warning@+1{{reference to __host__ variable 'gvar' as default argument in __device__ function}}
+  g0<<<1, 1>>>([] __device__(int arg = gvar) -> int { return arg; });
+  // expected-note-re@-1{{in instantiation of function template specialization 'g0<(lambda at {{.*}})>' requested here}}
+  return 0;
+}
Index: clang/test/CodeGenCUDA/function-overload.cu
===
--- clang/test/CodeGenCUDA/function-overload.cu
+++ clang/test/CodeGenCUDA/function-overload.cu
@@ -12,13 +12,15 @@
 #include "Inputs/cuda.h"
 
 // Check constructors/destructors for D/H functions
-int x;
+__device__ int x;
 struct s_cd_dh {
+// TODO: Need to generate warning on direct accesses on shadow variables.
   __host__ s_cd_dh() { x = 11; }
   __device__ s_cd_dh() { x = 12; }
 };
 
 struct s_cd_hd {
+// TODO: Need to generate warning on direct accesses on shadow variables.
   __host__ __device__ s_cd_hd() { x = 31; }
   __host__ __device__ ~s_cd_hd() { x = 32; }
 };
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -976,8 +976,6 @@
   startLambdaDefinition(Class, Intro.Range, MethodTyInfo, EndLoc, Params,
 ParamInfo.getDeclSpec().getConstexprSpecifier(),
 ParamInfo.getTrailingRequiresClause());
-  if (ExplicitParams)
-CheckCXXDefaultArguments(Method);
 
   // This represents the function body for the lambda function, check if we
   // have to apply optnone due to a pragma.
@@ -995,6 +993,10 @@
   if (getLangOpts().CUDA)
  

[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations

2020-05-04 Thread Steve Canon via Phabricator via cfe-commits
scanon added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3182
+enabled for the translation unit with the ``-fassociative-math`` flag.
+The pragma can take two values: ``on`` and ``off``.
+

rjmccall wrote:
> Do you want to add an example here?
Is the intention that this allows reassociation only within statements (like fp 
contract on)? Based on the ir in the tests, I think the answer is "no".

If so, should "on" be called "fast" instead, since its semantics match the 
"fast" semantics for contract, and reserve "on" for reassociation within a 
statement (that seems like it would also be useful, potentially)?

Otherwise, please add some tests with multiple statements.

I agree with John that `pragma clang fp reassociate` is a reasonable spelling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827



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


[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

2020-05-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7880
+return Builder.CreateBitCast(Val, Ty);
+  }
+

efriedma wrote:
> sdesmalen wrote:
> > efriedma wrote:
> > > sdesmalen wrote:
> > > > efriedma wrote:
> > > > > I'm vaguely suspicious this might be wrong for big-endian targets.  I 
> > > > > mean, this isn't unreasonable, but users might be surprised if 
> > > > > svreinterpret isn't a no-op.
> > > > For SVE the loads and stores (svld1 and svst1) are all endian safe, so 
> > > > no special consideration needs to be taken for big endian targets.
> > > > 
> > > > The ACLE specifies that:
> > > > > The svreinterpret functions simply reinterpret a vector of one type 
> > > > > as a vector of another type, without changing any of the bits.
> > > "bitcast" is specified to mean "reinterpret the bits like a store+load".  
> > > On big-endian NEON (and, I assume, SVE), that isn't a no-op.  See 
> > > http://llvm.org/docs/BigEndianNEON.html .
> > > 
> > > I mean, if the answer here is "yes, svreinterpret is supposed to lower to 
> > > a REV", then that's fine.  But I'd like to see some explciit 
> > > acknowledgement that that's intentional.
> > Thanks for pointing out that page, but for SVE I don't think the 
> > svreinterpret should lower to a REV.
> > 
> > This is probably where things are different from Neon. The ACLE SVE vectors 
> > such as `svint32_t` are opaque vector types and the only way to load/store 
> > them from/to memory is through the use of the svld1 and svst1 intrinsics 
> > which are endian safe (in that they use the ld1/st1 instructions that do 
> > endianess conversion on big endian targets). The ACLE does not expose any 
> > full-vector load/store (ldr/str) operations.
> Like that page describes, we use ld1/st1 for big-endian NEON, to match the 
> LLVM IR rules for laying out a vector.  If you use ld1/st1 to load/store 
> vectors on big-endian NEON, a bitcast is not a no-op.  As far as I know, SVE 
> ld1/st1 is equivalent to NEON ld1/st1 in the case where vscale=1.  Therefore, 
> on big-endian SVE, a bitcast is not a no-op.
> 
> That leaves the following options:
> 
> 1. svreinterpret is not a no-op.
> 2. svreinterpret is not equivalent to an LLVM IR bitcast, so this patch needs 
> to be changed.
(If you don't care about big-endian SVE right now, that's fine, but please at 
least leave a FIXME.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78756



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


[PATCH] D78750: [SveEmitter] Add builtins for svdupq and svdupq_lane

2020-05-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D78750



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


  1   2   >