[PATCH] D36364: [AArch64] Add support for a MinGW AArch64 target

2017-08-05 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
compnerd added inline comments.



Comment at: lib/Driver/ToolChains/MinGW.cpp:122-131
   if (TC.getArch() == llvm::Triple::x86)
 CmdArgs.push_back("i386pe");
-  if (TC.getArch() == llvm::Triple::x86_64)
+  else if (TC.getArch() == llvm::Triple::x86_64)
 CmdArgs.push_back("i386pep");
-  if (TC.getArch() == llvm::Triple::arm)
+  else if (TC.getArch() == llvm::Triple::arm)
 CmdArgs.push_back("thumb2pe");
+  else if (TC.getArch() == llvm::Triple::aarch64)

Can you use a switch here instead?


https://reviews.llvm.org/D36364



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

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

2017-08-05 Thread Hal Finkel via Phabricator via cfe-commits 
hfinkel added inline comments.



Comment at: lib/Driver/ToolChain.cpp:808
+  continue;
+  } else if (XOpenMPTargetNoTriple)
+// Passing device args: -Xopenmp-target -opt=val.

Please include {} around this else-if  code, even though it is not necessary, 
because the other blocks require it.



Comment at: lib/Driver/ToolChain.cpp:820
+  if (!XOpenMPTargetArg || Index > Prev + 1) {
+getDriver().Diag(diag::err_drv_invalid_Xopenmp_target_with_args)
+<< A->getAsString(Args);

Is this covered by a test case?



Comment at: lib/Driver/ToolChain.cpp:827
+  options::OPT_fopenmp_targets_EQ).size() != 1) {
+getDriver().Diag(diag::err_drv_Xopenmp_target_missing_triple);
+continue;

Is this covered by a test case?



Comment at: lib/Driver/ToolChain.cpp:834
+  // Ignore all but last -march=arch flag.
+  if (A->getOption().matches(options::OPT_march_EQ))
+DAL->eraseArg(options::OPT_march_EQ);

Why is `-march` special in this regard? Shouldn't the consumers just take the 
last one specified (e.g., use getLastArgValue in the ToolChain code)?



Comment at: test/Driver/openmp-offload.c:615
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: 
'-Xopenmp-target -march=pwr8'

Now that this is in common code, why are these arguments still unused?


https://reviews.llvm.org/D34784



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

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

2017-08-05 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits 
gtbercea updated this revision to Diff 109904.
gtbercea added a comment.

  Don't exclude flags when host matches offload toolchain.


https://reviews.llvm.org/D34784

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Driver/Compilation.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -597,3 +597,19 @@
 // RUN:   | FileCheck -check-prefix=CHK-FOPENMP-IS-DEVICE %s
 
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}} "-aux-triple" "powerpc64le--linux" {{.*}}.c" "-fopenmp-is-device" "-fopenmp-host-ir-file-path"
+
+/// ###
+
+/// Check -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-EQ-TARGET %s
+
+// CHK-FOPENMP-EQ-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8'
+
+/// ###
+
+/// Check -Xopenmp-target -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET %s
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target -march=pwr8'
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -212,8 +212,20 @@
   static_cast(getToolChain());
   assert(TC.getTriple().isNVPTX() && "Wrong platform");
 
+  StringRef GPUArchName;
+  std::vector GPUArchNames;
+  // If this is an OpenMP action we need to extract the device architecture from
+  // the -fopenmp-target-arch option.
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP)) {
+GPUArchNames = Args.getAllArgValues(options::OPT_march_EQ);
+assert(GPUArchNames.size() == 1 &&
+   "Exactly one GPU Arch required for ptxas.");
+GPUArchName = GPUArchNames[0];
+  } else
+GPUArchName = JA.getOffloadingArch();
+
   // Obtain architecture from the action.
-  CudaArch gpu_arch = StringToCudaArch(JA.getOffloadingArch());
+  CudaArch gpu_arch = StringToCudaArch(GPUArchName);
   assert(gpu_arch != CudaArch::UNKNOWN &&
  "Device action expected to have an architecture.");
 
@@ -405,7 +417,7 @@
 
   // For OpenMP device offloading, append derived arguments. Make sure
   // flags are not duplicated.
-  // TODO: Append the compute capability.
+  // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
 for (Arg *A : Args){
   bool IsDuplicate = false;
@@ -418,6 +430,15 @@
   if (!IsDuplicate)
 DAL->append(A);
 }
+
+auto MArchList = DAL->getAllArgValues(options::OPT_march_EQ);
+assert(MArchList.size() < 2 &&
+   "Too many archs under -Xopenmp-targets");
+if (MArchList.empty())
+  // Default compute capability for CUDA toolchain is sm_20.
+  DAL->AddJoinedArg(nullptr,
+  Opts.getOption(options::OPT_march_EQ), "sm_20");
+
 return DAL;
   }
 
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -775,3 +775,69 @@
 
   return VersionTuple();
 }
+
+llvm::opt::DerivedArgList *
+ToolChain::TranslateOpenMPTargetArgs(const llvm::opt::DerivedArgList ,
+Action::OffloadKind DeviceOffloadKind) const {
+  if (DeviceOffloadKind == Action::OFK_OpenMP) {
+DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
+const OptTable  = getDriver().getOpts();
+
+// Handle -Xopenmp-target flags
+for (Arg *A : Args) {
+  // Exclude flags which may only apply to the host toolchain.
+  // Do not exclude flags when the host triple (AuxTriple),
+  // matches the current toolchain triple.
+  if (A->getOption().matches(options::OPT_m_Group)) {
+if (getAuxTriple() && getAuxTriple()->str() == getTriple().str())
+  DAL->append(A);
+continue;
+  }
+
+  unsigned Index;
+  unsigned Prev;
+  bool XOpenMPTargetNoTriple = A->getOption().matches(
+  options::OPT_Xopenmp_target);
+
+  if (A->getOption().matches(options::OPT_Xopenmp_target_EQ)) {
+// Passing device args: -Xopenmp-target= -opt=val.
+if (A->getValue(0) == getTripleString())
+  Index = Args.getBaseArgs().MakeIndex(A->getValue(1));
+else
+  continue;
+  }

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

2017-08-05 Thread John McCall via Phabricator via cfe-commits 
rjmccall added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:138
+Additionally, for block pointers, the same restriction apply to copies of
+blocks. For example:
+

```
  Additionally, when the parameter is a `block pointer
  `, the same restriction 
applies
  to copies of the block.  For example:
```



Comment at: include/clang/Sema/Sema.h:1480
+  /// void (*to2)(__attribute__((noescape)) int *) = 
+  ///
+  const FunctionProtoType *

I assume this only returns noescape from parameters that are not noescape in 
ToType.



Comment at: lib/AST/ItaniumMangle.cpp:2700
+  if (FD->getParamDecl(I)->hasAttr())
+Out << "U8noescape";
 }

This is not the right place to do this:

* It needs to be mangled in function types, not just on function declarations.  
It looks like the code here supporting pass_object_size just gets this wrong.
* We don't want to mangle this in top-level declarations because doing so will 
make it more directly ABI-breaking to retroactively add this attribute to a 
declaration.

Instead, you should handle it in mangleExtParameterInfo.  Because these 
qualifiers are mangled in reverse alphabetical order, and "noescape" precedes 
"ns_consumed", it should be the last thing tested for in that function.



Comment at: lib/Sema/SemaDeclCXX.cpp:14024
+
+  if (OldFT->hasExtParameterInfos())
+for (unsigned I = 0, E = OldFT->getNumParams(); I != E; ++I)

I think we generally encourage the uses of braces when the nested statement is 
this complex, even if it is technically a single statement.



Comment at: lib/Sema/SemaDeclObjC.cpp:186
+oldDecl->hasAttr()) {
+  Diag(newDecl->getLocation(), diag::err_nsconsumed_attribute_mismatch);
+  Diag(oldDecl->getLocation(), diag::note_previous_decl) << "parameter";

I know this is existing code, but you should probably at least warn about 
ns_consumed mismatches here in non-ARC.



Comment at: lib/Sema/SemaExpr.cpp:7180
+  if (!FromType->hasExtParameterInfos())
+return FromType;
+

So this function removes noescape attributes from FromType to match ToType, but 
only if it's a legal conversion?  Maybe it would be more reasonable to report 
an inability to convert in a more direct way, like by returning a null type.

Also, I think you're much more likely to avoid implicitly stripping sugar if 
you take both of these (or at least FromType?) as a QualType, and also return a 
QualType.  Or you could pass the original QualType down separately and just 
return that in the cases where you don't need to change anything.



Comment at: lib/Sema/SemaExpr.cpp:7251
+if (const auto *rhproto = dyn_cast(rhptee))
+  rhptee = S.removeNoEscapeFromFunctionProto(lhproto, rhproto);
+

I think the right place to do this is probably mergeFunctionTypes.



Comment at: lib/Sema/SemaTemplate.cpp:5477
+  }
+}
+

I think this is not the right place to be doing this; instead, we should be 
applying a conversion to the argument expression in CheckTemplateArgument.  It 
looks like we already do that in C++17 mode, because C++17 broadened the rules 
here to allow an arbitrary function conversion (among the other things allowed 
by the "converted constant expression" rules, [temp.arg.nontype]p2).  We 
probably could just leave it at that, since earlier standards are (in their 
infinite wisdom) stricter about type-matching here.  If that poses a serious 
compatibility problem, the right fix is to modify the clause in 
CheckTemplateArgument for non-type template parameters where it deals with 
pointers, references, and member pointers to functions to specifically allow 
this conversion.  But I'll note that we don't seem to be similarly generous 
about noreturn.


https://reviews.llvm.org/D32210



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

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

2017-08-05 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits 
gtbercea updated this revision to Diff 109903.
gtbercea added a comment.

  New way to handle OpenMP target flags.


https://reviews.llvm.org/D34784

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Driver/Compilation.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -597,3 +597,19 @@
 // RUN:   | FileCheck -check-prefix=CHK-FOPENMP-IS-DEVICE %s
 
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}} "-aux-triple" "powerpc64le--linux" {{.*}}.c" "-fopenmp-is-device" "-fopenmp-host-ir-file-path"
+
+/// ###
+
+/// Check -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-EQ-TARGET %s
+
+// CHK-FOPENMP-EQ-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8'
+
+/// ###
+
+/// Check -Xopenmp-target -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET %s
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target -march=pwr8'
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -212,8 +212,20 @@
   static_cast(getToolChain());
   assert(TC.getTriple().isNVPTX() && "Wrong platform");
 
+  StringRef GPUArchName;
+  std::vector GPUArchNames;
+  // If this is an OpenMP action we need to extract the device architecture from
+  // the -fopenmp-target-arch option.
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP)) {
+GPUArchNames = Args.getAllArgValues(options::OPT_march_EQ);
+assert(GPUArchNames.size() == 1 &&
+   "Exactly one GPU Arch required for ptxas.");
+GPUArchName = GPUArchNames[0];
+  } else
+GPUArchName = JA.getOffloadingArch();
+
   // Obtain architecture from the action.
-  CudaArch gpu_arch = StringToCudaArch(JA.getOffloadingArch());
+  CudaArch gpu_arch = StringToCudaArch(GPUArchName);
   assert(gpu_arch != CudaArch::UNKNOWN &&
  "Device action expected to have an architecture.");
 
@@ -405,7 +417,7 @@
 
   // For OpenMP device offloading, append derived arguments. Make sure
   // flags are not duplicated.
-  // TODO: Append the compute capability.
+  // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
 for (Arg *A : Args){
   bool IsDuplicate = false;
@@ -418,6 +430,15 @@
   if (!IsDuplicate)
 DAL->append(A);
 }
+
+auto MArchList = DAL->getAllArgValues(options::OPT_march_EQ);
+assert(MArchList.size() < 2 &&
+   "Too many archs under -Xopenmp-targets");
+if (MArchList.empty())
+  // Default compute capability for CUDA toolchain is sm_20.
+  DAL->AddJoinedArg(nullptr,
+  Opts.getOption(options::OPT_march_EQ), "sm_20");
+
 return DAL;
   }
 
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -775,3 +775,66 @@
 
   return VersionTuple();
 }
+
+llvm::opt::DerivedArgList *
+ToolChain::TranslateOpenMPTargetArgs(const llvm::opt::DerivedArgList ,
+Action::OffloadKind DeviceOffloadKind) const {
+  if (DeviceOffloadKind == Action::OFK_OpenMP) {
+DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
+const OptTable  = getDriver().getOpts();
+
+// Handle -Xopenmp-target flags
+for (Arg *A : Args) {
+  // Exclude flags which may only apply to the host toolchain.
+  // TODO: find a way to keep flags from this group if host
+  //   toolchain and this offload toolchain have the same triple.
+  if (A->getOption().matches(options::OPT_m_Group))
+continue;
+
+  unsigned Index;
+  unsigned Prev;
+  bool XOpenMPTargetNoTriple = A->getOption().matches(
+  options::OPT_Xopenmp_target);
+
+  if (A->getOption().matches(options::OPT_Xopenmp_target_EQ)) {
+// Passing device args: -Xopenmp-target= -opt=val.
+if (A->getValue(0) == getTripleString())
+  Index = Args.getBaseArgs().MakeIndex(A->getValue(1));
+else
+  continue;
+  } else if (XOpenMPTargetNoTriple)
+// Passing device args: -Xopenmp-target -opt=val.
+Index =

Re: [PATCH] D24933: Enable configuration files in clang

2017-08-05 Thread Hal Finkel via cfe-commits 


On 07/24/2017 10:18 AM, Serge Pavlov wrote:
I am thinking about reducing the patch further to leave only the 
ability to include config file when clang is called as 
`target-clang-drivermode`. It is still useful for cross compilation 
tasks because:

- It is a convenient way to switch between supported targets,
- SDK producer can ship compiler with a set of appropriate options or 
prepare them during installation.
In this case if clang is called as `target-clang-drivermode`, it first 
tries to find file `target-drivermode.cfg` or `target.cfg` in a set of 
well-known directories, which in minimal case includes the directory 
where clang executable resides. If such file is found, options are 
 read from it, otherwise only option --target is added as clang does 
it now.


This solution has obvious drawbacks:
- User cannot specify config file in command line in the same way as 
he can choose a target: `clang --target `,
- On Windows symlinks are implemented as file copy, the solution looks 
awkward.
So more or less complete solution needs to allow specifying config 
file in command line.


I'd rather not reduce the patch in this way, and you didn't describe why 
you're considering reducing the patch. Can you please elaborate?




Using `@file` has some problems. Config file is merely a set of 
options, just as file included by `@file`. Different include file 
search is only a convenience and could be sacrificed. Comments and 
unused option warning suppression could be extended for all files 
included with `@file`. The real problem is the search path. To be 
useful, config files must be searched for in well-known directories, 
so that meaning of `clang @config_fille` does not depend on the 
current directory. So clang must have some rule to distinguish between 
config file and traditional use of `@file`. For instance, if file name 
ends with `.cfg` and there is a file with this name in config search 
directories, this is a config file and it is interpreted a bit 
differently. Of course, the file may be specified with full path, but 
this way is inconvenient.


I see no reason why we can't unify the processing but have different 
search-path rules for @file vs. --config file.




Another possible solution is to extend meaning of `--target` so that 
it fully matches with the use of `target-clang-drivermode`, that is 
the option `--target=hexagon` causes clang first to look for the file 
`hexagon.cfg` in well-known directories and use it if found. In this 
case treatment of `--target` is different if the option is specified 
in command line or in the content of config file (in the latter case 
it is processed as target name only), it may be confusing. Besides, 
use of config files is not restricted to the choice of target.


I think we should do this, so long as the implementation is reasonable, 
and the special case doesn't bother me in this regard. I don't view this 
as a replacement for '--config file', however, because, as you mention, 
the config files need not be restricted to target triples.


Thanks again,
Hal



Using special option for config files does not bring risk of 
compatibility breakage and does not change meaning of existing options.



Thanks,
--Serge

2017-05-10 11:25 GMT+07:00 Serge Pavlov >:


2017-05-10 3:46 GMT+07:00 Richard Smith >:

On 1 March 2017 at 02:50, Serge Pavlov via Phabricator
>
wrote:


Format of configuration file is similar to file used in
the construct `@file`, it is a set of options.
Configuration file have advantage over this construct:

- it is searched for in well-known places rather than in
current directory,


This (and suppressing unused-argument warnings) might well be
sufficient to justify a different command-line syntax rather
than @file...


Construct `@file` in this implementation is used only to read
parts of config file inside containing file. Driver knows that it
processes config file and can adjust treatment of `@file`. On the
other hand, driver might parse config files in a more complicated
way, for instance, it could treat line `# include(file_name)` as a
command to include another file.

- it may contain comments, long options may be split
between lines using trailing backslashes,
- other files may be included by `@file` and they will be
resolved relative to the including file,


... but I think we should just add these extensions to our
@file handling, and then use the exact same syntax and code to
handle config files and @file files. That is, the difference
between @ and --config would be that the latter looks in a
different directory and suppresses "unused argument"

[PATCH] D36362: [X86] Enable isel to use the PAUSE instruction even when SSE2 is disabled. Clang part

2017-08-05 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310191: [X86] Enable isel to use the PAUSE instruction even 
when SSE2 is disabled. (authored by ctopper).

Changed prior to commit:
  https://reviews.llvm.org/D36362?vs=109896=109902#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36362

Files:
  cfe/trunk/include/clang/Basic/BuiltinsX86.def
  cfe/trunk/test/CodeGen/pause.c


Index: cfe/trunk/include/clang/Basic/BuiltinsX86.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def
@@ -338,8 +338,8 @@
 TARGET_HEADER_BUILTIN(_mm_lfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
 TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
 TARGET_HEADER_BUILTIN(_mm_mfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
-TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2")
-TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
+TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "")
+TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "", "sse2")
Index: cfe/trunk/test/CodeGen/pause.c
===
--- cfe/trunk/test/CodeGen/pause.c
+++ cfe/trunk/test/CodeGen/pause.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature 
-sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature 
+sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+
+#include 
+
+void test_mm_pause() {
+  // CHECK-LABEL: test_mm_pause
+  // CHECK: call void @llvm.x86.sse2.pause()
+  return _mm_pause();
+}


Index: cfe/trunk/include/clang/Basic/BuiltinsX86.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def
@@ -338,8 +338,8 @@
 TARGET_HEADER_BUILTIN(_mm_lfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
 TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
 TARGET_HEADER_BUILTIN(_mm_mfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
-TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2")
-TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
+TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "")
+TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "", "sse2")
Index: cfe/trunk/test/CodeGen/pause.c
===
--- cfe/trunk/test/CodeGen/pause.c
+++ cfe/trunk/test/CodeGen/pause.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature -sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+
+#include 
+
+void test_mm_pause() {
+  // CHECK-LABEL: test_mm_pause
+  // CHECK: call void @llvm.x86.sse2.pause()
+  return _mm_pause();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r310191 - [X86] Enable isel to use the PAUSE instruction even when SSE2 is disabled. Clang part

2017-08-05 Thread Craig Topper via cfe-commits 
Author: ctopper
Date: Sat Aug  5 16:35:54 2017
New Revision: 310191

URL: http://llvm.org/viewvc/llvm-project?rev=310191=rev
Log:
[X86] Enable isel to use the PAUSE instruction even when SSE2 is disabled. 
Clang part

Summary:
On older processors this instruction encoding is treated as a NOP.

MSVC doesn't disable intrinsics based on features the way clang/gcc does. 
Because the PAUSE instruction encoding doesn't crash older processors, some 
software out there uses these intrinsics without checking for SSE2.

This change also seems to also be consistent with gcc behavior.

Fixes PR34079

Reviewers: RKSimon, zvi

Reviewed By: RKSimon

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/CodeGen/pause.c
Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=310191=310190=310191=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Sat Aug  5 16:35:54 2017
@@ -338,8 +338,8 @@ TARGET_BUILTIN(__builtin_ia32_lfence, "v
 TARGET_HEADER_BUILTIN(_mm_lfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
 TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
 TARGET_HEADER_BUILTIN(_mm_mfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
-TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2")
-TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
+TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "")
+TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "", "sse2")

Added: cfe/trunk/test/CodeGen/pause.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pause.c?rev=310191=auto
==
--- cfe/trunk/test/CodeGen/pause.c (added)
+++ cfe/trunk/test/CodeGen/pause.c Sat Aug  5 16:35:54 2017
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature 
-sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature 
+sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+
+#include 
+
+void test_mm_pause() {
+  // CHECK-LABEL: test_mm_pause
+  // CHECK: call void @llvm.x86.sse2.pause()
+  return _mm_pause();
+}


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

[PATCH] D36362: [X86] Enable isel to use the PAUSE instruction even when SSE2 is disabled. Clang part

2017-08-05 Thread Simon Pilgrim via Phabricator via cfe-commits 
RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D36362



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

2017-08-05 Thread Dominic Chen via Phabricator via cfe-commits 
ddcc added a comment.

@NoQ ping


https://reviews.llvm.org/D35450



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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

2017-08-05 Thread Jacob Bandes-Storch via Phabricator via cfe-commits 
jtbandes added inline comments.



Comment at: lib/Format/WhitespaceManager.cpp:650
+for (unsigned i = 0; i < Newlines; ++i)
+  Text.append(UseCRLF ? " \\\r\n" : " \\\n");
+return;

djasper wrote:
> Note that when you have an empty line, this would turn into:
> 
>   #define A \
> int i; \
>\<-- Note the 1-space indent here.
> int j; \
> int k;
> 
> With my alternative below, that "\" will just be put at column 0, which 
> probably isn't better or worse.
I suppose that can be changed back to 1 by using `std::max(1, 
EscapedNewlineColumn - 1);` instead, right? I don't have strong feelings about 
whether it should be 0 or 1.


https://reviews.llvm.org/D36019



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

[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines

2017-08-05 Thread Jacob Bandes-Storch via Phabricator via cfe-commits 
jtbandes updated this revision to Diff 109898.
jtbandes added a comment.

@djasper ok, done


https://reviews.llvm.org/D36019

Files:
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2309,6 +2309,30 @@
   EXPECT_EQ("template  f();", format("\\\ntemplate  f();"));
   EXPECT_EQ("/* \\  \\  \\\n */", format("\\\n/* \\  \\  \\\n */"));
   EXPECT_EQ("", format(""));
+
+  FormatStyle DontAlign = getLLVMStyle();
+  DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign;
+  DontAlign.MaxEmptyLinesToKeep = 3;
+  // FIXME: can't use verifyFormat here because the newline before
+  // "public:" is not inserted the first time it's reformatted
+  EXPECT_EQ("#define A \\\n"
+"  class Foo { \\\n"
+"void bar(); \\\n"
+"\\\n"
+"\\\n"
+"\\\n"
+"  public: \\\n"
+"void baz(); \\\n"
+"  };",
+format("#define A \\\n"
+   "  class Foo { \\\n"
+   "void bar(); \\\n"
+   "\\\n"
+   "\\\n"
+   "\\\n"
+   "  public: \\\n"
+   "void baz(); \\\n"
+   "  };", DontAlign));
 }
 
 TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) {
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -195,9 +195,9 @@
   /// \brief Stores \p Text as the replacement for the whitespace in \p Range.
   void storeReplacement(SourceRange Range, StringRef Text);
   void appendNewlineText(std::string , unsigned Newlines);
-  void appendNewlineText(std::string , unsigned Newlines,
- unsigned PreviousEndOfTokenColumn,
- unsigned EscapedNewlineColumn);
+  void appendEscapedNewlineText(std::string , unsigned Newlines,
+unsigned PreviousEndOfTokenColumn,
+unsigned EscapedNewlineColumn);
   void appendIndentText(std::string , unsigned IndentLevel,
 unsigned Spaces, unsigned WhitespaceStartColumn);
 
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -603,8 +603,9 @@
 if (C.CreateReplacement) {
   std::string ReplacementText = C.PreviousLinePostfix;
   if (C.ContinuesPPDirective)
-appendNewlineText(ReplacementText, C.NewlinesBefore,
-  C.PreviousEndOfTokenColumn, C.EscapedNewlineColumn);
+appendEscapedNewlineText(ReplacementText, C.NewlinesBefore,
+ C.PreviousEndOfTokenColumn,
+ C.EscapedNewlineColumn);
   else
 appendNewlineText(ReplacementText, C.NewlinesBefore);
   appendIndentText(ReplacementText, C.Tok->IndentLevel,
@@ -640,16 +641,17 @@
 Text.append(UseCRLF ? "\r\n" : "\n");
 }
 
-void WhitespaceManager::appendNewlineText(std::string , unsigned Newlines,
-  unsigned PreviousEndOfTokenColumn,
-  unsigned EscapedNewlineColumn) {
+void WhitespaceManager::appendEscapedNewlineText(std::string ,
+ unsigned Newlines,
+ unsigned 
PreviousEndOfTokenColumn,
+ unsigned 
EscapedNewlineColumn) {
   if (Newlines > 0) {
-unsigned Offset =
-std::min(EscapedNewlineColumn - 2, PreviousEndOfTokenColumn);
+unsigned Spaces =
+std::max(1, EscapedNewlineColumn - PreviousEndOfTokenColumn - 1);
 for (unsigned i = 0; i < Newlines; ++i) {
-  Text.append(EscapedNewlineColumn - Offset - 1, ' ');
+  Text.append(Spaces, ' ');
   Text.append(UseCRLF ? "\\\r\n" : "\\\n");
-  Offset = 0;
+  Spaces = std::max(0, EscapedNewlineColumn - 1);
 }
   }
 }


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2309,6 +2309,30 @@
   EXPECT_EQ("template  f();", format("\\\ntemplate  f();"));
   EXPECT_EQ("/* \\  \\  \\\n */", format("\\\n/* \\  \\  \\\n */"));
   EXPECT_EQ("", format(""));
+
+  FormatStyle DontAlign = getLLVMStyle();
+  DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign;
+  DontAlign.MaxEmptyLinesToKeep = 3;
+  // FIXME: can't use verifyFormat here because the newline before
+  // "public:" is not inserted the first time

[PATCH] D36362: [X86] Enable isel to use the PAUSE instruction even when SSE2 is disabled. Clang part

2017-08-05 Thread Craig Topper via Phabricator via cfe-commits 
craig.topper updated this revision to Diff 109896.
craig.topper added a comment.

Add test case


https://reviews.llvm.org/D36362

Files:
  include/clang/Basic/BuiltinsX86.def
  test/CodeGen/pause.c


Index: test/CodeGen/pause.c
===
--- /dev/null
+++ test/CodeGen/pause.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature 
-sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature 
+sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+
+#include 
+
+void test_mm_pause() {
+  // CHECK-LABEL: test_mm_pause
+  // CHECK: call void @llvm.x86.sse2.pause()
+  return _mm_pause();
+}
Index: include/clang/Basic/BuiltinsX86.def
===
--- include/clang/Basic/BuiltinsX86.def
+++ include/clang/Basic/BuiltinsX86.def
@@ -338,8 +338,8 @@
 TARGET_HEADER_BUILTIN(_mm_lfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
 TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
 TARGET_HEADER_BUILTIN(_mm_mfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
-TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2")
-TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
+TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "")
+TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "", "sse2")


Index: test/CodeGen/pause.c
===
--- /dev/null
+++ test/CodeGen/pause.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature -sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-pc-win32 -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+
+#include 
+
+void test_mm_pause() {
+  // CHECK-LABEL: test_mm_pause
+  // CHECK: call void @llvm.x86.sse2.pause()
+  return _mm_pause();
+}
Index: include/clang/Basic/BuiltinsX86.def
===
--- include/clang/Basic/BuiltinsX86.def
+++ include/clang/Basic/BuiltinsX86.def
@@ -338,8 +338,8 @@
 TARGET_HEADER_BUILTIN(_mm_lfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
 TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
 TARGET_HEADER_BUILTIN(_mm_mfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
-TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2")
-TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
+TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "")
+TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "", "sse2")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36364: [AArch64] Add support for a MinGW AArch64 target

2017-08-05 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo created this revision.
Herald added subscribers: kristof.beyls, javed.absar, rengolin, aemerson.

The machine name `arm64pe` passed to the linker is completely made up; gnu 
binutils doesn't (afaik) support this, so it only has to match what the 
corresponding (not yet merged) lld linker supports - similarly to the existing 
`thumb2pe` machine name.


https://reviews.llvm.org/D36364

Files:
  lib/Basic/Targets.cpp
  lib/Basic/Targets/AArch64.cpp
  lib/Basic/Targets/AArch64.h
  lib/Driver/ToolChains/CrossWindows.cpp
  lib/Driver/ToolChains/MinGW.cpp
  test/Preprocessor/predefined-macros.c

Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -199,3 +199,13 @@
 // CHECK-ARM64-WIN: #define _M_ARM64 1
 // CHECK-ARM64-WIN: #define _WIN32 1
 // CHECK-ARM64-WIN: #define _WIN64 1
+
+// RUN: %clang_cc1 -triple aarch64-windows-gnu %s -E -dM -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-MINGW
+
+// CHECK-ARM64-MINGW-NOT: #define _M_ARM64 1
+// CHECK-ARM64-MINGW: #define WIN32 1
+// CHECK-ARM64-MINGW: #define WIN64 1
+// CHECK-ARM64-MINGW: #define _WIN32 1
+// CHECK-ARM64-MINGW: #define _WIN64 1
+// CHECK-ARM64-MINGW: #define __aarch64__ 1
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -121,10 +121,14 @@
   CmdArgs.push_back("-m");
   if (TC.getArch() == llvm::Triple::x86)
 CmdArgs.push_back("i386pe");
-  if (TC.getArch() == llvm::Triple::x86_64)
+  else if (TC.getArch() == llvm::Triple::x86_64)
 CmdArgs.push_back("i386pep");
-  if (TC.getArch() == llvm::Triple::arm)
+  else if (TC.getArch() == llvm::Triple::arm)
 CmdArgs.push_back("thumb2pe");
+  else if (TC.getArch() == llvm::Triple::aarch64)
+CmdArgs.push_back("arm64pe");
+  else
+llvm_unreachable("Unsupported target architecture.");
 
   if (Args.hasArg(options::OPT_mwindows)) {
 CmdArgs.push_back("--subsystem");
Index: lib/Driver/ToolChains/CrossWindows.cpp
===
--- lib/Driver/ToolChains/CrossWindows.cpp
+++ lib/Driver/ToolChains/CrossWindows.cpp
@@ -36,6 +36,7 @@
 llvm_unreachable("unsupported architecture");
   case llvm::Triple::arm:
   case llvm::Triple::thumb:
+  case llvm::Triple::aarch64:
 break;
   case llvm::Triple::x86:
 CmdArgs.push_back("--32");
@@ -98,6 +99,9 @@
 // FIXME: this is incorrect for WinCE
 CmdArgs.push_back("thumb2pe");
 break;
+  case llvm::Triple::aarch64:
+CmdArgs.push_back("arm64pe");
+break;
   case llvm::Triple::x86:
 CmdArgs.push_back("i386pe");
 EntryPoint.append("_");
@@ -111,6 +115,7 @@
 switch (T.getArch()) {
 default:
   llvm_unreachable("unsupported architecture");
+case llvm::Triple::aarch64:
 case llvm::Triple::arm:
 case llvm::Triple::thumb:
 case llvm::Triple::x86_64:
Index: lib/Basic/Targets/AArch64.h
===
--- lib/Basic/Targets/AArch64.h
+++ lib/Basic/Targets/AArch64.h
@@ -89,24 +89,42 @@
   void setDataLayout() override;
 };
 
-class LLVM_LIBRARY_VISIBILITY MicrosoftARM64TargetInfo
+class LLVM_LIBRARY_VISIBILITY WindowsARM64TargetInfo
 : public WindowsTargetInfo {
   const llvm::Triple Triple;
 
 public:
-  MicrosoftARM64TargetInfo(const llvm::Triple ,
-   const TargetOptions );
+  WindowsARM64TargetInfo(const llvm::Triple ,
+ const TargetOptions );
 
   void setDataLayout() override;
 
+  BuiltinVaListKind getBuiltinVaListKind() const override;
+
+  CallingConvCheckResult checkCallingConvention(CallingConv CC) const override;
+};
+
+// Windows ARM, MS (C++) ABI
+class LLVM_LIBRARY_VISIBILITY MicrosoftARM64TargetInfo
+: public WindowsARM64TargetInfo {
+public:
+  MicrosoftARM64TargetInfo(const llvm::Triple ,
+   const TargetOptions );
+
   void getVisualStudioDefines(const LangOptions ,
   MacroBuilder ) const;
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override;
+};
 
-  BuiltinVaListKind getBuiltinVaListKind() const override;
+// ARM64 MinGW target
+class LLVM_LIBRARY_VISIBILITY MinGWARM64TargetInfo
+: public WindowsARM64TargetInfo {
+public:
+  MinGWARM64TargetInfo(const llvm::Triple , const TargetOptions );
 
-  CallingConvCheckResult checkCallingConvention(CallingConv CC) const override;
+  void getTargetDefines(const LangOptions ,
+MacroBuilder ) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY AArch64beTargetInfo : public AArch64TargetInfo {
Index: lib/Basic/Targets/AArch64.cpp
===
--- lib/Basic/Targets/AArch64.cpp
+++

[PATCH] D36363: [test] Remove an unintentional -x cl flag in an aarch64-windows test

2017-08-05 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo created this revision.
Herald added subscribers: kristof.beyls, Anastasia, rengolin, aemerson.

This test was only intended to test compiling C, not OpenCL.


https://reviews.llvm.org/D36363

Files:
  test/Preprocessor/predefined-macros.c


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -193,7 +193,7 @@
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_WORK_GROUP 1
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_WORK_ITEM 0
 
-// RUN: %clang_cc1 -triple aarch64-windows %s -E -dM -o - -x cl \
+// RUN: %clang_cc1 -triple aarch64-windows %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-WIN
 
 // CHECK-ARM64-WIN: #define _M_ARM64 1


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -193,7 +193,7 @@
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_WORK_GROUP 1
 // MSCOPE:#define __OPENCL_MEMORY_SCOPE_WORK_ITEM 0
 
-// RUN: %clang_cc1 -triple aarch64-windows %s -E -dM -o - -x cl \
+// RUN: %clang_cc1 -triple aarch64-windows %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-ARM64-WIN
 
 // CHECK-ARM64-WIN: #define _M_ARM64 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36362: [X86] Enable isel to use the PAUSE instruction even when SSE2 is disabled. Clang part

2017-08-05 Thread Simon Pilgrim via Phabricator via cfe-commits 
RKSimon added a comment.

Codegen test case on no-sse2 target?


https://reviews.llvm.org/D36362



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

[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

2017-08-05 Thread Taewook Oh via Phabricator via cfe-commits 
twoh added a comment.

I think it is generally good to match what GCC does to not to confuse people.


https://reviews.llvm.org/D34796



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

[PATCH] D36362: [X86] Enable isel to use the PAUSE instruction even when SSE2 is disabled. Clang part

2017-08-05 Thread Craig Topper via Phabricator via cfe-commits 
craig.topper created this revision.

On older processors this instruction encoding is treated as a NOP.

MSVC doesn't disable intrinsics based on features the way clang/gcc does. 
Because the PAUSE instruction encoding doesn't crash older processors, some 
software out there uses these intrinsics without checking for SSE2.

This change also seems to also be consistent with gcc behavior.


https://reviews.llvm.org/D36362

Files:
  include/clang/Basic/BuiltinsX86.def


Index: include/clang/Basic/BuiltinsX86.def
===
--- include/clang/Basic/BuiltinsX86.def
+++ include/clang/Basic/BuiltinsX86.def
@@ -338,8 +338,8 @@
 TARGET_HEADER_BUILTIN(_mm_lfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
 TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
 TARGET_HEADER_BUILTIN(_mm_mfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
-TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2")
-TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, 
"sse2")
+TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "")
+TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "", "sse2")


Index: include/clang/Basic/BuiltinsX86.def
===
--- include/clang/Basic/BuiltinsX86.def
+++ include/clang/Basic/BuiltinsX86.def
@@ -338,8 +338,8 @@
 TARGET_HEADER_BUILTIN(_mm_lfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
 TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
 TARGET_HEADER_BUILTIN(_mm_mfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
-TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2")
-TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
+TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "")
+TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "")
 TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "", "sse2")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36353: Instantiate constexpr function when it is used

2017-08-05 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

The current behaviour is correct according to the latest discussions of CWG. We 
shouldn't be delaying instantiation; we just need to fix the recent regression 
for functions where we try to instantiate functions that don't have a complete 
body yet.


https://reviews.llvm.org/D36353



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

[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

2017-08-05 Thread David Li via Phabricator via cfe-commits 
davidxl added a comment.

The patch itself is fine. The meta question is whether we expect this option to 
be generally useful?


https://reviews.llvm.org/D34796



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

[PATCH] D35782: [C++2a][Preprocessor] Implement p0306 __VA_OPT__ (Comma omission and comma deletion)

2017-08-05 Thread Faisal Vali via Phabricator via cfe-commits 
faisalv updated this revision to Diff 109883.
faisalv marked 7 inline comments as done.
faisalv added a comment.

Hi Richard,

  This patch attempts to incorporate all the changes you requested.  

You asked for some consideration on the rationale for sticking with my approach 
(as opposed to the alternative you posed), so here are my reasons (FWTW) for 
not modeling the VA_OPT as a macro argument:

- the standard does not specify it as behaving exactly as a macro argument 
(specifically no rescanning, after placemarker token removal) - and so i would 
have to special case the argument expansion logic - which currently composes 
well by just launching a tokenlexer on the argument.
- it would be the only argument that would need to have *other* arguments 
substituted into it
- unlike every other argument it would not be represented by just 1 token
- in regards to the source-location mapping, my sense is that the ExpansionInfo 
that represents the various expansions might need to add a representation for 
this 'fourth' kind of expansion (since expanded args are represented by a valid 
LocStart but an invalid LocEnd, and since this is more than one token, things 
could get hairy)
- it might require complicating 'Parsing' the macro definition (and the 
MacroArgs, MacroInfo objects) and the general tokenLexer, 
ExpandFunctionArguments() logic more than any gained conceptual elegance (in my 
opinion)

Thanks Richard - Look forward to your review and thoughts!


Repository:
  rL LLVM

https://reviews.llvm.org/D35782

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/TokenLexer.h
  include/clang/Lex/VariadicMacroSupport.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/Preprocessor.cpp
  lib/Lex/TokenLexer.cpp
  test/Preprocessor/macro_vaopt_check.cpp
  test/Preprocessor/macro_vaopt_expand.cpp

Index: test/Preprocessor/macro_vaopt_expand.cpp
===
--- test/Preprocessor/macro_vaopt_expand.cpp
+++ test/Preprocessor/macro_vaopt_expand.cpp
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -E %s -pedantic -std=c++2a | FileCheck -strict-whitespace %s
+
+#define LPAREN ( 
+#define RPAREN ) 
+
+#define A0 expandedA0
+#define A1  expandedA1 A0
+#define A2  expandedA2 A1
+#define A3  expandedA3 A2
+
+#define A() B LPAREN )
+#define B() C LPAREN )
+#define C() D LPAREN )
+
+
+#define F(x, y) x + y 
+#define ELLIP_FUNC(...) __VA_OPT__(__VA_ARGS__)
+
+1: ELLIP_FUNC(F, LPAREN, 'a', 'b', RPAREN); 
+2: ELLIP_FUNC(F LPAREN 'a', 'b' RPAREN); 
+#undef F
+#undef ELLIP_FUNC
+
+// CHECK: 1: F, (, 'a', 'b', );
+// CHECK: 2: 'a' + 'b';
+
+#define F(...) f(0 __VA_OPT__(,) __VA_ARGS__)
+3: F(a, b, c) // replaced by f(0, a, b, c) 
+4: F() // replaced by f(0)
+
+// CHECK: 3: f(0 , a, b, c) 
+// CHECK: 4: f(0 )
+#undef F
+
+#define G(X, ...) f(0, X __VA_OPT__(,) __VA_ARGS__)
+
+5: G(a, b, c) // replaced by f(0, a , b, c) 
+6: G(a) // replaced by f(0, a) 
+7: G(a,) // replaced by f(0, a) 
+7.1: G(a,,)
+
+
+// CHECK: 5: f(0, a , b, c) 
+// CHECK: 6: f(0, a ) 
+// CHECK: 7: f(0, a ) 
+// CHECK: 7.1: f(0, a , ,)
+#undef G 
+
+#define HT_B() TONG
+
+#define F(x, ...) HT_ ## __VA_OPT__(x x A()  #x)
+
+8: F(1)
+9: F(A(),1)
+
+// CHECK: 8: HT_
+// CHECK: 9: TONG C ( ) B ( ) "A()"
+#undef HT_B
+#undef F
+
+#define F(a,...) #__VA_OPT__(A1 a)
+
+10: F(A())
+11: F(A1 A(), 1)
+// CHECK: 10: ""
+// CHECK: 11: "A1 expandedA1 expandedA0 B ( )"
+#undef F
+
+
+#define F(a,...) a ## __VA_OPT__(A1 a) ## __VA_ARGS__ ## a
+12.0: F()
+12: F(,)
+13: F(B,)
+// CHECK: 12.0: 
+// CHECK: 12: 
+// CHECK: 13: BB 
+#undef F
+
+#define F(...) #__VA_OPT__()  X ## __VA_OPT__()  #__VA_OPT__()
+
+14: F()
+15: F(1)
+
+// CHECK: 14: "" X ""
+// CHECK: 15: "" X ""
+
+#undef F
+
+#define SDEF(sname, ...) S sname __VA_OPT__(= { __VA_ARGS__ })
+
+16: SDEF(foo); // replaced by S foo; 
+17: SDEF(bar, 1, 2); // replaced by S bar = { 1, 2 }; 
+
+// CHECK: 16: S foo ;
+// CHECK: 17: S bar = { 1, 2 }; 
+#undef SDEF
+
+#define F(a,...) A() #__VA_OPT__(A3 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A3) A()
+
+18: F()
+19: F(,)
+20: F(,A3)
+21: F(A3, A(),A0)
+
+
+// CHECK: 18: B ( ) "" B ( ) 
+// CHECK: 19: B ( ) "" B ( ) 
+// CHECK: 20: B ( ) "A3 expandedA3 expandedA2 expandedA1 expandedA0 A3C A3" B ( )
+// CHECK: 21: B ( ) "A3 B ( ),expandedA0 A3A(),A0A3C A3" B ( )
+
+#undef F
+
+#define F(a,...) A() #__VA_OPT__(A3 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A3) a __VA_OPT__(A0 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A0) A()
+
+22: F()
+23: F(,)
+24: F(,A0)
+25: F(A0, A(),A0)
+
+
+// CHECK: 22: B ( ) "" B ( ) 
+// CHECK: 23: B ( ) "" B ( ) 
+// CHECK: 24: B ( ) "A3 expandedA0 A0C A3" expandedA0 expandedA0 A0C expandedA0 B ( )
+// CHECK: 25: B ( ) "A3 B ( ),expandedA0 A0A(),A0A0C A3" expandedA0 expandedA0 C ( ),expandedA0 A0A(),A0A0C expandedA0 B ( )
+
+#undef F
+
+#define F(a,...)  __VA_OPT__(B a ## a) ## 1
+#define G(a,...)  __VA_OPT__(B a) ## 1
+26: F(,1)
+26_1: G(,1)
+// CHECK: 26: B1
+// CHECK:

[PATCH] D36359: [clang-format] Put '/**' and '*/' on own lines in jsdocs ending in comment pragmas

2017-08-05 Thread Krasimir Georgiev via Phabricator via cfe-commits 
krasimir created this revision.
Herald added a subscriber: klimek.

This handles a case where the trailing '*/' of a multiline jsdoc eding in a 
comment pragma wouldn't be put on a new line.


https://reviews.llvm.org/D36359

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -158,6 +158,53 @@
"var x = 1;\n"
"}",
getGoogleJSStyleWithColumns(20)));
+
+  // Don't break the first line of a single line short jsdoc comment pragma.
+  EXPECT_EQ("/** @returns j */",
+format("/** @returns j */",
+   getGoogleJSStyleWithColumns(20)));
+
+  // Break a single line long jsdoc comment pragma.
+  EXPECT_EQ("/**\n"
+" * @returns {string} jsdoc line 12\n"
+" */",
+format("/** @returns {string} jsdoc line 12 */",
+   getGoogleJSStyleWithColumns(20)));
+
+  EXPECT_EQ("/**\n"
+" * @returns {string} jsdoc line 12\n"
+" */",
+format("/** @returns {string} jsdoc line 12  */",
+   getGoogleJSStyleWithColumns(20)));
+
+  EXPECT_EQ("/**\n"
+" * @returns {string} jsdoc line 12\n"
+" */",
+format("/** @returns {string} jsdoc line 12*/",
+   getGoogleJSStyleWithColumns(20)));
+
+  // Fix a multiline jsdoc comment ending in a comment pragma.
+  EXPECT_EQ("/**\n"
+" * line 1\n"
+" * line 2\n"
+" * @returns {string} jsdoc line 12\n"
+" */",
+format("/** line 1\n"
+   " * line 2\n"
+   " * @returns {string} jsdoc line 12 */",
+   getGoogleJSStyleWithColumns(20)));
+
+  EXPECT_EQ("/**\n"
+" * line 1\n"
+" * line 2\n"
+" *\n"
+" * @returns j\n"
+" */",
+format("/** line 1\n"
+   " * line 2\n"
+   " *\n"
+   " * @returns j */",
+   getGoogleJSStyleWithColumns(20)));
 }
 
 TEST_F(FormatTestJS, UnderstandsJavaScriptOperators) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1383,8 +1383,8 @@
 }
   }
 
-  BreakableToken::Split SplitAfterLastLine = Token->getSplitAfterLastLine(
-  TailOffset, ColumnLimit, CommentPragmasRegex);
+  BreakableToken::Split SplitAfterLastLine =
+  Token->getSplitAfterLastLine(TailOffset, ColumnLimit);
   if (SplitAfterLastLine.first != StringRef::npos) {
 if (!DryRun)
   Token->replaceWhitespaceAfterLastLine(TailOffset, SplitAfterLastLine,
Index: lib/Format/BreakableToken.h
===
--- lib/Format/BreakableToken.h
+++ lib/Format/BreakableToken.h
@@ -161,8 +161,8 @@
   ///
   /// A result having offset == StringRef::npos means that no reformat is
   /// necessary.
-  virtual Split getSplitAfterLastLine(unsigned TailOffset, unsigned ColumnLimit,
-  llvm::Regex ) const {
+  virtual Split getSplitAfterLastLine(unsigned TailOffset,
+  unsigned ColumnLimit) const {
 return Split(StringRef::npos, 0);
   }
 
@@ -347,8 +347,8 @@
   void replaceWhitespaceBefore(unsigned LineIndex, unsigned PreviousEndColumn,
unsigned ColumnLimit, Split SplitBefore,
WhitespaceManager ) override;
-  Split getSplitAfterLastLine(unsigned TailOffset, unsigned ColumnLimit,
-  llvm::Regex ) const override;
+  Split getSplitAfterLastLine(unsigned TailOffset,
+  unsigned ColumnLimit) const override;
 
   bool mayReflow(unsigned LineIndex,
  llvm::Regex ) const override;
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -681,12 +681,17 @@
   InPPDirective, /*Newlines=*/1, ContentColumn[LineIndex] - Prefix.size());
 }
 
-BreakableToken::Split BreakableBlockComment::getSplitAfterLastLine(
-unsigned TailOffset, unsigned ColumnLimit,
-llvm::Regex ) const {
-  if (DelimitersOnNewline)
-return getSplit(Lines.size() - 1, TailOffset, ColumnLimit,
-CommentPragmasRegex);
+BreakableToken::Split
+BreakableBlockComment::getSplitAfterLastLine(unsigned TailOffset,
+ unsigned ColumnLimit) const {
+  if (DelimitersOnNewline) {
+StringRef TrimmedContent =

[PATCH] D35847: clang-format: Fix left pointer alignment after delctype/typeof

2017-08-05 Thread Krasimir Georgiev via Phabricator via cfe-commits 
krasimir accepted this revision.
krasimir added a comment.
This revision is now accepted and ready to land.

Looks good! Please reformat the newly added code blocks with `clang-format` 
before submitting.




Comment at: lib/Format/TokenAnnotator.cpp:1402
+  FormatToken *TokenBeforeMatchingParen =
+PrevToken->MatchingParen->getPreviousNonComment();
+  if (TokenBeforeMatchingParen &&

Please reformat this block with `clang-format`: `PrevToken` gets indented by 
two more columns to the right.



Comment at: lib/Format/TokenAnnotator.cpp:2212
+if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
+  if (!Left.MatchingParen)
+return true;

Pleasereformat this block with `clang-format`: when I run it, it produces:
```
  if (Right.is(TT_PointerOrReference)) {
if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
  if (!Left.MatchingParen)
return true;
  FormatToken *TokenBeforeMatchingParen =
  Left.MatchingParen->getPreviousNonComment();
  if (!TokenBeforeMatchingParen ||
  !TokenBeforeMatchingParen->isOneOf(tok::kw_typeof, tok::kw_decltype))
return true;
}
return (Left.Tok.isLiteral() ||
(Left.is(tok::kw_const) && Left.Previous &&
 Left.Previous->is(tok::r_paren)) ||
(!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
 (Style.PointerAlignment != FormatStyle::PAS_Left ||
  (Line.IsMultiVariableDeclStmt &&
   (Left.NestingLevel == 0 ||
(Left.NestingLevel == 1 && Line.First->is(tok::kw_for)));
  }
```


https://reviews.llvm.org/D35847



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

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

2017-08-05 Thread Blitz Rakete via Phabricator via cfe-commits 
Rakete created this revision.

This adds a new error for missing parentheses around lambdas in delete 
operators.

  int main() {
delete []() { return new int(); }();
  }

This will result in:

  test.cpp:2:10: error: missing parentheses around lambda expression
delete []() { return new int(); }();
   ^~~
  1 error generated.




https://reviews.llvm.org/D36357

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  lib/Parse/ParseExprCXX.cpp
  test/Parser/cxx0x-lambda-expressions.cpp
  test/SemaCXX/new-delete-0x.cpp


Index: test/SemaCXX/new-delete-0x.cpp
===
--- test/SemaCXX/new-delete-0x.cpp
+++ test/SemaCXX/new-delete-0x.cpp
@@ -34,6 +34,5 @@
 void bad_deletes()
 {
   // 'delete []' is always array delete, per [expr.delete]p1.
-  // FIXME: Give a better diagnostic.
-  delete []{ return (int*)0; }(); // expected-error {{expected expression}}
+  delete []{ return (int*)0; }(); // expected-error{{missing parentheses 
around lambda expression}}
 }
Index: test/Parser/cxx0x-lambda-expressions.cpp
===
--- test/Parser/cxx0x-lambda-expressions.cpp
+++ test/Parser/cxx0x-lambda-expressions.cpp
@@ -53,7 +53,7 @@
   void delete_lambda(int *p) {
 delete [] p;
 delete [] (int*) { new int }; // ok, compound-literal, not lambda
-delete [] { return new int; } (); // expected-error{{expected expression}}
+delete [] { return new int; } (); // expected-error{{missing parentheses 
around lambda expression}}
 delete [&] { return new int; } (); // ok, lambda
   }
 
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -2884,15 +2884,39 @@
 //   [Footnote: A lambda expression with a lambda-introducer that consists
 //  of empty square brackets can follow the delete keyword if
 //  the lambda expression is enclosed in parentheses.]
-// FIXME: Produce a better diagnostic if the '[]' is unambiguously a
-//lambda-introducer.
-ArrayDelete = true;
-BalancedDelimiterTracker T(*this, tok::l_square);
+TentativeParsingAction TPA(*this);
 
-T.consumeOpen();
-T.consumeClose();
-if (T.getCloseLocation().isInvalid())
-  return ExprError();
+// Basic lookahead to check if we have a lambda expression. If we
+// encounter two braces with a semicolon, we can be pretty sure
+// that this is a lambda, not say a compound literal. 
+if (!SkipUntil(tok::l_brace, SkipUntilFlags::StopAtSemi) ||
+(NextToken().isNot(tok::r_brace) && !SkipUntil(tok::semi)) ||
+!SkipUntil(tok::r_brace, SkipUntilFlags::StopAtSemi)) {
+  TPA.Revert();
+  ArrayDelete = true;
+  BalancedDelimiterTracker T(*this, tok::l_square);
+
+  T.consumeOpen();
+  T.consumeClose();
+  if (T.getCloseLocation().isInvalid())
+return ExprError();
+} else {
+  TPA.Revert();
+
+  // Warn if the non-capturing lambda isn't surrounded by parenthesis
+  // to disambiguate it from 'delete[]'. 
+  ExprResult Lambda = TryParseLambdaExpression();
+  if (!Lambda.isInvalid()) {
+SourceLocation StartLoc = Lambda.get()->getLocStart();
+Diag(StartLoc, diag::err_lambda_after_array_delete)
+  << SourceRange(StartLoc, Lambda.get()->getLocEnd());
+
+// Evaluate any postfix expressions used on the lambda.
+Lambda = ParsePostfixExpressionSuffix(Lambda);
+return Actions.ActOnCXXDelete(Start, UseGlobal, /*ArrayForm=*/false,
+  Lambda.get());
+  }
+}
   }
 
   ExprResult Operand(ParseCastExpression(false));
Index: include/clang/Basic/DiagnosticParseKinds.td
===
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -99,6 +99,8 @@
   InGroup, DefaultIgnore;
 def ext_alignof_expr : ExtWarn<
   "%0 applied to an expression is a GNU extension">, 
InGroup;
+def err_lambda_after_array_delete : Error<
+  "missing parentheses around lambda expression">;
 
 def warn_microsoft_dependent_exists : Warning<
   "dependent %select{__if_not_exists|__if_exists}0 declarations are ignored">, 


Index: test/SemaCXX/new-delete-0x.cpp
===
--- test/SemaCXX/new-delete-0x.cpp
+++ test/SemaCXX/new-delete-0x.cpp
@@ -34,6 +34,5 @@
 void bad_deletes()
 {
   // 'delete []' is always array delete, per [expr.delete]p1.
-  // FIXME: Give a better diagnostic.
-  delete []{ return (int*)0; }(); // expected-error {{expected expression}}
+  delete []{ return (int*)0; }(); // expected-error{{missing parentheses around lambda expression}}
 }
Index: test/Parser/cxx0x-lambda-expressions.cpp

Re: r305903 - Function with unparsed body is a definition

2017-08-05 Thread Serge Pavlov via cfe-commits 
This problem is addressed by https://reviews.llvm.org/D36353.

Thanks,
--Serge

2017-08-05 8:44 GMT+07:00 Alexander Kornienko :

> Thank you for the analysis! It would be nice if you could implement a
> short term workaround to avoid crashes. Maybe Richard has ideas on what a
> proper solution should look like.
>
>
> On Wed, Aug 2, 2017 at 8:05 PM, Serge Pavlov  wrote:
>
>> At first thank you for the nice test case.
>>
>> This crash is not caused by r305903. The root cause is that instantiation
>> of f is triggered when parse of f is not finished yet. This is the
>> case just addressed by that change.
>> The instantiation is requested by the code (https://github.com/llvm-mirro
>> r/clang/blob/master/lib/Sema/SemaExpr.cpp#L13622):
>>
>> else if (Func->isConstexpr())
>> // Do not defer instantiations of constexpr functions, to avoid
>> the
>> // expression evaluator needing to call back into Sema if it sees
>> a
>> // call to such a function.
>> InstantiateFunctionDefinition(PointOfInstantiation, Func);
>>
>>
>> It looks like a violation of C++ Standard, as function is instantiated
>> when there is no use of it. However if this check is removed and
>> instantiation is postponed as for non-constexpr functions, lots of tests
>> fail.
>>
>> Without r305903 the call to `getDefinition` for templated declaration of
>> f returned null, declaration f was moved to the list of pending
>> instantiations and crash is not observed. It does not mean however that
>> original source code would be compiled correctly.
>>
>> It this is urgent problem, I can prepare a patch that will mimic behavior
>> before r305903 for instantiation of constexpr functions. If there is some
>> time I would investigate this problem in depth, as it manifests itself in
>> number of cases, for instance in https://bugs.llvm.org/show_
>> bug.cgi?id=33561.
>>
>> Thanks,
>> --Serge
>>
>> 2017-08-01 23:28 GMT+07:00 Serge Pavlov :
>>
>>> Yes, sure, I will investigate it.
>>>
>>> Thanks,
>>> --Serge
>>>
>>> 2017-08-01 21:32 GMT+07:00 Alexander Kornienko :
>>>
 This change causes an assertion failure on valid code. Could you take a
 look at fixing this?

 A reduced test case:

 $ cat /tmp/SemaTemplateInstantiateDecl-crash2.cpp
 template 
 constexpr void f(T) {
   f(0);
 }
 $ clang -fsyntax-only -std=c++11 /tmp/SemaTemplateInstantiateDe
 cl-crash2.cpp
 assert.h assertion failed at llvm/tools/clang/lib/Sema/Sema
 TemplateInstantiateDecl.cpp:3840 in void clang::Sema::InstantiateFuncti
 onDefinition(clang::SourceLocation, clang::FunctionDecl *, bool, bool,
 bool): (Pattern |
 | PatternDecl->isDefaulted()) && "unexpected kind of function template
 definition"
 *** Check failure stack trace: ***
 @  0x6094c4a  __assert_fail
 @  0x2705bfe  clang::Sema::InstantiateFunctionDefinition()
 @  0x2c1fb13  clang::Sema::MarkFunctionReferenced()
 @  0x2bec07b  clang::Sema::MarkAnyDeclReferenced()
 @  0x2c2327f  MarkExprReferenced()
 @  0x2be8f0a  clang::Sema::MarkDeclRefReferenced()
 @  0x2980ac0  clang::Sema::FixOverloadedFunctionReference()
 @  0x2982e66  FinishOverloadedCallExpr()
 @  0x2982b39  clang::Sema::BuildOverloadedCallExpr()
 @  0x2be461b  clang::Sema::ActOnCallExpr()
 @  0x2571e90  clang::Parser::ParsePostfixExpressionSuffix()
 @  0x25784cb  clang::Parser::ParseCastExpression()
 @  0x2570865  clang::Parser::ParseCastExpression()
 @  0x256f1e3  clang::Parser::ParseAssignmentExpression()
 @  0x256f0bf  clang::Parser::ParseExpression()
 @  0x2517eeb  clang::Parser::ParseExprStatement()
 @  0x2517074  clang::Parser::ParseStatement
 OrDeclarationAfterAttributes()
 @  0x2516b50  clang::Parser::ParseStatementOrDeclaration()
 @  0x251deb4  clang::Parser::ParseCompoundStatementBody()
 @  0x251ea53  clang::Parser::ParseFunctionStatementBody()
 @  0x24f5b23  clang::Parser::ParseFunctionDefinition()
 @  0x25082a5  clang::Parser::ParseSingleDec
 larationAfterTemplate()
 @  0x2507652  clang::Parser::ParseTemplateD
 eclarationOrSpecialization()
 @  0x2506fa5  clang::Parser::ParseDeclarati
 onStartingWithTemplate()
 @  0x25b6853  clang::Parser::ParseDeclaration()
 @  0x24f36d5  clang::Parser::ParseExternalDeclaration()
 @  0x24f2739  clang::Parser::ParseTopLevelDecl()
 @  0x24f220e  clang::Parser::ParseFirstTopLevelDecl()
 @  0x24ed582  clang::ParseAST()

 On Wed, Jun 21, 2017 at 2:46 PM, Serge Pavlov via

[PATCH] D36353: Instantiate constexpr function when it is used

2017-08-05 Thread Serge Pavlov via Phabricator via cfe-commits 
sepavloff created this revision.

Previous behavior was to instantiate constexpr function always, even if
it is used in the body of a template. For instance, in the following code:

  template constexpr T foo(T x) { return x; }
  template T bar(T x) { return foo(0); }

the function template specialization foo was instantiated because it
was used in the body of template bar. It is a violation of C++ Standard
requirement that an implementation shall not implicitly instantiate a
function template unless such instantiation is required ([temp.inst]p8).
This behavior is different from the case of non-constexpr function, which
indeed are instantiated when needed.

With this change constexpr functions are instantiated in the same way as
non-constexpr functions - when they are actually used. To make it possible
constant evaluator must be able to instantiate function definition. It is
provided by ASTContext, which in turn make the instantiation with help of
Sema.

This change fixes PR33561.


https://reviews.llvm.org/D36353

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprConstant.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaTemplate/instantiate-constexpr-function.cpp

Index: test/SemaTemplate/instantiate-constexpr-function.cpp
===
--- /dev/null
+++ test/SemaTemplate/instantiate-constexpr-function.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++14 -verify %s
+// expected-no-diagnostics
+
+template constexpr void func_01(T) {
+  func_01(0);
+}
+
+
+template constexpr unsigned func_02a() {
+  return sizeof(T);
+}
+template constexpr T func_02b(T x) {
+  return x + func_02a();
+}
+constexpr long val_02 = func_02b(14L);
+
+
+template constexpr T func_03(T) {
+  return T::xyz;
+}
+template T func_04(T x) {
+  return x;
+}
+template<> constexpr long func_04(long x) {
+  return 66;
+}
+constexpr long var_04 = func_04(0L);
+static_assert(var_04 == 66, "error");
+
+
+template struct C_05 {
+  constexpr T func_05() { return T::xyz; }
+};
+C_05 var_05;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13629,17 +13629,12 @@
   }
 }
 
-if (!AlreadyInstantiated || Func->isConstexpr()) {
+if (!AlreadyInstantiated) {
   if (isa(Func->getDeclContext()) &&
   cast(Func->getDeclContext())->isLocalClass() &&
   CodeSynthesisContexts.size())
 PendingLocalImplicitInstantiations.push_back(
 std::make_pair(Func, PointOfInstantiation));
-  else if (Func->isConstexpr())
-// Do not defer instantiations of constexpr functions, to avoid the
-// expression evaluator needing to call back into Sema if it sees a
-// call to such a function.
-InstantiateFunctionDefinition(PointOfInstantiation, Func);
   else {
 Func->setInstantiationIsPending(true);
 PendingInstantiations.push_back(std::make_pair(Func,
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -113,6 +113,20 @@
 } // end namespace sema
 } // end namespace clang
 
+namespace {
+class SemaTemplateInstantiator : public ASTContext::InstantiationHelper {
+  Sema 
+public:
+  ~SemaTemplateInstantiator() {}
+  SemaTemplateInstantiator(Sema ) : S(S) {}
+  bool instantiateFunctionDefinition(SourceLocation PointOfInstantiation,
+ FunctionDecl *Function) override {
+S.InstantiateFunctionDefinition(PointOfInstantiation, Function);
+return Function->hasBody();
+  }
+};
+}
+
 Sema::Sema(Preprocessor , ASTContext , ASTConsumer ,
TranslationUnitKind TUKind, CodeCompleteConsumer *CodeCompleter)
 : ExternalSource(nullptr), isMultiplexExternalSource(false),
@@ -171,6 +185,8 @@
   SemaPPCallbackHandler = Callbacks.get();
   PP.addPPCallbacks(std::move(Callbacks));
   SemaPPCallbackHandler->set(*this);
+
+  Context.setInstantiator(new SemaTemplateInstantiator(*this));
 }
 
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
@@ -359,6 +375,9 @@
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
 
+  ASTContext::InstantiationHelper *Inst = Context.removeInstantiator();
+  delete Inst;
+
   assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -2564,6 +2564,30 @@
   return false;
 }
 
+/// Returns body of the function described by the specified declaration,
+/// instantiating its definition if necessary.
+///
+/// \param[in]  FD The function which body is retrieved.
+/// \param[in]  Loc Location that becomes a point of instantiation.
+/// \param[out] Definition The redeclaration that provides the body.
+///
+/// Behaves like FinctionDecl::getBody, but also can

[PATCH] D36315: [mips] Support implicit gpopt with N64 when using -fno-pic

2017-08-05 Thread Simon Atanasyan via Phabricator via cfe-commits 
atanasyan accepted this revision.
atanasyan added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D36315



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