[PATCH] D77116: temp tests

2020-03-30 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77116

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/Inputs/dummy_pragma_once.h
  clang/test/Modules/Inputs/dummy_textual_header.h
  clang/test/Modules/Inputs/header_in_imported_module.h
  clang/test/Modules/Inputs/imported_module.cppm
  clang/test/Modules/Inputs/module.map
  clang/test/Modules/header-in-imported-module.c
  clang/test/Modules/import-pragma-once.c

Index: clang/test/Modules/import-pragma-once.c
===
--- /dev/null
+++ clang/test/Modules/import-pragma-once.c
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x c %s
+// expected-no-diagnostics
+#include "dummy_pragma_once.h"
+#include "dummy_textual_header.h"
+
+void *p = 
+void *q = 
Index: clang/test/Modules/header-in-imported-module.c
===
--- /dev/null
+++ clang/test/Modules/header-in-imported-module.c
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-ts --precompile -o %t/ModuleB39206.pcm %S/Inputs/imported_module.cppm
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-ts -c %t/ModuleB39206.pcm -o %t/ModuleB39206.o
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-ts -fmodule-file=%t/ModuleB39206.pcm -verify  -c %s
+// expected-no-diagnostics
+
+// Bug 39206
+
+#include "header_in_imported_module.h"
+module ModuleB39206;
+
+#ifndef ALL_GOOD
+#error "missing macro in impl"
+#endif
Index: clang/test/Modules/Inputs/module.map
===
--- clang/test/Modules/Inputs/module.map
+++ clang/test/Modules/Inputs/module.map
@@ -282,6 +282,11 @@
   header "dummy.h"
 }
 
+module dummy_pragma_once {
+  header "dummy_pragma_once.h"
+  textual header "dummy_textual_header.h"
+}
+
 module builtin {
   header "builtin.h"
   explicit module sub {
Index: clang/test/Modules/Inputs/imported_module.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/imported_module.cppm
@@ -0,0 +1,5 @@
+export module ModuleB39206;
+#include "header.h"
+
+#ifndef ALL_GOOD
+#error "Missing macro in module decl"
\ No newline at end of file
Index: clang/test/Modules/Inputs/header_in_imported_module.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/header_in_imported_module.h
@@ -0,0 +1,3 @@
+#pragma once
+
+#define ALL_GOOD
Index: clang/test/Modules/Inputs/dummy_textual_header.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_textual_header.h
@@ -0,0 +1,2 @@
+#pragma once
+int y = 6;
Index: clang/test/Modules/Inputs/dummy_pragma_once.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_pragma_once.h
@@ -0,0 +1,3 @@
+#include "dummy_textual_header.h"
+
+int x = 5;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1621,7 +1621,7 @@
   endian::Writer LE(Out, little);
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
   LE.write(KeyLen);
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 2 + 4 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1678,6 +1678,9 @@
   }
   LE.write(Offset);
 
+  // Write this file UID.
+  LE.write(Data.HFI.UID);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1705,7 +1708,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch ) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch ) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator Generator;
   SmallVector SavedStrings;
@@ -1783,8 +1786,7 @@
 // changed since it was loaded. Also skip it if it's for a modular header
 // from a different module; in 

[PATCH] D77115: [WebAssembly] Emit .llvmcmd and .llvmbc as custom sections

2020-03-30 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 created this revision.
Herald added subscribers: cfe-commits, jfb, sunfish, aheejin, hiraditya, 
jgravelle-google, dschuff.
Herald added a project: clang.
sbc100 added reviewers: alexcrichton, sunfish.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77115

Files:
  clang/test/Driver/embed-bitcode-wasm.c
  clang/test/Driver/fembed-bitcode.c
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/MC/WasmObjectWriter.cpp


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -436,10 +436,6 @@
   uint64_t FixupOffset = Layout.getFragmentOffset(Fragment) + 
Fixup.getOffset();
   MCContext  = Asm.getContext();
 
-  // The .init_array isn't translated as data, so don't do relocations in it.
-  if (FixupSection.getSectionName().startswith(".init_array"))
-return;
-
   if (const MCSymbolRefExpr *RefB = Target.getSymB()) {
 // To get here the A - B expression must have failed evaluateAsRelocatable.
 // This means either A or B must be undefined and in WebAssembly we can't
@@ -502,6 +498,10 @@
 SymA->setUsedInReloc();
   }
 
+  // The .init_array isn't translated as data, so don't do relocations in it.
+  if (FixupSection.getSectionName().startswith(".init_array"))
+return;
+
   if (RefA->getKind() == MCSymbolRefExpr::VK_GOT)
 SymA->setUsedInGOT();
 
@@ -1090,10 +1090,7 @@
   if (Sym.isComdat() && !Sym.isDefined())
 return false;
 
-  if (Sym.isTemporary() && Sym.getName().empty())
-return false;
-
-  if (Sym.isTemporary() && Sym.isData() && !Sym.getSize())
+  if (Sym.isTemporary())
 return false;
 
   if (Sym.isSection())
@@ -1565,7 +1562,7 @@
 report_fatal_error("fixups in .init_array should be symbol 
references");
   const auto  = cast(SymRef->getSymbol());
   if (TargetSym.getIndex() == InvalidIndex)
-report_fatal_error("symbols in .init_array should exist in symbtab");
+report_fatal_error("symbols in .init_array should exist in symtab");
   if (!TargetSym.isFunction())
 report_fatal_error("symbols in .init_array should be for functions");
   InitFuncs.push_back(
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1765,6 +1765,14 @@
 
   StringRef Name = GO->getSection();
 
+  // Certain data sections we treat as custom named section rather than
+  // segments within the data section.
+  // This could be avoided if all data segements in (the wasm sense) were
+  // represented as thier on sections (in the llvm sense).
+  // TODO(sbc): https://github.com/WebAssembly/tool-conventions/issues/138
+  if (Name == ".llvmcmd" || Name == ".llvmbc")
+Kind = SectionKind::getMetadata();
+
   StringRef Group = "";
   if (const Comdat *C = getWasmComdat(GO)) {
 Group = C->getName();
Index: clang/test/Driver/fembed-bitcode.c
===
--- clang/test/Driver/fembed-bitcode.c
+++ clang/test/Driver/fembed-bitcode.c
@@ -30,3 +30,12 @@
 // RUN: | FileCheck --check-prefix=CHECK-HEXAGON %s
 // CHECK-HEXAGON: "-target-feature"
 // CHECK-HEXAGON: "+reserved-r19"
+//
+// RUN: %clang -target wasm32-unknown-unknown -fembed-bitcode=all -pthread -c 
%s -o /dev/null -### 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-WASM %s
+
+// CHECK-WASM: "-cc1"
+// CHECK-WASM: "-target-feature" "+atomics"
+
+// CHECK-WASM: "-cc1"
+// CHECK-WASM: "-target-feature" "+atomics"
Index: clang/test/Driver/embed-bitcode-wasm.c
===
--- /dev/null
+++ clang/test/Driver/embed-bitcode-wasm.c
@@ -0,0 +1,6 @@
+// REQUIRES: webassembly-registered-target
+
+// RUN: %clang -c -target wasm32-unknown-unknown %s -fembed-bitcode -o %t.o
+// RUN: llvm-readobj -S %t.o | FileCheck --check-prefix=CHECK %s
+// CHECK:   Name: .llvmbc
+// CHECK:   Name: .llvmcmd


Index: llvm/lib/MC/WasmObjectWriter.cpp
===
--- llvm/lib/MC/WasmObjectWriter.cpp
+++ llvm/lib/MC/WasmObjectWriter.cpp
@@ -436,10 +436,6 @@
   uint64_t FixupOffset = Layout.getFragmentOffset(Fragment) + Fixup.getOffset();
   MCContext  = Asm.getContext();
 
-  // The .init_array isn't translated as data, so don't do relocations in it.
-  if (FixupSection.getSectionName().startswith(".init_array"))
-return;
-
   if (const MCSymbolRefExpr *RefB = Target.getSymB()) {
 // To get here the A - B expression must have failed evaluateAsRelocatable.
 // This means either A or B must be undefined and in WebAssembly we can't
@@ -502,6 +498,10 @@
 SymA->setUsedInReloc();
   }
 
+  // The .init_array isn't translated as data, so don't do 

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

2020-03-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:12248
+  ///   SYCLDiagIfDeviceCode(Loc, diag::err_type_unsupported) << "__float128";
+  DeviceDiagBuilder SYCLDiagIfDeviceCode(SourceLocation Loc, unsigned DiagID);
+

Fznamznon wrote:
> rjmccall wrote:
> > Will this collect notes associated with the diagnostic correctly?
> Could you please make your question a bit more concrete?
> This function is supposed to work in the same way as 
> `Sema::CUDADiagIfDeviceCode` and `Sema::diagIfOpenMPDeviceCode` . It emits 
> given diagnostic if the current context is known as "device code" and makes 
> this diagnostic deferred otherwise. It uses the `DeviceDiagBuilder` which was 
> implemented earlier. This `DeviceDiagBuilder` also tries to emit callstack 
> notes for the given diagnostics. Do you mean these callstack notes or 
> something else?
Logically, notes that are emitted after a warning or error are considered to be 
part of that diagnostic.  A custom `DiagBuilder` that only redirects the main 
diagnostic but allows the notes to still be emitted will effectively cause 
those notes to misleadingly follow whatever previous diagnostic might have been 
emitted.

I call this out specifically because some of the places where you're using this 
still seem to try to emit notes afterwards, at least in some cases.  It's 
possible that `CUDADiagIfDeviceCode` happens to not be used in such a way.  
Really I'm not sure this conditional `DiagBuilder` approach was a good idea the 
first time, and I think we should probably reconsider rather than duplicating 
it.



Comment at: clang/lib/Sema/SemaAvailability.cpp:479
+case UnavailableAttr::IR_SYCLForbiddenType:
+  diag_available_here = diag::err_type_unsupported;
+  break;

Fznamznon wrote:
> rjmccall wrote:
> > All of the other cases are setting this to a note, not an error, so I 
> > suspect this will read wrong.
> Yes, this is not a note. For such samples:
> 
> ```
> int main() {
>   __float128 CapturedToDevice = 1;
>   kernel([=]() {
> decltype(CapturedToDevice) D;
>   });
> }
> ```
> It looks like this:
> ```
> float128.cpp:63:14: error: 'CapturedToDevice' is unavailable
> decltype(CapturedToDevice) D;
>  ^
> float128.cpp:59:14: error: '__float128' is not supported on this target   /// 
> This emitted instead of note 
>   __float128 CapturedToDevice = 1;
>  ^
> ```
> I had feeling that it should probably be a note. But there is no implemented 
> note for unsupported types. I think I can add a new one if it will make it 
> better. Should I?
Yeah, this should be a note, like "note: variable is unavailable because it 
uses a type '__float128' that is not supported on this target".  You should add 
that.



Comment at: clang/lib/Sema/SemaAvailability.cpp:534
+if (S.getLangOpts().SYCLIsDevice)
+  S.SYCLDiagIfDeviceCode(Loc, diag) << ReferringDecl;
+else

Fznamznon wrote:
> rjmccall wrote:
> > Are you sure you want to be applying this to all of the possible 
> > diagnostics here, rather than just for SYCLForbiddenType unavailable 
> > attributes?
> I suppose it is reasonable if we want to reuse unavaliable attribute for 
> other SYCL use cases. Plus, In SYCL we don't know where is device code until 
> we instantiate templates, it happens late, so we have to defer any diagnostic 
> while compiling for device, otherwise we can point to host code where much 
> more is allowed.
My point is actually the reverse of that.  This code path is also used for 
normal `unavailable` attributes, not just the special ones you're synthesizing. 
 Diagnostics from the use of explicitly-unavailable declarations shouldn't get 
any special treatment here, no more than you'd give special treatment to a 
diagnostic arising from an attempt to assign a pointer into a `float`.  In the 
logic above where you recognize `IR_SYCLForbiddenType`, I think you should just 
check whether you should transitively defer the diagnostic and, if so, do so 
and then bail out of this function early.  That might mean you don't need the 
custom DiagBuilder, too.



Comment at: clang/lib/Sema/SemaDecl.cpp:18030
+  if (LangOpts.SYCLIsDevice && FD->hasAttr())
+return FunctionEmissionStatus::Emitted;
+

Fznamznon wrote:
> rjmccall wrote:
> > So you want to emit it for the definition in addition to emitting it for 
> > specific specializations?
> Somehow diagnostics are emitted only for the definitions. 
> Without this change diagnostics aren't emitted at all.
Hmm.  We might be marking the template pattern invalid; that could result in 
all sorts of diagnostics being suppressed.  We definitely shouldn't be marking 
things invalid without emitting an eager diagnostic.


Repository:
  rG LLVM Github Monorepo

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


[PATCH] D77028: [NFC] Refactor DeferredDiagsEmitter and skip redundant visit

2020-03-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I made a suggestion in the other patch about restructuring things out of 
`visitUsedDecl`.  I'll review this when that's done.


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

https://reviews.llvm.org/D77028



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


[PATCH] D76937: Fix infinite recursion in deferred diagnostic emitter

2020-03-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D76937#1950764 , @yaxunl wrote:

> In D76937#1950077 , @rjmccall wrote:
>
> > Can you explain what exactly the emission/semantic model is for variables?  
> > Normal code-generation absolutely triggers the emission of many variables 
> > lazily (e.g. internal-linkage globals, C++ inline variables); and any 
> > variable that's *not* being emitted lazily actually needs to be treated as 
> > a potential root into the delayed-diagnostic graph.
>
>
> Currently only in the following case a global variable can change the 
> emission state of a function:
>
>   int foobar3() { return 1; }
>   #pragma omp declare target
>   int (*C)() = 
>   #pragma omp end declare target
>
>
> The global variable needs to be in a omp declare target directive. Its 
> initializer references a host function.
>
> This will cause the function emitted in device compilation.
>
> This is not transitive for variable declaration/references, e.g. the 
> following case will not cause foobar3 to be emitted in device compilation, 
> unless variable C itself is enclosed in omp declare target directive.
>
>   int foobar3() { return 1; }
>   int (*C)() = 
>   #pragma omp declare target
>   int (*D)() = C;
>   #pragma omp end declare target
>
>
> Since we logged all such variable declarations in 
> DeclsToCheckForDeferredDiags, we only need to check variables decls in 
> DeclsToCheckForDeferredDiags and do not need to check variable decls in 
> DeclRefExpr.


Okay.  There are other AST nodes besides `DeclRefExpr` that can cause an 
ODR-use of a `VarDecl`; you could add special cases for them, too, but part of 
the point of this design is so that you can stop worrying about that kind of 
thing.  I think I've said this before, but you're making things harder for 
yourself by re-using `visitUsedDecl` as both the "callback" of the 
`UsedDeclVisitor` and the original method that you call for things from 
`DeclsToCheckForDeferredDiags`.  Just use `visitUsedDecl` for the former, make 
a new method for the latter, and extract any common logic you want (e.g. for 
`FunctionDecl`s) into helper methods that both of them can use.  That way you 
can just unconditionally ignore non-functions in `visitUsedDecl`, assert that 
`DeclsToCheckForDeferredDiags` only contains the kinds of declarations you 
think it should, and so on.


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

https://reviews.llvm.org/D76937



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


[PATCH] D77077: [clang] CodeGen: Make getOrEmitProtocol public for Swift

2020-03-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:148
+ llvm::function_ref
+ createProtocolReference);
 }  // end namespace CodeGen

aschwaighofer wrote:
> rjmccall wrote:
> > I would call this `emitObjCProtocolObject` or something, and maybe say in 
> > the comment:
> > 
> > > Get a pointer to a protocol object for the given declaration, emitting it 
> > > if it hasn't already been emitted in this translation unit.  Note that 
> > > the ABI for emitting a protocol reference in code (e.g. for a `@protocol` 
> > > expression) in most runtimes is not as simple as just materializing a 
> > > pointer to this object.
> > 
> > Can you explain the need for the callback?   Are you expecting to use this 
> > for Swift-declared protocols by synthesizing an ObjC protocol declaration 
> > for them?  I can see why you'd need a callback in that case.
> > Can you explain the need for the callback? Are you expecting to use this 
> > for Swift-declared protocols by synthesizing an ObjC protocol declaration 
> > for them? I can see why you'd need a callback in that case.
> 
> The objective C protocol references other protocols in its inherited list. 
> Swift has an internal list that keeps track of those protocols references and 
> makes sure to initialized them for the debugger and also makes sure that the 
> underlying protocol structure is emitted. The call back is to keep this list 
> in sync.
The problem is that this is really, really intertwined with the ABI logic.  
This callback basically has to know exactly how the caller is calling it and 
and what it's expected to build in response.

Swift code can use arbitrary inline Objective-C functions and therefore emit 
arbitrary Objective-C code.  Is there an approach that handles that while also 
being less invasive for Clang IRGen?  Can we, e.g., inspect the built 
llvm::Module to figure out retroactively what protocol refs we need to update 
dynamically instead of trying to track them during the build?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77077



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


[PATCH] D73521: [analyzer] add-new-checker.py: Introduction

2020-03-30 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 253777.
Charusso marked 3 inline comments as done.
Charusso added a comment.
Herald added a subscriber: ASDenysPetrov.

- Remove the test of creating a live checker, instead copy over the live 
checker when the script runs.
- Simplify the script by adding the new package to the end of the file.
- In case of the `checkers.rst` a non-alpha package is going to be added before 
the alpha packages.
- According to this change simplify the tests.
- `DummyChecker` -> `ExampleChecker`.


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

https://reviews.llvm.org/D73521

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ExampleChecker.cpp
  clang/test/Analysis/add-new-checker/add-main-package.rst
  clang/test/Analysis/add-new-checker/add-main-package.td
  clang/test/Analysis/add-new-checker/add-multiple-packages.rst
  clang/test/Analysis/add-new-checker/add-multiple-packages.td
  clang/test/Analysis/add-new-checker/check-add-new-checker.py
  clang/test/Analysis/add-new-checker/checker-name.rst
  clang/test/Analysis/add-new-checker/checker-name.td
  clang/test/Analysis/add-new-checker/flow-package-exist-checker.rst
  clang/test/Analysis/add-new-checker/flow-package-exist-checker.td
  clang/test/Analysis/add-new-checker/flow-package-not-exist.rst
  clang/test/Analysis/add-new-checker/flow-package-not-exist.td
  clang/test/Analysis/add-new-checker/lit.local.cfg.py
  clang/test/Analysis/example-checker.cpp
  clang/utils/analyzer/add-new-checker.py

Index: clang/utils/analyzer/add-new-checker.py
===
--- /dev/null
+++ clang/utils/analyzer/add-new-checker.py
@@ -0,0 +1,637 @@
+#!/usr/bin/env python
+#
+#===- add-new-checker.py - Creates a Static Analyzer checker --*- python -*-===#
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#======#
+#
+# Example usage: python add-new-checker.py alpha.package.Foo
+#
+#======#
+
+import argparse
+import json
+import os
+import re
+import subprocess
+
+
+class Checker:
+def __init__(self, packages, name):
+self.packages = packages
+self.name = name
+
+def name(self):
+return self.name
+
+def packages(self):
+return self.packages
+
+def __str__(self):
+return '.'.join(self.packages) + '.' + self.name
+
+
+# Obtain the longest known package-chain of the 'packages' chain. For example
+# the checker creation of 'alpha.core.a.Checker' would return ['alpha', 'core'].
+def get_best_package_match(packages, known_packages):
+best_package_match = []
+for i in range(len(packages)):
+temp_packages = packages[:i + 1]
+package_name = '.'.join(temp_packages)
+if package_name in known_packages:
+best_package_match = temp_packages
+return best_package_match
+
+
+# Put every additional package and the checker into an increasing size jagged
+# array. For example the checker creation of 'core.a.Checker' would return
+# '[['core', 'a'], ['core', 'a', 'Checker']]'.
+def get_addition(packages, name, best_package_match):
+addition = []
+match_count = len(best_package_match)
+for i, package in enumerate(packages):
+if i < match_count:
+continue
+addition.append(best_package_match + packages[match_count : i + 1])
+addition.append(packages + [name])
+return addition
+
+
+def add_checkers_entry(clang_path, checker, args):
+checkers_include_path = os.path.join(clang_path, 
+'include', 'clang', 'StaticAnalyzer', 'Checkers')
+checkers_path = os.path.join(checkers_include_path, 'Checkers.td')
+
+# This only could test '.td'.
+if args.is_test:
+checkers_path = args.test_path
+if not checkers_path.endswith('.td.tmp'):
+return
+
+packages = checker.packages
+name = checker.name
+
+# See if TableGen is found.
+try:
+devnull = open(os.devnull)
+subprocess.Popen([args.tblgen_path, '-h'], stdout=devnull,
+ stderr=devnull).communicate()
+except OSError as e:
+print("[error] llvm-tblgen is not found. Please specify it explicitly. "
+  "For example: '--tblgen-path llvm-tblgen'")
+raise
+
+# Load the checkers' TableGen.
+data = None
+try:
+data = subprocess.check_output([args.tblgen_path, '-dump-json',
+checkers_path,
+'-I=' + checkers_include_path])
+except subprocess.CalledProcessError as e:
+print('TableGen JSON 

[PATCH] D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX

2020-03-30 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 253774.
ZarkoCA added a comment.

Rebased to include byval changes.

Fixed name of array.

Fixed up test case as per comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76130

Files:
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll
  llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll

Index: llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
@@ -0,0 +1,366 @@
+; RUN: llc -O2 -mtriple powerpc-ibm-aix-xcoff -stop-after=machine-cp -verify-machineinstrs < %s | \
+; RUN: FileCheck --check-prefixes=CHECK,32BIT %s
+
+; RUN: llc -O2 -verify-machineinstrs -mcpu=pwr4 -mattr=-altivec \
+; RUN:  -mtriple powerpc-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefixes=CHECKASM,ASM32 %s
+
+  target datalayout = "E-m:e-i64:64-n32:64"
+  target triple = "powerpc64-ibm-aix-xcoff"
+  
+  define i32 @int_va_arg(i32 %a, ...) local_unnamed_addr  {
+  entry:
+%arg1 = alloca i8*, align 8
+%arg2 = alloca i8*, align 8
+%0 = bitcast i8** %arg1 to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0) 
+%1 = bitcast i8** %arg2 to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %1)
+call void @llvm.va_start(i8* nonnull %0)
+call void @llvm.va_copy(i8* nonnull %1, i8* nonnull %0)
+%2 = va_arg i8** %arg1, i32
+%add = add nsw i32 %2, %a
+%3 = va_arg i8** %arg2, i32
+%mul = shl i32 %3, 1
+%add3 = add nsw i32 %add, %mul
+call void @llvm.va_end(i8* nonnull %0)
+call void @llvm.va_end(i8* nonnull %1)
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %1) 
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0) 
+ret i32 %add3
+  }
+
+  declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) 
+  declare void @llvm.va_start(i8*) 
+  declare void @llvm.va_copy(i8*, i8*) 
+  declare void @llvm.va_end(i8*) 
+  declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) 
+  
+; 64BIT-LABEL:   name:int_va_arg
+; 64BIT-LABEL:   liveins:
+; 64BIT-DAG: - { reg: '$x3', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x4', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x5', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x6', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x7', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x8', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x9', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x10', virtual-reg: '' }
+
+; 64BIT-LABEL:   fixedStack:
+; 64BIT-DAG: - { id: 0, type: default, offset: 56, size: 8
+
+; 64BIT-LABEL:   stack:
+; 64BIT-DAG: - { id: 0, name: arg1, type: default, offset: 0, size: 8
+; 64BIT-DAG: - { id: 1, name: arg2, type: default, offset: 0, size: 8 
+
+; 64BIT-LABEL:   body: |
+; 64BIT-DAG: liveins: $x3, $x4, $x5, $x6, $x7, $x8, $x9, $x10
+; 64BIT-DAG: renamable $x4 = ADDI8 %fixed-stack.0, 0
+; 64BIT-DAG: STD killed renamable $x4, 0, %fixed-stack.0 :: (store 8 into %fixed-stack.0)
+; 64BIT-DAG: STD killed renamable $x5, 8, %fixed-stack.0 :: (store 8 into %fixed-stack.0 + 8)
+; 64BIT-DAG: STD killed renamable $x6, 16, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x7, 24, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x8, 32, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x9, 40, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x10, 48, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: renamable $r11 = LWZ 0, %fixed-stack.0 :: (load 4 from %fixed-stack.0, align 8)
+; 64BIT-DAG: STD renamable $x4, 0, %stack.0.arg1 :: (store 8 into %ir.0)
+; 64BIT-DAG: STD killed renamable $x4, 0, %stack.1.arg2 :: (store 8 into %ir.1)
+; 64BIT-DAG: renamable $x4 = LD 0, %stack.1.arg2 :: (load 8 from %ir.arg2)
+; 64BIT-DAG: renamable $x5 = ADDI8 %fixed-stack.0, 4
+; 64BIT-DAG: STD killed renamable $x5, 0, %stack.0.arg1 :: (store 8 into %ir.arg1)
+; 64BIT-DAG: renamable $x5 = ADDI8 renamable $x4, 4
+; 64BIT-DAG: STD killed renamable $x5, 0, %stack.1.arg2 :: (store 8 into %ir.arg2)
+; 64BIT-DAG: renamable $r4 = LWZ 0, killed renamable $x4 :: (load 4)
+; 64BIT-DAG: renamable $r3 = nsw ADD4 killed renamable $r11, renamable $r3, implicit killed $x3
+; 64BIT-DAG: renamable $r4 = RLWINM killed renamable $r4, 1, 0, 30
+; 64BIT-DAG: renamable $r3 = nsw ADD4 killed renamable $r3, killed renamable $r4, implicit-def $x3
+; 64BIT-DAG: BLR8 implicit $lr8, implicit $rm, implicit $x3
+
+; ASM64-LABEL:   .int_va_arg:
+; ASM64-LABEL:   std 4, 56(1)
+; ASM64-LABEL:   std 5, 64(1)
+; ASM64-LABEL:   std 6, 72(1)
+; ASM64-LABEL:   std 7, 80(1)
+; ASM64-LABEL:   std 8, 88(1)
+; ASM64-LABEL:   std 9, 96(1)
+; ASM64-LABEL:   std 10, 104(1)
+; 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-30 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 253776.
ZarkoCA added a comment.

Fixed test cases to use builtins again, set no soft float abi for AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/aix32-dwarf-error.c

Index: clang/test/CodeGen/aix32-dwarf-error.c
===
--- /dev/null
+++ clang/test/CodeGen/aix32-dwarf-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -emit-llvm %s
+
+static unsigned char dwarf_reg_size_table[1024];
+
+int test() {
+  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table); //expected-error {{cannot compile this __builtin_init_dwarf_reg_size_table yet}}
+  return __builtin_dwarf_sp_column();
+}
Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,39 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+
+void aix_varg(int a, ...) {
+  __builtin_va_list arg;
+  __builtin_va_start(arg, a);
+  __builtin_va_list arg2;
+  __builtin_va_arg(arg, int);
+  __builtin_va_copy(arg2, arg);
+  __builtin_va_end(arg);
+  __builtin_va_end(arg2);
+}
+
+  // 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+  // 32BIT-NEXT:  entry:
+  // 32BIT-NEXT:%a.addr = alloca i32, align 4
+  // 32BIT-NEXT:%arg = alloca i8*, align 4
+  // 32BIT-NEXT:%arg2 = alloca i8*, align 4
+  // 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+  // 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+  // 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+  // 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+  // 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+  // 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+  // 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+  // 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:%3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_copy(i8* %2, i8* %3)
+  // 32BIT-NEXT:%arg3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg3)
+  // 32BIT-NEXT:%arg24 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg24)
+  // 32BIT-NEXT:ret void
+  // 32BIT-NEXT:  }
+  // 32BIT: declare void @llvm.va_start(i8*) #1
+  // 32BIT: declare void @llvm.va_copy(i8*, i8*) #1
+  // 32BIT: declare void @llvm.va_end(i8*) #1
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4173,14 +4173,15 @@
 
 // PowerPC-32
 namespace {
-/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
-class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+/// PowerPC32ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF
+/// (SVR4), Darwin and AIX.
+class PowerPC32ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
 
   CharUnits getParamTypeAlignment(QualType Ty) const;
 
 public:
-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
+  PowerPC32ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI)
   : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
 
   Address EmitVAArg(CodeGenFunction , Address VAListAddr,
@@ -4190,7 +4191,7 @@
 class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   PPC32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
-  : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 // This is recovered from gcc output.
@@ -4200,9 +4201,22 @@
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
llvm::Value *Address) const override;
 };
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  PPCAIX32TargetCodeGenInfo(CodeGenTypes , bool SoftFloatABI)
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
+
+  int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
+return 1; // r1 is the dedicated stack pointer
+  }
+
+  bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction ,
+   llvm::Value *Address) const override;
+};
 }
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements
   if (const ComplexType *CTy = 

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-30 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked 4 inline comments as done.
ZarkoCA added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10019
+  return SetCGInfo(
+  new PPCAIX32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == 
"soft"));
 return SetCGInfo(

jasonliu wrote:
> Does AIX have soft Float? If not, do we want to always pass in 'false'? 
Thanks, missed changing this.  I set it to hard.



Comment at: clang/test/CodeGen/aix-vararg.c:4
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s --check-prefix=32BIT
+#include 
+

jasonliu wrote:
> Any reason we don't use __builtin_va... any more?
My mistake, I somehow included the old version of the test in the diff. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



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


[PATCH] D73521: [analyzer] add-new-checker.py: Introduction

2020-03-30 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment.

In D73521#1923693 , @NoQ wrote:

> Or is each test run updating the repo? Can we simply make the script do `cat 
> DummyChecker.cpp | sed s/DummyChecker/$NAME/g > $NAME.cpp` and store the 
> checker only once?


It was updating, yes. The first idea was to copy over the live checker, but I 
wanted to make it as portable as possible.




Comment at: clang/utils/analyzer/add-new-checker.py:714-715
+help='the path to llvm-tblgen')
+parser.add_argument('--force-creation', dest='is_force_creation',
+action='store_true', help=argparse.SUPPRESS)
+parser.add_argument('--test', dest='is_test',

NoQ wrote:
> So, `--overwrite` and add help?
It was only made for the test. Removed.


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

https://reviews.llvm.org/D73521



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


[PATCH] D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX

2020-03-30 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA marked 2 inline comments as done.
ZarkoCA added inline comments.



Comment at: llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll:17
+call void @llvm.va_start(i8* nonnull %0)
+call void @llvm.va_copy(i8* nonnull %0, i8* nonnull %0)
+%argp.cur = load i8*, i8** %arg, align 4

jasonliu wrote:
> I think it would be more clear if we actually have another va_list to copy 
> instead of just va_copying itself. The same applies to all the places we call 
> va_copy. 
> Also it's not very clear to me which part in the check result actually tells 
> me about the effect of the va_copy though.
Those are both good points. I'm rewriting the test and adding comments to the 
IR/assembly to make it clearer. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76130



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


[PATCH] D77113: [OpenMP][NFC] Move and simplify directive -> allowed clause mapping

2020-03-30 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert created this revision.
jdoerfert added reviewers: lebedev.ri, JonChesterfield, ABataev.
Herald added subscribers: jfb, guansong, bollu, hiraditya.
Herald added a project: clang.
jdoerfert added a reviewer: fghanim.

Move the listing of allowed clauses per OpenMP directive to the new
macro file in `llvm/Frontend/OpenMP`. Also, use a single generic macro
that specifies the directive and one allowed clause explicitly instead
of a dedicated macro per directive.

We save 800 loc and boilerplate for all new directives/clauses with no
functional change. We also need to include the macro file only once and
not once per directive.

Depends on D77112 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77113

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/lib/Basic/OpenMPKinds.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp

Index: llvm/lib/Frontend/OpenMP/OMPConstants.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPConstants.cpp
+++ llvm/lib/Frontend/OpenMP/OMPConstants.cpp
@@ -54,6 +54,17 @@
   llvm_unreachable("Invalid OpenMP clause kind");
 }
 
+bool llvm::omp::isAllowedClauseForDirective(Directive D, Clause C,
+unsigned Version) {
+  assert(unsigned(D) <= unsigned(OMPD_unknown));
+  assert(unsigned(C) <= unsigned(OMPC_unknown));
+#define OMP_DIRECTIVE_CLAUSE(Dir, MinVersion, MaxVersion, Cl)  \
+  if (D == Dir && C == Cl && MinVersion <= Version && MaxVersion >= Version)   \
+return true;
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  return false;
+}
+
 /// Declarations for LLVM-IR types (simple, array, function and structure) are
 /// generated below. Their names are defined and used in OpenMPKinds.def. Here
 /// we provide the declarations, the initializeTypes function will provide the
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -664,3 +664,732 @@
 #undef __OMP_REQUIRES_TRAIT
 #undef OMP_REQUIRES_TRAIT
 ///}
+
+
+/// Clauses allowed per directive
+///
+///{
+
+#ifndef OMP_DIRECTIVE_CLAUSE
+#define OMP_DIRECTIVE_CLAUSE(Directive, MinVersion, MaxVersion, Clause)
+#endif
+
+#define __OMP_DIRECTIVE_CLAUSE(Directive, MinVersion, MaxVersion, Clause)  \
+  OMP_DIRECTIVE_CLAUSE(OMPD_##Directive, unsigned(MinVersion), \
+   unsigned(MaxVersion), OMPC_##Clause)
+
+__OMP_DIRECTIVE_CLAUSE(scan, 50, ~0, inclusive)
+__OMP_DIRECTIVE_CLAUSE(scan, 50, ~0, exclusive)
+
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, if)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, num_threads)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, default)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, proc_bind)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, private)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, firstprivate)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, shared)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, reduction)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, copyin)
+__OMP_DIRECTIVE_CLAUSE(parallel, 1, ~0, allocate)
+
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, private)
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, lastprivate)
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, linear)
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, aligned)
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, safelen)
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, simdlen)
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, collapse)
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, reduction)
+__OMP_DIRECTIVE_CLAUSE(simd, 1, ~0, allocate)
+__OMP_DIRECTIVE_CLAUSE(simd, 50, ~0, if)
+__OMP_DIRECTIVE_CLAUSE(simd, 50, ~0, nontemporal)
+__OMP_DIRECTIVE_CLAUSE(simd, 50, ~0, order)
+
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, private)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, lastprivate)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, firstprivate)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, reduction)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, collapse)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, schedule)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, ordered)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, nowait)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, linear)
+__OMP_DIRECTIVE_CLAUSE(for, 1, ~0, allocate)
+__OMP_DIRECTIVE_CLAUSE(for, 50, ~0, order)
+
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, private)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, firstprivate)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, lastprivate)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, reduction)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, schedule)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, collapse)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, nowait)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, safelen)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, simdlen)
+__OMP_DIRECTIVE_CLAUSE(for_simd, 1, ~0, linear)

[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

2020-03-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked an inline comment as done.
rsmith added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:16008-16013
+// In the MS ABI, the complete destructor is implicitly defined,
+// even if the base destructor is user defined.
+CXXRecordDecl *Parent = Destructor->getParent();
+if (Parent->getNumVBases() > 0 &&
+!Parent->definedImplicitCompleteDestructor())
+  DefineImplicitCompleteDestructor(Loc, Destructor);

rnk wrote:
> rsmith wrote:
> > Can we avoid doing this when we know the call is to a base subobject 
> > destructor or uses virtual dispatch? Should we? (I mean I guess ideally 
> > we'd change this function to take a `GlobalDecl` instead of a 
> > `FunctionDecl*`, but that seems like a lot of work.)
> > 
> > How does MSVC behave?
> I think we can skip this if virtual dispatch is used, but I'm not sure what 
> kinds of devirtualization might break my assumptions.
> 
> For example, uncommenting `final` here causes MSVC to diagnose the error, but 
> it otherwise it compiles:
> ```
> struct NoDtor { ~NoDtor() = delete; };
> struct DefaultedDtor : NoDtor { ~DefaultedDtor() = default; };
> struct HasVBases /*final*/ : virtual DefaultedDtor {
>   virtual ~HasVBases();
> };
> void deleteIt1(HasVBases *p) { delete p; }
> ```
> 
> If the NoDtor destructor is undeleted and the class is made final, then both 
> deleting and complete dtors are emitted for HasVBases. Clang would need to 
> mark DefaultedDtor referenced in this case.
> 
> I think it's OK to "overdiagnose" in this case. It's not possible to emit a 
> vtable here without diagnosing the error.
Great, if we're happy to treat all uses of the destructor as using the complete 
object destructor, then doing this work as part of marking the destructor used 
seems like it would be the best solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77081



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


Re: [clang] 499b2a8 - PR45294: Fix handling of assumed template names looked up in the lexical

2020-03-30 Thread Richard Smith via cfe-commits
Already fixed in llvmorg-11-init-7209-g33087323007 :)

On Mon, 30 Mar 2020 at 18:30, Hubert Tong via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> I have not found which of the few commits on Friday having to do with
> template-ids is responsible, but this now produces a crash (trace is below):
> struct A;
> struct B : A {};
>
> Richard, would you take a look?
>
> Thanks,
>
>
> Hubert Tong
>
> :2:12: error: unknown template name 'A'
> struct B : A {};
>^
> PLEASE submit a bug report to https://bugs.llvm.org/ and include the
> crash backtrace, preprocessed source, and associated run script.
> Stack dump:
> 0.  Program arguments: /build/bin/clang++ -cc1 -xc++ -
> 1.  :2:19: current parser token '{'
> 2.  :2:1: parsing struct/union/class body 'B'
>  #0 0x3fffb2016418 llvm::sys::PrintStackTrace(llvm::raw_ostream&)
> (/build/bin/../lib/libLLVMSupport.so.11git+0x206418)
>  #1 0x3fffb2016540 PrintStackTraceSignalHandler(void*)
> (/build/bin/../lib/libLLVMSupport.so.11git+0x206540)
>  #2 0x3fffb2013b38 llvm::sys::RunSignalHandlers()
> (/build/bin/../lib/libLLVMSupport.so.11git+0x203b38)
>  #3 0x3fffb2013d2c SignalHandler(int)
> (/build/bin/../lib/libLLVMSupport.so.11git+0x203d2c)
>  #4 0x3fffb4570478  0x478
> clang::Sema::ActOnBaseSpecifier(clang::Decl*, clang::SourceRange,
> clang::ParsedAttributes&, bool, clang::AccessSpecifier,
> clang::OpaquePtr, clang::SourceLocation,
> clang::SourceLocation)
>  #5 0x3fffb4570478
>  #6 0x3fffb4570478 clang::Parser::ParseBaseSpecifier(clang::Decl*)
> (+0x478)
>  #7 0x001c0017
>  #8 0x3fffad949354 clang::Parser::ParseBaseClause(clang::Decl*)
> (/build/bin/../lib/../lib/libclangSema.so.11git+0x469354)
>  #9 0x3fffae1b3368
> clang::Parser::ParseCXXMemberSpecification(clang::SourceLocation,
> clang::SourceLocation, clang::Parser::ParsedAttributesWithRange&, unsigned
> int, clang::Decl*) (/build/bin/../lib/../lib/libclangParse.so.11git+0x73368)
> #10 0x3fffae1b3668
> clang::Parser::ParseClassSpecifier(clang::tok::TokenKind,
> clang::SourceLocation, clang::DeclSpec&, clang::Parser::ParsedTemplateInfo
> const&, clang::AccessSpecifier, bool, clang::Parser::DeclSpecContext,
> clang::Parser::ParsedAttributesWithRange&)
> (/build/bin/../lib/../lib/libclangParse.so.11git+0x73668)
> #11 0x3fffae1b884c
> clang::Parser::ParseDeclarationSpecifiers(clang::DeclSpec&,
> clang::Parser::ParsedTemplateInfo const&, clang::AccessSpecifier,
> clang::Parser::DeclSpecContext, clang::Parser::LateParsedAttrList*)
> (/build/bin/../lib/../lib/libclangParse.so.11git+0x7884c)
> #12 0x3fffae1ba2ac
> clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&,
> clang::ParsingDeclSpec&, clang::AccessSpecifier)
> (/build/bin/../lib/../lib/libclangParse.so.11git+0x7a2ac)
> #13 0x3fffae19b4f0
> clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&,
> clang::ParsingDeclSpec*, clang::AccessSpecifier) (.part.221)
> (/build/bin/../lib/../lib/libclangParse.so.11git+0x5b4f0)
> #14 0x3fffae248894
> clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&,
> clang::ParsingDeclSpec*)
> (/build/bin/../lib/../lib/libclangParse.so.11git+0x108894)
> #15 0x3fffae249174
> clang::Parser::ParseTopLevelDecl(clang::OpaquePtr&,
> bool) (/build/bin/../lib/../lib/libclangParse.so.11git+0x109174)
> #16 0x3fffae24d880 clang::ParseAST(clang::Sema&, bool, bool)
> (/build/bin/../lib/../lib/libclangParse.so.11git+0x10d880)
> #17 0x3fffae24f00c clang::ASTFrontendAction::ExecuteAction()
> (/build/bin/../lib/../lib/libclangParse.so.11git+0x10f00c)
> #18 0x3fffae1743a8 clang::FrontendAction::Execute()
> (/build/bin/../lib/../lib/libclangParse.so.11git+0x343a8)
> #19 0x3fffb07bc5c0
> clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)
> (/build/bin/../lib/libclangFrontend.so.11git+0x11c5c0)
> #20 0x3fffb07c100c
> clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
> (/build/bin/../lib/libclangFrontend.so.11git+0x12100c)
> #21 0x3fffb076d900 cc1_main(llvm::ArrayRef, char const*,
> void*) (/build/bin/../lib/libclangFrontend.so.11git+0xcd900)
> #22 0x3fffb06749dc ExecuteCC1Tool(llvm::SmallVectorImpl&)
> (/build/bin/../lib/libclangFrontendTool.so.11git+0x49dc)
> #23 0x1001635c main (/build/bin/clang+++0x1001635c)
> #24 0x1000f6b8 generic_start_main.isra.0
> (/build/bin/clang+++0x1000f6b8)
> #25 0x1000cdcc __libc_start_main (/build/bin/clang+++0x1000cdcc)
>
> /build/bin/../lib/libLLVMSupport.so.11git(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x38)[0x3fffb2016418]
> /build/bin/../lib/libLLVMSupport.so.11git(+0x206540)[0x3fffb2016540]
>
> /build/bin/../lib/libLLVMSupport.so.11git(_ZN4llvm3sys17RunSignalHandlersEv+0x78)[0x3fffb2013b38]
> /build/bin/../lib/libLLVMSupport.so.11git(+0x203d2c)[0x3fffb2013d2c]
> [0x3fffb4570478]
> [0x1c0017]
>
> 

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-30 Thread Sriraman Tallam via Phabricator via cfe-commits
tmsriram marked an inline comment as done.
tmsriram added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:5
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm 
-funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int glob;

wmi wrote:
> MaskRay wrote:
> > Might be worth adding a few other internal linkage names.
> > 
> > * an object in an unnamed namespace
> > * an extern "C" static function
> > * a function-local static variable
> > * `label: &`
> > 
> > Hope @mtrofin and @davidxl can clarify what internal names may benefit AFDO 
> > and we can add such internal names specifically.
> Only internal functions matter for AFDO. Other types of internal names are 
> irrelevant.
extern "C" static is not very useful as the name is not exposed outside the 
object file.  C++ compiler will still mangle the name. for eg:

a.cpp

extern "C" {
  static int bar() {
return 0;
  }
}

int foo() {
  printf("%p\n", bar);
}

$ nm a.o 
0040 t _ZL3barv


Also, for label: &, it is not preserved in the symbol table as a .L 
is used which is deleted by the assembler.  I can throw a asm("") directive but 
that will bypass the front-end.  Is there anyway to preserve this?  

I can add the rest of the cases.




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

https://reviews.llvm.org/D73307



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


Re: [clang] 499b2a8 - PR45294: Fix handling of assumed template names looked up in the lexical

2020-03-30 Thread Hubert Tong via cfe-commits
I have not found which of the few commits on Friday having to do with
template-ids is responsible, but this now produces a crash (trace is below):
struct A;
struct B : A {};

Richard, would you take a look?

Thanks,


Hubert Tong

:2:12: error: unknown template name 'A'
struct B : A {};
   ^
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash
backtrace, preprocessed source, and associated run script.
Stack dump:
0.  Program arguments: /build/bin/clang++ -cc1 -xc++ -
1.  :2:19: current parser token '{'
2.  :2:1: parsing struct/union/class body 'B'
 #0 0x3fffb2016418 llvm::sys::PrintStackTrace(llvm::raw_ostream&)
(/build/bin/../lib/libLLVMSupport.so.11git+0x206418)
 #1 0x3fffb2016540 PrintStackTraceSignalHandler(void*)
(/build/bin/../lib/libLLVMSupport.so.11git+0x206540)
 #2 0x3fffb2013b38 llvm::sys::RunSignalHandlers()
(/build/bin/../lib/libLLVMSupport.so.11git+0x203b38)
 #3 0x3fffb2013d2c SignalHandler(int)
(/build/bin/../lib/libLLVMSupport.so.11git+0x203d2c)
 #4 0x3fffb4570478  0x478 clang::Sema::ActOnBaseSpecifier(clang::Decl*,
clang::SourceRange, clang::ParsedAttributes&, bool, clang::AccessSpecifier,
clang::OpaquePtr, clang::SourceLocation,
clang::SourceLocation)
 #5 0x3fffb4570478
 #6 0x3fffb4570478 clang::Parser::ParseBaseSpecifier(clang::Decl*)
(+0x478)
 #7 0x001c0017
 #8 0x3fffad949354 clang::Parser::ParseBaseClause(clang::Decl*)
(/build/bin/../lib/../lib/libclangSema.so.11git+0x469354)
 #9 0x3fffae1b3368
clang::Parser::ParseCXXMemberSpecification(clang::SourceLocation,
clang::SourceLocation, clang::Parser::ParsedAttributesWithRange&, unsigned
int, clang::Decl*) (/build/bin/../lib/../lib/libclangParse.so.11git+0x73368)
#10 0x3fffae1b3668
clang::Parser::ParseClassSpecifier(clang::tok::TokenKind,
clang::SourceLocation, clang::DeclSpec&, clang::Parser::ParsedTemplateInfo
const&, clang::AccessSpecifier, bool, clang::Parser::DeclSpecContext,
clang::Parser::ParsedAttributesWithRange&)
(/build/bin/../lib/../lib/libclangParse.so.11git+0x73668)
#11 0x3fffae1b884c
clang::Parser::ParseDeclarationSpecifiers(clang::DeclSpec&,
clang::Parser::ParsedTemplateInfo const&, clang::AccessSpecifier,
clang::Parser::DeclSpecContext, clang::Parser::LateParsedAttrList*)
(/build/bin/../lib/../lib/libclangParse.so.11git+0x7884c)
#12 0x3fffae1ba2ac
clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&,
clang::ParsingDeclSpec&, clang::AccessSpecifier)
(/build/bin/../lib/../lib/libclangParse.so.11git+0x7a2ac)
#13 0x3fffae19b4f0
clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&,
clang::ParsingDeclSpec*, clang::AccessSpecifier) (.part.221)
(/build/bin/../lib/../lib/libclangParse.so.11git+0x5b4f0)
#14 0x3fffae248894
clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&,
clang::ParsingDeclSpec*)
(/build/bin/../lib/../lib/libclangParse.so.11git+0x108894)
#15 0x3fffae249174
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr&,
bool) (/build/bin/../lib/../lib/libclangParse.so.11git+0x109174)
#16 0x3fffae24d880 clang::ParseAST(clang::Sema&, bool, bool)
(/build/bin/../lib/../lib/libclangParse.so.11git+0x10d880)
#17 0x3fffae24f00c clang::ASTFrontendAction::ExecuteAction()
(/build/bin/../lib/../lib/libclangParse.so.11git+0x10f00c)
#18 0x3fffae1743a8 clang::FrontendAction::Execute()
(/build/bin/../lib/../lib/libclangParse.so.11git+0x343a8)
#19 0x3fffb07bc5c0
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)
(/build/bin/../lib/libclangFrontend.so.11git+0x11c5c0)
#20 0x3fffb07c100c
clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
(/build/bin/../lib/libclangFrontend.so.11git+0x12100c)
#21 0x3fffb076d900 cc1_main(llvm::ArrayRef, char const*,
void*) (/build/bin/../lib/libclangFrontend.so.11git+0xcd900)
#22 0x3fffb06749dc ExecuteCC1Tool(llvm::SmallVectorImpl&)
(/build/bin/../lib/libclangFrontendTool.so.11git+0x49dc)
#23 0x1001635c main (/build/bin/clang+++0x1001635c)
#24 0x1000f6b8 generic_start_main.isra.0
(/build/bin/clang+++0x1000f6b8)
#25 0x1000cdcc __libc_start_main (/build/bin/clang+++0x1000cdcc)
/build/bin/../lib/libLLVMSupport.so.11git(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x38)[0x3fffb2016418]
/build/bin/../lib/libLLVMSupport.so.11git(+0x206540)[0x3fffb2016540]
/build/bin/../lib/libLLVMSupport.so.11git(_ZN4llvm3sys17RunSignalHandlersEv+0x78)[0x3fffb2013b38]
/build/bin/../lib/libLLVMSupport.so.11git(+0x203d2c)[0x3fffb2013d2c]
[0x3fffb4570478]
[0x1c0017]
/build/bin/../lib/../lib/libclangSema.so.11git(_ZN5clang4Sema18ActOnBaseSpecifierEPNS_4DeclENS_11SourceRangeERNS_16ParsedAttributesEbNS_15AccessSpecifierENS_9OpaquePtrINS_8QualTypeEEENS_14SourceLocationESA_+0x234)[0x3fffad949354]
/build/bin/../lib/../lib/libclangParse.so.11git(_ZN5clang6Parser18ParseBaseSpecifierEPNS_4DeclE+0x1b8)[0x3fffae1b3368]

[PATCH] D76987: Rename options --cuda-gpu-arch and --no-cuda-gpu-arch

2020-03-30 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG764f54bb857b: Rename options --cuda-gpu-arch and 
--no-cuda-gpu-arch (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76987

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


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2526,13 +2526,13 @@
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
-if (!(A->getOption().matches(options::OPT_cuda_gpu_arch_EQ) ||
-  A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ)))
+if (!(A->getOption().matches(options::OPT_offload_arch_EQ) ||
+  A->getOption().matches(options::OPT_no_offload_arch_EQ)))
   continue;
 A->claim();
 
 const StringRef ArchStr = A->getValue();
-if (A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ) &&
+if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
 ArchStr == "all") {
   GpuArchs.clear();
   continue;
@@ -2541,9 +2541,9 @@
 if (Arch == CudaArch::UNKNOWN) {
   C.getDriver().Diag(clang::diag::err_drv_cuda_bad_gpu_arch) << 
ArchStr;
   Error = true;
-} else if (A->getOption().matches(options::OPT_cuda_gpu_arch_EQ))
+} else if (A->getOption().matches(options::OPT_offload_arch_EQ))
   GpuArchs.insert(Arch);
-else if (A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ))
+else if (A->getOption().matches(options::OPT_no_offload_arch_EQ))
   GpuArchs.erase(Arch);
 else
   llvm_unreachable("Unexpected option.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -571,13 +571,17 @@
   HelpText<"Include PTX for the following GPU architecture (e.g. sm_35) or 
'all'. May be specified more than once.">;
 def no_cuda_include_ptx_EQ : Joined<["--"], "no-cuda-include-ptx=">, 
Flags<[DriverOption]>,
   HelpText<"Do not include PTX for the following GPU architecture (e.g. sm_35) 
or 'all'. May be specified more than once.">;
+def offload_arch_EQ : Joined<["--"], "offload-arch=">, Flags<[DriverOption]>,
+  HelpText<"CUDA/HIP offloading device architecture (e.g. sm_35, gfx906).  May 
be specified more than once.">;
 def cuda_gpu_arch_EQ : Joined<["--"], "cuda-gpu-arch=">, Flags<[DriverOption]>,
-  HelpText<"CUDA GPU architecture (e.g. sm_35).  May be specified more than 
once.">;
+  Alias;
 def hip_link : Flag<["--"], "hip-link">,
   HelpText<"Link clang-offload-bundler bundles for HIP">;
-def no_cuda_gpu_arch_EQ : Joined<["--"], "no-cuda-gpu-arch=">, 
Flags<[DriverOption]>,
-  HelpText<"Remove GPU architecture (e.g. sm_35) from the list of GPUs to 
compile for. "
+def no_offload_arch_EQ : Joined<["--"], "no-offload-arch=">, 
Flags<[DriverOption]>,
+  HelpText<"Remove CUDA/HIP offloading device architecture (e.g. sm_35, 
gfx906) from the list of devices to compile for. "
"'all' resets the list to its default value.">;
+def no_cuda_gpu_arch_EQ : Joined<["--"], "no-cuda-gpu-arch=">, 
Flags<[DriverOption]>,
+  Alias;
 def cuda_noopt_device_debug : Flag<["--"], "cuda-noopt-device-debug">,
   HelpText<"Enable device-side debug info generation. Disables ptxas 
optimizations.">;
 def no_cuda_version_check : Flag<["--"], "no-cuda-version-check">,


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2526,13 +2526,13 @@
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
-if (!(A->getOption().matches(options::OPT_cuda_gpu_arch_EQ) ||
-  A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ)))
+if (!(A->getOption().matches(options::OPT_offload_arch_EQ) ||
+  A->getOption().matches(options::OPT_no_offload_arch_EQ)))
   continue;
 A->claim();
 
 const StringRef ArchStr = A->getValue();
-if (A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ) &&
+if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
 ArchStr == "all") {
   GpuArchs.clear();
   continue;
@@ -2541,9 +2541,9 @@
 if (Arch == CudaArch::UNKNOWN) {
   C.getDriver().Diag(clang::diag::err_drv_cuda_bad_gpu_arch) << ArchStr;
   Error = true;
-} else if (A->getOption().matches(options::OPT_cuda_gpu_arch_EQ))
+} else if (A->getOption().matches(options::OPT_offload_arch_EQ))
   GpuArchs.insert(Arch);
-else if 

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

2020-03-30 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

Can this be revived? Changing the enum to a string still sounds good to me


Repository:
  rC Clang

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

https://reviews.llvm.org/D75917



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


[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-03-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 253751.
njames93 added a comment.

- Fix assertions failing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77085

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -1,6 +1,8 @@
 #include "ClangTidyOptions.h"
-#include "gtest/gtest.h"
+#include "ClangTidyCheck.h"
+#include "ClangTidyDiagnosticConsumer.h"
 #include "llvm/ADT/StringExtras.h"
+#include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
@@ -97,6 +99,155 @@
 llvm::join(Options.ExtraArgsBefore->begin(),
Options.ExtraArgsBefore->end(), ","));
 }
+
+class TestCheck : public ClangTidyCheck {
+public:
+  TestCheck(ClangTidyContext *Context) : ClangTidyCheck("test", Context) {}
+
+  template  auto getLocal(Args &&... Arguments) {
+return Options.get(std::forward(Arguments)...);
+  }
+
+  template  auto getGlobal(Args &&... Arguments) {
+return Options.getLocalOrGlobal(std::forward(Arguments)...);
+  }
+
+  template 
+  auto getIntLocal(Args &&... Arguments) {
+return Options.get(std::forward(Arguments)...);
+  }
+
+  template 
+  auto getIntGlobal(Args &&... Arguments) {
+return Options.getLocalOrGlobal(std::forward(Arguments)...);
+  }
+};
+
+#define CHECK_VAL(Value, Expected) \
+  do { \
+auto Item = Value; \
+ASSERT_TRUE(!!Item);   \
+EXPECT_EQ(*Item, Expected);\
+  } while (false)
+
+#define CHECK_ERROR(Value, ErrorType, ExpectedMessage) \
+  do { \
+auto Item = Value; \
+ASSERT_FALSE(Item);\
+ASSERT_TRUE(Item.errorIsA());   \
+ASSERT_FALSE(llvm::handleErrors(   \
+Item.takeError(), [&](const ErrorType ) -> llvm::Error {   \
+  EXPECT_EQ(Err.message(), ExpectedMessage);   \
+  return llvm::Error::success();   \
+}));   \
+  } while (false)
+
+TEST(CheckOptionsValidation, MissingOptions) {
+  ClangTidyOptions Options;
+  ClangTidyContext Context(std::make_unique(
+  ClangTidyGlobalOptions(), Options));
+  TestCheck TestCheck();
+  CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError,
+  "warning: Option not found 'test.Opt'\n");
+  EXPECT_EQ(TestCheck.getLocal("Opt", "Unknown"), "Unknown");
+}
+
+TEST(CheckOptionsValidation, ValidIntOptions) {
+  ClangTidyOptions Options;
+  auto  = Options.CheckOptions;
+  CheckOptions["test.IntExpected1"] = "1";
+  CheckOptions["test.IntExpected2"] = "1WithMore";
+  CheckOptions["test.IntExpected3"] = "NoInt";
+  CheckOptions["GlobalIntExpected1"] = "1";
+  CheckOptions["GlobalIntExpected2"] = "NoInt";
+  CheckOptions["test.DefaultedIntInvalid"] = "NoInt";
+  CheckOptions["GlobalIntInvalid"] = "NoInt";
+
+  ClangTidyContext Context(std::make_unique(
+  ClangTidyGlobalOptions(), Options));
+  TestCheck TestCheck();
+
+#define CHECK_ERROR_INT(Name, Expected)\
+  CHECK_ERROR(Name, UnparseableIntegerOptionError, Expected)
+
+  CHECK_VAL(TestCheck.getIntLocal("IntExpected1"), 1);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalIntExpected1"), 1);
+  CHECK_ERROR_INT(TestCheck.getIntLocal("IntExpected2"),
+  "warning: Invalid configuration value '1WithMore' for option "
+  "'test.IntExpected2'; expected an integer value\n");
+  CHECK_ERROR_INT(TestCheck.getIntLocal("IntExpected3"),
+  "warning: Invalid configuration value 'NoInt' for option "
+  "'test.IntExpected3'; expected an integer value\n");
+  CHECK_ERROR_INT(TestCheck.getIntGlobal("GlobalIntExpected2"),
+  "warning: Invalid configuration value 'NoInt' for option "
+  "'GlobalIntExpected2'; expected an integer value\n");
+  ASSERT_EQ(TestCheck.getIntLocal("DefaultedIntInvalid", 1), 1);
+  ASSERT_EQ(TestCheck.getIntGlobal("GlobalIntInvalid", 1), 1);
+
+#undef CHECK_ERROR_INT
+}
+
+TEST(ValidConfiguration, 

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-30 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:5
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm 
-funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int glob;

MaskRay wrote:
> Might be worth adding a few other internal linkage names.
> 
> * an object in an unnamed namespace
> * an extern "C" static function
> * a function-local static variable
> * `label: &`
> 
> Hope @mtrofin and @davidxl can clarify what internal names may benefit AFDO 
> and we can add such internal names specifically.
Only internal functions matter for AFDO. Other types of internal names are 
irrelevant.


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

https://reviews.llvm.org/D73307



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


[PATCH] D76987: Rename options --cuda-gpu-arch and --no-cuda-gpu-arch

2020-03-30 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D76987#1950366 , @gregrodgers wrote:

> This was discussed on llvm-dev three years ago.  Here is the thread.
>
> http://lists.llvm.org/pipermail/llvm-dev/2017-February/109930.html
>
> The last name discussed was "-- offload-arch".   I don't believe we need a 
> list option anymore.   So ignore the very old request for --offload-archs.
>
> I am ok with the patch the way it is.   In the future, we should consider 
> renaming the CudaArch class to OffloadArch class .  Also the GpuArchList is 
> currently only initialized in CudaActionBuilder.   Eventually this is will 
> have to be done for HIPActionBuilder and OpenMPActionBuilder.   Could you 
> consider creating a function to  InitializeGpuArchList ?


GpuArchList is initialized by member function initialize() of  
CudaActionBuilderBase, which is inherited by CudaActionBuilder and 
HIPActionBuilder, therefore GpuArchList is initialized in both 
CudaActionBuilder and HIPActionBuilder.


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

https://reviews.llvm.org/D76987



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


[clang] 764f54b - Rename options --cuda-gpu-arch and --no-cuda-gpu-arch

2020-03-30 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2020-03-30T20:29:50-04:00
New Revision: 764f54bb857b0fbc6742f306b09f640e0043791d

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

LOG: Rename options --cuda-gpu-arch and --no-cuda-gpu-arch

Per discussion

http://lists.llvm.org/pipermail/llvm-dev/2017-February/109930.html

Rename -cuda-gpu-arch and --no-cuda-gpu-arch to
--offload-arch and --no-offload-arch.

The original options will be alias to the new options.

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 1358b463eb1a..2e8d4b1d2363 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -571,13 +571,17 @@ def cuda_include_ptx_EQ : Joined<["--"], 
"cuda-include-ptx=">, Flags<[DriverOpti
   HelpText<"Include PTX for the following GPU architecture (e.g. sm_35) or 
'all'. May be specified more than once.">;
 def no_cuda_include_ptx_EQ : Joined<["--"], "no-cuda-include-ptx=">, 
Flags<[DriverOption]>,
   HelpText<"Do not include PTX for the following GPU architecture (e.g. sm_35) 
or 'all'. May be specified more than once.">;
+def offload_arch_EQ : Joined<["--"], "offload-arch=">, Flags<[DriverOption]>,
+  HelpText<"CUDA/HIP offloading device architecture (e.g. sm_35, gfx906).  May 
be specified more than once.">;
 def cuda_gpu_arch_EQ : Joined<["--"], "cuda-gpu-arch=">, Flags<[DriverOption]>,
-  HelpText<"CUDA GPU architecture (e.g. sm_35).  May be specified more than 
once.">;
+  Alias;
 def hip_link : Flag<["--"], "hip-link">,
   HelpText<"Link clang-offload-bundler bundles for HIP">;
-def no_cuda_gpu_arch_EQ : Joined<["--"], "no-cuda-gpu-arch=">, 
Flags<[DriverOption]>,
-  HelpText<"Remove GPU architecture (e.g. sm_35) from the list of GPUs to 
compile for. "
+def no_offload_arch_EQ : Joined<["--"], "no-offload-arch=">, 
Flags<[DriverOption]>,
+  HelpText<"Remove CUDA/HIP offloading device architecture (e.g. sm_35, 
gfx906) from the list of devices to compile for. "
"'all' resets the list to its default value.">;
+def no_cuda_gpu_arch_EQ : Joined<["--"], "no-cuda-gpu-arch=">, 
Flags<[DriverOption]>,
+  Alias;
 def cuda_noopt_device_debug : Flag<["--"], "cuda-noopt-device-debug">,
   HelpText<"Enable device-side debug info generation. Disables ptxas 
optimizations.">;
 def no_cuda_version_check : Flag<["--"], "no-cuda-version-check">,

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 4afe3e635fc8..9e1f41345ea2 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2526,13 +2526,13 @@ class OffloadingActionBuilder final {
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
-if (!(A->getOption().matches(options::OPT_cuda_gpu_arch_EQ) ||
-  A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ)))
+if (!(A->getOption().matches(options::OPT_offload_arch_EQ) ||
+  A->getOption().matches(options::OPT_no_offload_arch_EQ)))
   continue;
 A->claim();
 
 const StringRef ArchStr = A->getValue();
-if (A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ) &&
+if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
 ArchStr == "all") {
   GpuArchs.clear();
   continue;
@@ -2541,9 +2541,9 @@ class OffloadingActionBuilder final {
 if (Arch == CudaArch::UNKNOWN) {
   C.getDriver().Diag(clang::diag::err_drv_cuda_bad_gpu_arch) << 
ArchStr;
   Error = true;
-} else if (A->getOption().matches(options::OPT_cuda_gpu_arch_EQ))
+} else if (A->getOption().matches(options::OPT_offload_arch_EQ))
   GpuArchs.insert(Arch);
-else if (A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ))
+else if (A->getOption().matches(options::OPT_no_offload_arch_EQ))
   GpuArchs.erase(Arch);
 else
   llvm_unreachable("Unexpected option.");



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


[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

2020-03-30 Thread Thomas Lively via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5074776de478: [WebAssembly] Import wasm_simd128.h from 
Emscripten (authored by tlively).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76959

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/wasm_simd128.h

Index: clang/lib/Headers/wasm_simd128.h
===
--- /dev/null
+++ clang/lib/Headers/wasm_simd128.h
@@ -0,0 +1,1145 @@
+/*=== wasm_simd128.h - WebAssembly portable SIMD intrinsics ===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __WASM_SIMD128_H
+#define __WASM_SIMD128_H
+
+#include 
+#include 
+
+// User-facing type
+typedef int32_t v128_t __attribute__((__vector_size__(16), __aligned__(16)));
+
+// Internal types determined by clang builtin definitions
+typedef int32_t __v128_u __attribute__((__vector_size__(16), __aligned__(1)));
+typedef char __i8x16 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef signed char __s8x16
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned char __u8x16
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef short __i16x8 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned short __u16x8
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef int __i32x4 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned int __u32x4
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef long long __i64x2 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef float __f32x4 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef double __f64x2 __attribute__((__vector_size__(16), __aligned__(16)));
+
+#define __DEFAULT_FN_ATTRS \
+  __attribute__((__always_inline__, __nodebug__, __target__("simd128"),\
+ __min_vector_width__(128)))
+
+#define __REQUIRE_CONSTANT(e)  \
+  _Static_assert(__builtin_constant_p(e), "Expected constant")
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_v128_load(const void *__mem) {
+  // UB-free unaligned access copied from xmmintrin.h
+  struct __wasm_v128_load_struct {
+__v128_u __v;
+  } __attribute__((__packed__, __may_alias__));
+  return ((const struct __wasm_v128_load_struct *)__mem)->__v;
+}
+
+#ifdef __wasm_unimplemented_simd128__
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v8x16_load_splat(const void *__mem) {
+  struct __wasm_v8x16_load_splat_struct {
+uint8_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint8_t __v = ((const struct __wasm_v8x16_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u8x16){__v, __v, __v, __v, __v, __v, __v, __v,
+   __v, __v, __v, __v, __v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v16x8_load_splat(const void *__mem) {
+  struct __wasm_v16x8_load_splat_struct {
+uint16_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint16_t __v = ((const struct __wasm_v16x8_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u16x8){__v, __v, __v, __v, __v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v32x4_load_splat(const void *__mem) {
+  struct __wasm_v32x4_load_splat_struct {
+uint32_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint32_t __v = ((const struct __wasm_v32x4_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u32x4){__v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v64x2_load_splat(const void *__mem) {
+  struct __wasm_v64x2_load_splat_struct {
+uint64_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint64_t __v = ((const struct __wasm_v64x2_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u64x2){__v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_i16x8_load_8x8(const void *__mem) {
+  typedef int8_t __i8x8 __attribute__((__vector_size__(8), __aligned__(8)));
+  struct __wasm_i16x8_load_8x8_struct {
+__i8x8 __v;
+  } __attribute__((__packed__, __may_alias__));
+  __i8x8 __v = ((const struct __wasm_i16x8_load_8x8_struct *)__mem)->__v;
+  return (v128_t) __builtin_convertvector(__v, __i16x8);
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_u16x8_load_8x8(const void *__mem) {
+  typedef uint8_t __u8x8 __attribute__((__vector_size__(8), __aligned__(8)));
+  struct __wasm_u16x8_load_8x8_struct {
+__u8x8 __v;
+  } 

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

2020-03-30 Thread Bhopesh Bassi via Phabricator via cfe-commits
bbassi added a comment.

@MyDeveloperDay Can you please share your thoughts on my comment above?


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

https://reviews.llvm.org/D33029



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


[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

2020-03-30 Thread Thomas Lively via Phabricator via cfe-commits
tlively marked an inline comment as done.
tlively added inline comments.



Comment at: clang/lib/Headers/wasm_simd128.h:30
+typedef long long __i64x2 __attribute__((__vector_size__(16), 
__aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));

sunfish wrote:
> tlively wrote:
> > sunfish wrote:
> > > Since this file is already including , instead of `char`, 
> > > `unsigned char`, `short`, `unsigned short`, etc., can these use `int8_t`, 
> > > `uint8_t`, `int16_t`, `uint16_t`, and so on, for clarity? Also, this 
> > > would avoid any possible behavior change under `-funsigned-char`.
> > Unfortunately not :( This change gives errors like
> > 
> > ```
> > error: cannot initialize a parameter of type 
> > '__attribute__((__vector_size__(16 * sizeof(char char' (vector of 16 
> > 'char' values) with an rvalue of type '__i8x16' (vector of 16 'int8_t' 
> > values)
> >   return (v128_t)__builtin_wasm_abs_i8x16((__i8x16)a);
> > ```
> > 
> > Even changing `char` to `signed char` doesn't work. I would change the 
> > builtins to use the better types, but the builtin definition system does 
> > not allow those more interesting types.
> > 
> > This is especially annoying because `-funsigned-char` does indeed change 
> > the behavior of the comparison intrinsics. I will have to do extra work in 
> > those functions to make it work.
> Would changing the clang definitions of the builtins to use signed char (Sc) 
> explicitly fix this, like:
> 
> -TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", 
> "unimplemented-simd128")
> +TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16ScV16ScV16Sc", "nc", 
> "unimplemented-simd128")
> 
> and so on for all the wasm simd builtins?
Neat, I hadn't remembered that those signed and unsigned prefixes existed. Yes, 
that would enable some simplifications, but I think they would be better done 
in a follow-on CL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76959



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


[clang] 3308732 - Fix crash if base specifier parsing hits an invalid type annotation.

2020-03-30 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-03-30T17:21:40-07:00
New Revision: 330873230071ffc2aebc0fe74db55e7a530c2f1b

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

LOG: Fix crash if base specifier parsing hits an invalid type annotation.

Also change type annotation representation from ParsedType to TypeResult
to make it clearer to consumers that they can represent invalid types.

Added: 


Modified: 
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/DeclSpec.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExpr.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Parse/ParseTemplate.cpp
clang/lib/Parse/Parser.cpp
clang/test/Parser/cxx-class.cpp

Removed: 




diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index c64a01b73a4a..290da0006116 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -763,13 +763,17 @@ class Parser : public CodeCompletionHandler {
   }
 
   /// getTypeAnnotation - Read a parsed type out of an annotation token.
-  static ParsedType getTypeAnnotation(const Token ) {
+  static TypeResult getTypeAnnotation(const Token ) {
+if (!Tok.getAnnotationValue())
+  return TypeError();
 return ParsedType::getFromOpaquePtr(Tok.getAnnotationValue());
   }
 
 private:
-  static void setTypeAnnotation(Token , ParsedType T) {
-Tok.setAnnotationValue(T.getAsOpaquePtr());
+  static void setTypeAnnotation(Token , TypeResult T) {
+assert((T.isInvalid() || T.get()) &&
+   "produced a valid-but-null type annotation?");
+Tok.setAnnotationValue(T.isInvalid() ? nullptr : T.get().getAsOpaquePtr());
   }
 
   static NamedDecl *getNonTypeAnnotation(const Token ) {

diff  --git a/clang/include/clang/Sema/DeclSpec.h 
b/clang/include/clang/Sema/DeclSpec.h
index 5a6bedcd6f59..78010b7bb46e 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -667,6 +667,13 @@ class DeclSpec {
   bool SetTypeSpecType(TST T, SourceLocation Loc, const char *,
unsigned , ParsedType Rep,
const PrintingPolicy );
+  bool SetTypeSpecType(TST T, SourceLocation Loc, const char *,
+   unsigned , TypeResult Rep,
+   const PrintingPolicy ) {
+if (Rep.isInvalid())
+  return SetTypeSpecError();
+return SetTypeSpecType(T, Loc, PrevSpec, DiagID, Rep.get(), Policy);
+  }
   bool SetTypeSpecType(TST T, SourceLocation Loc, const char *,
unsigned , Decl *Rep, bool Owned,
const PrintingPolicy );

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 95706fb999c4..e60d1f68fba5 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3217,16 +3217,12 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec ,
   if (Next.is(tok::annot_typename)) {
 DS.getTypeSpecScope() = SS;
 ConsumeAnnotationToken(); // The C++ scope.
-if (Tok.getAnnotationValue()) {
-  ParsedType T = getTypeAnnotation(Tok);
-  isInvalid = DS.SetTypeSpecType(DeclSpec::TST_typename,
- Tok.getAnnotationEndLoc(),
- PrevSpec, DiagID, T, Policy);
-  if (isInvalid)
-break;
-}
-else
-  DS.SetTypeSpecError();
+TypeResult T = getTypeAnnotation(Tok);
+isInvalid = DS.SetTypeSpecType(DeclSpec::TST_typename,
+   Tok.getAnnotationEndLoc(),
+   PrevSpec, DiagID, T, Policy);
+if (isInvalid)
+  break;
 DS.SetRangeEnd(Tok.getAnnotationEndLoc());
 ConsumeAnnotationToken(); // The typename
   }
@@ -3294,13 +3290,9 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec ,
   if (DS.hasTypeSpecifier() && DS.hasTagDefinition())
 goto DoneWithDeclSpec;
 
-  if (Tok.getAnnotationValue()) {
-ParsedType T = getTypeAnnotation(Tok);
-isInvalid = DS.SetTypeSpecType(DeclSpec::TST_typename, Loc, PrevSpec,
-   DiagID, T, Policy);
-  } else
-DS.SetTypeSpecError();
-
+  TypeResult T = getTypeAnnotation(Tok);
+  isInvalid = DS.SetTypeSpecType(DeclSpec::TST_typename, Loc, PrevSpec,
+ DiagID, T, Policy);
   if (isInvalid)
 break;
 

diff  --git a/clang/lib/Parse/ParseDeclCXX.cpp 
b/clang/lib/Parse/ParseDeclCXX.cpp
index 98918efaf295..710632379ace 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ 

[clang] 5074776 - [WebAssembly] Import wasm_simd128.h from Emscripten

2020-03-30 Thread Thomas Lively via cfe-commits

Author: Thomas Lively
Date: 2020-03-30T17:04:18-07:00
New Revision: 5074776de478a114ece3f82668aa1363b2f17c92

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

LOG: [WebAssembly] Import wasm_simd128.h from Emscripten

Summary:
As the WebAssembly SIMD proposal nears stabilization, there is desire
to use it with toolchains other than Emscripten. Moving the intrinsics
header to clang will make it available to WASI toolchains as well.

Reviewers: aheejin, sunfish

Subscribers: dschuff, mgorny, sbc100, jgravelle-google, cfe-commits

Tags: #clang

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

Added: 
clang/lib/Headers/wasm_simd128.h

Modified: 
clang/lib/Headers/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Headers/CMakeLists.txt 
b/clang/lib/Headers/CMakeLists.txt
index 28d43cb7ed35..8124549bfc48 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -109,6 +109,7 @@ set(files
   vecintrin.h
   vpclmulqdqintrin.h
   waitpkgintrin.h
+  wasm_simd128.h
   wbnoinvdintrin.h
   wmmintrin.h
   __wmmintrin_aes.h

diff  --git a/clang/lib/Headers/wasm_simd128.h 
b/clang/lib/Headers/wasm_simd128.h
new file mode 100644
index ..3b30ddbd527b
--- /dev/null
+++ b/clang/lib/Headers/wasm_simd128.h
@@ -0,0 +1,1145 @@
+/*=== wasm_simd128.h - WebAssembly portable SIMD intrinsics ===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __WASM_SIMD128_H
+#define __WASM_SIMD128_H
+
+#include 
+#include 
+
+// User-facing type
+typedef int32_t v128_t __attribute__((__vector_size__(16), __aligned__(16)));
+
+// Internal types determined by clang builtin definitions
+typedef int32_t __v128_u __attribute__((__vector_size__(16), __aligned__(1)));
+typedef char __i8x16 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef signed char __s8x16
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned char __u8x16
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef short __i16x8 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned short __u16x8
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef int __i32x4 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned int __u32x4
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef long long __i64x2 __attribute__((__vector_size__(16), 
__aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef float __f32x4 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef double __f64x2 __attribute__((__vector_size__(16), __aligned__(16)));
+
+#define __DEFAULT_FN_ATTRS 
\
+  __attribute__((__always_inline__, __nodebug__, __target__("simd128"),
\
+ __min_vector_width__(128)))
+
+#define __REQUIRE_CONSTANT(e)  
\
+  _Static_assert(__builtin_constant_p(e), "Expected constant")
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_v128_load(const void *__mem) {
+  // UB-free unaligned access copied from xmmintrin.h
+  struct __wasm_v128_load_struct {
+__v128_u __v;
+  } __attribute__((__packed__, __may_alias__));
+  return ((const struct __wasm_v128_load_struct *)__mem)->__v;
+}
+
+#ifdef __wasm_unimplemented_simd128__
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v8x16_load_splat(const void *__mem) {
+  struct __wasm_v8x16_load_splat_struct {
+uint8_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint8_t __v = ((const struct __wasm_v8x16_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u8x16){__v, __v, __v, __v, __v, __v, __v, __v,
+   __v, __v, __v, __v, __v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v16x8_load_splat(const void *__mem) {
+  struct __wasm_v16x8_load_splat_struct {
+uint16_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint16_t __v = ((const struct __wasm_v16x8_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u16x8){__v, __v, __v, __v, __v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v32x4_load_splat(const void *__mem) {
+  struct __wasm_v32x4_load_splat_struct {
+uint32_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint32_t __v = ((const struct __wasm_v32x4_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u32x4){__v, __v, __v, __v};
+}
+
+static __inline__ v128_t 

[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

2020-03-30 Thread Dan Gohman via Phabricator via cfe-commits
sunfish accepted this revision.
sunfish added a comment.
This revision is now accepted and ready to land.

Cool, LGTM, with optional suggestion for signed char below:




Comment at: clang/lib/Headers/wasm_simd128.h:30
+typedef long long __i64x2 __attribute__((__vector_size__(16), 
__aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));

tlively wrote:
> sunfish wrote:
> > Since this file is already including , instead of `char`, 
> > `unsigned char`, `short`, `unsigned short`, etc., can these use `int8_t`, 
> > `uint8_t`, `int16_t`, `uint16_t`, and so on, for clarity? Also, this would 
> > avoid any possible behavior change under `-funsigned-char`.
> Unfortunately not :( This change gives errors like
> 
> ```
> error: cannot initialize a parameter of type 
> '__attribute__((__vector_size__(16 * sizeof(char char' (vector of 16 
> 'char' values) with an rvalue of type '__i8x16' (vector of 16 'int8_t' values)
>   return (v128_t)__builtin_wasm_abs_i8x16((__i8x16)a);
> ```
> 
> Even changing `char` to `signed char` doesn't work. I would change the 
> builtins to use the better types, but the builtin definition system does not 
> allow those more interesting types.
> 
> This is especially annoying because `-funsigned-char` does indeed change the 
> behavior of the comparison intrinsics. I will have to do extra work in those 
> functions to make it work.
Would changing the clang definitions of the builtins to use signed char (Sc) 
explicitly fix this, like:

-TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", 
"unimplemented-simd128")
+TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16ScV16ScV16Sc", "nc", 
"unimplemented-simd128")

and so on for all the wasm simd builtins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76959



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


[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

2020-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked 2 inline comments as done.
rnk added a comment.

Thanks for the feedback, I'm going to investigate if we can use the `used` 
destructor bit to do this.




Comment at: clang/include/clang/AST/DeclCXX.h:959-963
+  /// Indicates if the complete destructor has been implicitly declared
+  /// yet. Only relevant in the Microsoft C++.
+  bool definedImplicitCompleteDestructor() const {
+return data().DefinedImplicitCompleteDestructor;
+  }

rsmith wrote:
> "declared" in comment but "defined" in function name. Which is it?
> 
> I wonder if perhaps the right answer is neither: if we had separate `Decl`s 
> for the complete and base destructors, this would be tracked by the "used" 
> bit on the complete object destructor. So perhaps `isCompleteDestructorUsed` 
> and `setCompleteDestructorUsed` would make sense? (We could consider tracking 
> this on the destructor instead of here, but I suppose tracking it here gives 
> us the serialization and merging logic for free.)
Actually, can we just reuse the `used` bit on the destructor? I don't think 
there is any way for the user to "use" the base destructor without using the 
complete destructor. The only case I can think of that calls the base 
destructor (ignoring aliasing optimizations) is the complete destructor of a 
derived class. Such a class will also reference all the virtual bases of the 
current class, so the semantic checks we are doing here are redundant, not 
wrong.

Calling a pseudo-destructor for example uses the complete destructor, I think.



Comment at: clang/lib/Sema/SemaExpr.cpp:16008-16013
+// In the MS ABI, the complete destructor is implicitly defined,
+// even if the base destructor is user defined.
+CXXRecordDecl *Parent = Destructor->getParent();
+if (Parent->getNumVBases() > 0 &&
+!Parent->definedImplicitCompleteDestructor())
+  DefineImplicitCompleteDestructor(Loc, Destructor);

rsmith wrote:
> Can we avoid doing this when we know the call is to a base subobject 
> destructor or uses virtual dispatch? Should we? (I mean I guess ideally we'd 
> change this function to take a `GlobalDecl` instead of a `FunctionDecl*`, but 
> that seems like a lot of work.)
> 
> How does MSVC behave?
I think we can skip this if virtual dispatch is used, but I'm not sure what 
kinds of devirtualization might break my assumptions.

For example, uncommenting `final` here causes MSVC to diagnose the error, but 
it otherwise it compiles:
```
struct NoDtor { ~NoDtor() = delete; };
struct DefaultedDtor : NoDtor { ~DefaultedDtor() = default; };
struct HasVBases /*final*/ : virtual DefaultedDtor {
  virtual ~HasVBases();
};
void deleteIt1(HasVBases *p) { delete p; }
```

If the NoDtor destructor is undeleted and the class is made final, then both 
deleting and complete dtors are emitted for HasVBases. Clang would need to mark 
DefaultedDtor referenced in this case.

I think it's OK to "overdiagnose" in this case. It's not possible to emit a 
vtable here without diagnosing the error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77081



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


[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1165
   const auto *ND = cast(GD.getDecl());
+
   std::string MangledName = getMangledNameImpl(*this, GD, ND);

Delete this unrelated cosmetic change.


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

https://reviews.llvm.org/D73307



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


[PATCH] D77103: Add a new -fglobal-isel option and make -fexperimental-isel an alias for it.

2020-03-30 Thread Amara Emerson via Phabricator via cfe-commits
aemerson created this revision.
aemerson added reviewers: paquette, arsenm, qcolombet, bogner.
aemerson added a project: LLVM.
Herald added subscribers: cfe-commits, danielkiss, kristof.beyls, wdng.
Herald added a project: clang.
aemerson added a comment.

Sorry, forgot to add cfe-commits to the original diff.


Since GlobalISel is maturing and is already on at -O0 for AArch64, it's not 
completely "experimental". Create a more appropriate driver flag and make the 
older option an alias for it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77103

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/global-isel.c

Index: clang/test/Driver/global-isel.c
===
--- clang/test/Driver/global-isel.c
+++ clang/test/Driver/global-isel.c
@@ -1,24 +1,35 @@
 // REQUIRES: x86-registered-target,aarch64-registered-target
 
+// RUN: %clang -fglobal-isel -S -### %s 2>&1 | FileCheck --check-prefix=ENABLED %s
+// RUN: %clang -fno-global-isel -S -### %s 2>&1 | FileCheck --check-prefix=DISABLED %s
+
+// RUN: %clang -target aarch64 -fglobal-isel -S %s -### 2>&1 | FileCheck --check-prefix=ARM64-DEFAULT %s
+// RUN: %clang -target aarch64 -fglobal-isel -S -O0 %s -### 2>&1 | FileCheck --check-prefix=ARM64-O0 %s
+// RUN: %clang -target aarch64 -fglobal-isel -S -O2 %s -### 2>&1 | FileCheck --check-prefix=ARM64-O2 %s
+// RUN: %clang -target aarch64 -fglobal-isel -Wno-global-isel -S -O2 %s -### 2>&1 | FileCheck --check-prefix=ARM64-O2-NOWARN %s
+
+// RUN: %clang -target x86_64 -fglobal-isel -S %s -### 2>&1 | FileCheck --check-prefix=X86_64 %s
+
+// Now test the aliases.
+
 // RUN: %clang -fexperimental-isel -S -### %s 2>&1 | FileCheck --check-prefix=ENABLED %s
 // RUN: %clang -fno-experimental-isel -S -### %s 2>&1 | FileCheck --check-prefix=DISABLED %s
 
 // RUN: %clang -target aarch64 -fexperimental-isel -S %s -### 2>&1 | FileCheck --check-prefix=ARM64-DEFAULT %s
 // RUN: %clang -target aarch64 -fexperimental-isel -S -O0 %s -### 2>&1 | FileCheck --check-prefix=ARM64-O0 %s
 // RUN: %clang -target aarch64 -fexperimental-isel -S -O2 %s -### 2>&1 | FileCheck --check-prefix=ARM64-O2 %s
-// RUN: %clang -target aarch64 -fexperimental-isel -Wno-experimental-isel -S -O2 %s -### 2>&1 | FileCheck --check-prefix=ARM64-O2-NOWARN %s
 
 // RUN: %clang -target x86_64 -fexperimental-isel -S %s -### 2>&1 | FileCheck --check-prefix=X86_64 %s
 
 // ENABLED: "-mllvm" "-global-isel=1"
 // DISABLED: "-mllvm" "-global-isel=0"
 
-// ARM64-DEFAULT-NOT: warning: -fexperimental-sel
+// ARM64-DEFAULT-NOT: warning: -fglobal-isel
 // ARM64-DEFAULT-NOT: "-global-isel-abort=2"
-// ARM64-O0-NOT: warning: -fexperimental-sel
-// ARM64-O2: warning: -fexperimental-isel support is incomplete for this architecture at the current optimization level
+// ARM64-O0-NOT: warning: -fglobal-isel
+// ARM64-O2: warning: -fglobal-isel support is incomplete for this architecture at the current optimization level
 // ARM64-O2: "-mllvm" "-global-isel-abort=2"
-// ARM64-O2-NOWARN-NOT: warning: -fexperimental-isel
+// ARM64-O2-NOWARN-NOT: warning: -fglobal-isel
 
-// X86_64: -fexperimental-isel support for the 'x86_64' architecture is incomplete
+// X86_64: -fglobal-isel support for the 'x86_64' architecture is incomplete
 // X86_64: "-mllvm" "-global-isel-abort=2"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6062,10 +6062,10 @@
   if (SplitLTOUnit)
 CmdArgs.push_back("-fsplit-lto-unit");
 
-  if (Arg *A = Args.getLastArg(options::OPT_fexperimental_isel,
-   options::OPT_fno_experimental_isel)) {
+  if (Arg *A = Args.getLastArg(options::OPT_fglobal_isel,
+   options::OPT_fno_global_isel)) {
 CmdArgs.push_back("-mllvm");
-if (A->getOption().matches(options::OPT_fexperimental_isel)) {
+if (A->getOption().matches(options::OPT_fglobal_isel)) {
   CmdArgs.push_back("-global-isel=1");
 
   // GISel is on by default on AArch64 -O0, so don't bother adding
@@ -6084,9 +6084,9 @@
 CmdArgs.push_back("-global-isel-abort=2");
 
 if (!IsArchSupported)
-  D.Diag(diag::warn_drv_experimental_isel_incomplete) << Triple.getArchName();
+  D.Diag(diag::warn_drv_global_isel_incomplete) << Triple.getArchName();
 else
-  D.Diag(diag::warn_drv_experimental_isel_incomplete_opt);
+  D.Diag(diag::warn_drv_global_isel_incomplete_opt);
   }
 } else {
   CmdArgs.push_back("-global-isel=0");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ 

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D73307#1942838 , @tmsriram wrote:

> In D73307#1942805 , @MaskRay wrote:
>
> > In D73307#1932131 , @rnk wrote:
> >
> > > At a higher level, should this just be an IR pass that clang adds into 
> > > the pipeline when the flag is set? It should be safe to rename internal 
> > > functions and give private functions internal linkage. It would be less 
> > > invasive to clang and have better separation of concerns. As written, 
> > > this is based on the source filename on the module, which is accessible 
> > > from IR. The only reason I can think of against this is that the debug 
> > > info might refer to the function linkage name, but maybe that is 
> > > calculated later during codegen.
> >
> >
> > I second this suggestion. `clang/lib/CodeGen/BackendUtil.cpp` 
> > `EmitAssemblyHelper::EmitAssemblyWithNewPassManager` ModulePassManager may 
> > be a more appropriate place for this feature. There are examples from both 
> > sides:
>
>
> @rnk getMangledNameImpl for example adds multi-versioning suffixes,  
> "__regcall3_" and  "__device_stub".  Adding the hash is just so simple.  Is a 
> pass really needed?  If so, I should parse "." suffixed 
> Multi-versioining names and insert the hash in between as the demangling 
> looks better?
>
> > 
> > 
> > - clang frontend: `#pragma redefine_extname`
> > - middle end IR->IR: the old pass manager has a feature -frewrite-map-file, 
> > which does a similar thing but is implemented as an IR transformation.


There may be some other interactions @rnk noticed. I am happy with either way.




Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:5
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm 
-funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int glob;

Might be worth adding a few other internal linkage names.

* an object in an unnamed namespace
* an extern "C" static function
* a function-local static variable
* `label: &`

Hope @mtrofin and @davidxl can clarify what internal names may benefit AFDO and 
we can add such internal names specifically.


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

https://reviews.llvm.org/D73307



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


[PATCH] D77103: Add a new -fglobal-isel option and make -fexperimental-isel an alias for it.

2020-03-30 Thread Amara Emerson via Phabricator via cfe-commits
aemerson added a comment.

Sorry, forgot to add cfe-commits to the original diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77103



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


[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

2020-03-30 Thread Thomas Lively via Phabricator via cfe-commits
tlively added a comment.

I believe all the feedback has now been addressed.




Comment at: clang/lib/Headers/wasm_simd128.h:30
+typedef long long __i64x2 __attribute__((__vector_size__(16), 
__aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));

sunfish wrote:
> Since this file is already including , instead of `char`, `unsigned 
> char`, `short`, `unsigned short`, etc., can these use `int8_t`, `uint8_t`, 
> `int16_t`, `uint16_t`, and so on, for clarity? Also, this would avoid any 
> possible behavior change under `-funsigned-char`.
Unfortunately not :( This change gives errors like

```
error: cannot initialize a parameter of type '__attribute__((__vector_size__(16 
* sizeof(char char' (vector of 16 'char' values) with an rvalue of type 
'__i8x16' (vector of 16 'int8_t' values)
  return (v128_t)__builtin_wasm_abs_i8x16((__i8x16)a);
```

Even changing `char` to `signed char` doesn't work. I would change the builtins 
to use the better types, but the builtin definition system does not allow those 
more interesting types.

This is especially annoying because `-funsigned-char` does indeed change the 
behavior of the comparison intrinsics. I will have to do extra work in those 
functions to make it work.



Comment at: clang/lib/Headers/wasm_simd128.h:40
+#define __REQUIRE_CONSTANT(e)  
\
+  _Static_assert(__builtin_constant_p(e), "Expected constant")
+

sunfish wrote:
> In clang's xmmintrin.h, helper macros like these `__DEFAULT_FN_ATTRS` and 
> `__REQUIRE_CONSTANT` are `#undef`ed at the end of the file.
I can do that for `__DEFAULT_FN_ATTRS` but unfortunately not for 
`__REQUIRE_CONSTANT` because `__REQUIRE_CONSTANT` is present in the macro 
expansion of `wasm_i8x16_const` and friends.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76959



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


[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

2020-03-30 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan added a comment.

In D76389#1951135 , @arsenm wrote:

> Commit message should say only if?


Updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76389



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


[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

2020-03-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:959-963
+  /// Indicates if the complete destructor has been implicitly declared
+  /// yet. Only relevant in the Microsoft C++.
+  bool definedImplicitCompleteDestructor() const {
+return data().DefinedImplicitCompleteDestructor;
+  }

"declared" in comment but "defined" in function name. Which is it?

I wonder if perhaps the right answer is neither: if we had separate `Decl`s for 
the complete and base destructors, this would be tracked by the "used" bit on 
the complete object destructor. So perhaps `isCompleteDestructorUsed` and 
`setCompleteDestructorUsed` would make sense? (We could consider tracking this 
on the destructor instead of here, but I suppose tracking it here gives us the 
serialization and merging logic for free.)



Comment at: clang/include/clang/Sema/Sema.h:5562-5565
+  /// Define an implicit complete constructor for classes with vbases in the MS
+  /// ABI.
+  void DefineImplicitCompleteDestructor(SourceLocation CurrentLocation,
+CXXDestructorDecl *Dtor);

Following the prior comment, naming this `MarkCompleteDestructorUsed` might 
make sense.



Comment at: clang/lib/Sema/SemaExpr.cpp:16008-16013
+// In the MS ABI, the complete destructor is implicitly defined,
+// even if the base destructor is user defined.
+CXXRecordDecl *Parent = Destructor->getParent();
+if (Parent->getNumVBases() > 0 &&
+!Parent->definedImplicitCompleteDestructor())
+  DefineImplicitCompleteDestructor(Loc, Destructor);

Can we avoid doing this when we know the call is to a base subobject destructor 
or uses virtual dispatch? Should we? (I mean I guess ideally we'd change this 
function to take a `GlobalDecl` instead of a `FunctionDecl*`, but that seems 
like a lot of work.)

How does MSVC behave?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77081



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


[PATCH] D74541: [Analyzer] Use note tags to track iterator increments and decrements

2020-03-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/test/Analysis/iterator-modelling.cpp:434
+  //expected-note@-1 0-1{{Calling 'next}}
+  //expected-note@-2 0-1{{Passing the value 1 via 2nd parameter 'n'}}
+  //expected-note@Inputs/system-header-simulator-cxx.h:814 0-1{{Iterator 'it' 
incremented by 1}}

baloghadamsoftware wrote:
> Strange, that we do not get this note for `prev`.
Mmm, why `0-1`?


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

https://reviews.llvm.org/D74541



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


[PATCH] D76389: [NewPM] Run the Speculative Execution Pass if the target has divergent branches

2020-03-30 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

Commit message should say only if?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76389



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


[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

2020-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

PTAL, here's how I imagine this is supposed to look, but the definition data 
bit name could probably be improved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77081



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


[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

2020-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 253715.
rnk added a comment.

- add def data bit
- add tests
- fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77081

Files:
  clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/class.access/p4.cpp
  clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp
  clang/test/SemaCXX/ms-implicit-complete-dtor.cpp

Index: clang/test/SemaCXX/ms-implicit-complete-dtor.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ms-implicit-complete-dtor.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -std=c++17 -verify -Wno-defaulted-function-deleted %s -triple x86_64-windows-msvc
+
+// MSVC emits the complete destructor as if it were its own special member.
+// Clang attempts to do the same. This affects the diagnostics clang emits,
+// because deleting a type with a user declared constructor implicitly
+// references the destructors of virtual bases, which might be deleted or access
+// controlled.
+
+namespace t1 {
+struct A {
+  ~A() = delete; // expected-note {{deleted here}}
+};
+struct B { // expected-error {{attempt to use a deleted function}}
+  B() = default;
+  A o; // expected-note {{destructor of 'B' is implicitly deleted because field 'o' has a deleted destructor}}
+};
+struct C : virtual B {
+  ~C();
+};
+void delete1(C *p) { delete p; } // expected-note {{in implicit destructor for 't1::C' first required here}}
+void delete2(C *p) { delete p; }
+}
+
+namespace t2 {
+struct A {
+private:
+  ~A();
+};
+struct B { // expected-error {{attempt to use a deleted function}}
+  B() = default;
+  A o; // expected-note {{destructor of 'B' is implicitly deleted because field 'o' has an inaccessible destructor}}
+};
+struct C : virtual B {
+  ~C();
+};
+void useCompleteDtor(C *p) { delete p; } // expected-note {{in implicit destructor for 't2::C' first required here}}
+}
+
+namespace t3 {
+template 
+class Base { ~Base(); }; // expected-note 1{{declared private here}}
+// No diagnostic.
+class Derived0 : virtual Base<0> { ~Derived0(); };
+class Derived1 : virtual Base<1> {};
+// Emitting complete dtor causes a diagnostic.
+struct Derived2 : // expected-error {{inherited virtual base class 'Base<2>' has private destructor}}
+  virtual Base<2> {
+  ~Derived2();
+};
+void useCompleteDtor(Derived2 *p) { delete p; } // expected-note {{in implicit destructor for 't3::Derived2' first required here}}
+}
Index: clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -std=c++17 -emit-llvm %s -triple x86_64-windows-msvc -o - | FileCheck %s
+
+// Make sure virtual base base destructors get referenced and emitted if
+// necessary when the complete ("vbase") destructor is emitted. In this case,
+// clang previously did not emit ~DefaultedDtor.
+struct HasDtor { ~HasDtor(); };
+struct DefaultedDtor {
+  ~DefaultedDtor() = default;
+  HasDtor o;
+};
+struct HasCompleteDtor : virtual DefaultedDtor {
+  ~HasCompleteDtor();
+};
+void useCompleteDtor(HasCompleteDtor *p) { delete p; }
+
+// CHECK-LABEL: define dso_local void @"?useCompleteDtor@@YAXPEAUHasCompleteDtor@@@Z"(%struct.HasCompleteDtor* %p)
+// CHECK: call void @"??_DHasCompleteDtor@@QEAAXXZ"({{.*}})
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"??_DHasCompleteDtor@@QEAAXXZ"(%struct.HasCompleteDtor* %this)
+// CHECK: call void @"??1HasCompleteDtor@@QEAA@XZ"({{.*}})
+// CHECK: call void @"??1DefaultedDtor@@QEAA@XZ"({{.*}})
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"??1DefaultedDtor@@QEAA@XZ"(%struct.DefaultedDtor* %this)
+// CHECK: call void @"??1HasDtor@@QEAA@XZ"(%struct.HasDtor* %{{.*}})
+
Index: clang/test/CXX/class.access/p4.cpp
===
--- clang/test/CXX/class.access/p4.cpp
+++ clang/test/CXX/class.access/p4.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++11 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify %s
 
 // C++0x [class.access]p4:
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ 

[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

2020-03-30 Thread Thomas Lively via Phabricator via cfe-commits
tlively updated this revision to Diff 253713.
tlively added a comment.

- Use header guard macro instead of pragma


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76959

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/wasm_simd128.h

Index: clang/lib/Headers/wasm_simd128.h
===
--- /dev/null
+++ clang/lib/Headers/wasm_simd128.h
@@ -0,0 +1,1145 @@
+/*=== wasm_simd128.h - WebAssembly portable SIMD intrinsics ===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __WASM_SIMD128_H
+#define __WASM_SIMD128_H
+
+#include 
+#include 
+
+// User-facing type
+typedef int32_t v128_t __attribute__((__vector_size__(16), __aligned__(16)));
+
+// Internal types determined by clang builtin definitions
+typedef int32_t __v128_u __attribute__((__vector_size__(16), __aligned__(1)));
+typedef char __i8x16 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef signed char __s8x16
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned char __u8x16
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef short __i16x8 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned short __u16x8
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef int __i32x4 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned int __u32x4
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef long long __i64x2 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef float __f32x4 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef double __f64x2 __attribute__((__vector_size__(16), __aligned__(16)));
+
+#define __DEFAULT_FN_ATTRS \
+  __attribute__((__always_inline__, __nodebug__, __target__("simd128"),\
+ __min_vector_width__(128)))
+
+#define __REQUIRE_CONSTANT(e)  \
+  _Static_assert(__builtin_constant_p(e), "Expected constant")
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_v128_load(const void *__mem) {
+  // UB-free unaligned access copied from xmmintrin.h
+  struct __wasm_v128_load_struct {
+__v128_u __v;
+  } __attribute__((__packed__, __may_alias__));
+  return ((const struct __wasm_v128_load_struct *)__mem)->__v;
+}
+
+#ifdef __wasm_unimplemented_simd128__
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v8x16_load_splat(const void *__mem) {
+  struct __wasm_v8x16_load_splat_struct {
+uint8_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint8_t __v = ((const struct __wasm_v8x16_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u8x16){__v, __v, __v, __v, __v, __v, __v, __v,
+   __v, __v, __v, __v, __v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v16x8_load_splat(const void *__mem) {
+  struct __wasm_v16x8_load_splat_struct {
+uint16_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint16_t __v = ((const struct __wasm_v16x8_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u16x8){__v, __v, __v, __v, __v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v32x4_load_splat(const void *__mem) {
+  struct __wasm_v32x4_load_splat_struct {
+uint32_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint32_t __v = ((const struct __wasm_v32x4_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u32x4){__v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v64x2_load_splat(const void *__mem) {
+  struct __wasm_v64x2_load_splat_struct {
+uint64_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint64_t __v = ((const struct __wasm_v64x2_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u64x2){__v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_i16x8_load_8x8(const void *__mem) {
+  typedef int8_t __i8x8 __attribute__((__vector_size__(8), __aligned__(8)));
+  struct __wasm_i16x8_load_8x8_struct {
+__i8x8 __v;
+  } __attribute__((__packed__, __may_alias__));
+  __i8x8 __v = ((const struct __wasm_i16x8_load_8x8_struct *)__mem)->__v;
+  return (v128_t) __builtin_convertvector(__v, __i16x8);
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_u16x8_load_8x8(const void *__mem) {
+  typedef uint8_t __u8x8 __attribute__((__vector_size__(8), __aligned__(8)));
+  struct __wasm_u16x8_load_8x8_struct {
+__u8x8 __v;
+  } __attribute__((__packed__, __may_alias__));
+  __u8x8 __v = ((const struct 

[PATCH] D77077: [clang] CodeGen: Make getOrEmitProtocol public for Swift

2020-03-30 Thread Arnold Schwaighofer via Phabricator via cfe-commits
aschwaighofer updated this revision to Diff 253712.
aschwaighofer added a comment.

- Change API name to emitObjCProtocolObject.
- Make stuff compile on current ToT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77077

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.cpp
  clang/lib/CodeGen/CGObjCRuntime.h

Index: clang/lib/CodeGen/CGObjCRuntime.h
===
--- clang/lib/CodeGen/CGObjCRuntime.h
+++ clang/lib/CodeGen/CGObjCRuntime.h
@@ -211,6 +211,16 @@
   /// implementations.
   virtual void GenerateProtocol(const ObjCProtocolDecl *OPD) = 0;
 
+  /// getOrEmitProtocol - Get the protocol object for the given
+  /// declaration, emitting it if necessary. The return value has type
+  /// ProtocolPtrTy.
+  /// \p getProtocolReference is called by the implementation when a
+  /// reference to another protocol is needed.
+  virtual llvm::Constant *getOrEmitProtocol(
+  const ObjCProtocolDecl *OPD,
+  llvm::function_ref
+  getProtocolReference) = 0;
+
   /// Generate a function preamble for a method with the specified
   /// types.
 
Index: clang/lib/CodeGen/CGObjCRuntime.cpp
===
--- clang/lib/CodeGen/CGObjCRuntime.cpp
+++ clang/lib/CodeGen/CGObjCRuntime.cpp
@@ -13,14 +13,15 @@
 //===--===//
 
 #include "CGObjCRuntime.h"
-#include "CGCleanup.h"
 #include "CGCXXABI.h"
+#include "CGCleanup.h"
 #include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
@@ -383,3 +384,11 @@
 CGM.getTypes().GetFunctionType(argsInfo)->getPointerTo();
   return MessageSendInfo(argsInfo, signatureType);
 }
+
+llvm::Constant *clang::CodeGen::emitObjCProtocolObject(
+CodeGenModule , const ObjCProtocolDecl *protocol,
+llvm::function_ref
+createProtocolReference) {
+  return CGM.getObjCRuntime().getOrEmitProtocol(protocol,
+createProtocolReference);
+}
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1110,7 +1110,7 @@
   /// GetOrEmitProtocol - Get the protocol object for the given
   /// declaration, emitting it if necessary. The return value has type
   /// ProtocolPtrTy.
-  virtual llvm::Constant *GetOrEmitProtocol(const ObjCProtocolDecl *PD)=0;
+  llvm::Constant *GetOrEmitProtocol(const ObjCProtocolDecl *PD);
 
   /// GetOrEmitProtocolRef - Get a forward reference to the protocol
   /// object for the given declaration, emitting it if needed. These
@@ -1288,10 +1288,15 @@
   llvm::Constant *emitMethodList(Twine Name, MethodListType MLT,
  ArrayRef Methods);
 
-  /// GetOrEmitProtocol - Get the protocol object for the given
+  /// getOrEmitProtocol - Get the protocol object for the given
   /// declaration, emitting it if necessary. The return value has type
   /// ProtocolPtrTy.
-  llvm::Constant *GetOrEmitProtocol(const ObjCProtocolDecl *PD) override;
+  /// \p getProtocolReference is called by the implementation when a
+  /// reference to another protocol is needed.
+  llvm::Constant *getOrEmitProtocol(
+  const ObjCProtocolDecl *OPD,
+  llvm::function_ref
+  getProtocolReference) override;
 
   /// GetOrEmitProtocolRef - Get a forward reference to the protocol
   /// object for the given declaration, emitting it if needed. These
@@ -1309,9 +1314,11 @@
 
   /// EmitProtocolList - Generate the list of referenced
   /// protocols. The return value has type ProtocolListPtrTy.
-  llvm::Constant *EmitProtocolList(Twine Name,
-   ObjCProtocolDecl::protocol_iterator begin,
-   ObjCProtocolDecl::protocol_iterator end);
+  llvm::Constant *EmitProtocolList(
+  Twine Name, ObjCProtocolDecl::protocol_iterator begin,
+  ObjCProtocolDecl::protocol_iterator end,
+  llvm::function_ref
+  getProtocolRef);
 
   /// EmitSelector - Return a Value*, of type ObjCTypes.SelectorPtrTy,
   /// for the given selector.
@@ -1471,10 +1478,15 @@
 const ObjCIvarDecl *Ivar,
 unsigned long int offset);
 
-  /// GetOrEmitProtocol - Get the protocol object for the given
+  /// getOrEmitProtocol - Get the protocol object for the given
   /// declaration, emitting it if necessary. The return value has type
   /// ProtocolPtrTy.
-  

[PATCH] D77077: [clang] CodeGen: Make getOrEmitProtocol public for Swift

2020-03-30 Thread Arnold Schwaighofer via Phabricator via cfe-commits
aschwaighofer marked an inline comment as done.
aschwaighofer added inline comments.



Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:148
+ llvm::function_ref
+ createProtocolReference);
 }  // end namespace CodeGen

rjmccall wrote:
> I would call this `emitObjCProtocolObject` or something, and maybe say in the 
> comment:
> 
> > Get a pointer to a protocol object for the given declaration, emitting it 
> > if it hasn't already been emitted in this translation unit.  Note that the 
> > ABI for emitting a protocol reference in code (e.g. for a `@protocol` 
> > expression) in most runtimes is not as simple as just materializing a 
> > pointer to this object.
> 
> Can you explain the need for the callback?   Are you expecting to use this 
> for Swift-declared protocols by synthesizing an ObjC protocol declaration for 
> them?  I can see why you'd need a callback in that case.
> Can you explain the need for the callback? Are you expecting to use this for 
> Swift-declared protocols by synthesizing an ObjC protocol declaration for 
> them? I can see why you'd need a callback in that case.

The objective C protocol references other protocols in its inherited list. 
Swift has an internal list that keeps track of those protocols references and 
makes sure to initialized them for the debugger and also makes sure that the 
underlying protocol structure is emitted. The call back is to keep this list in 
sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77077



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


[PATCH] D76140: [InlineFunction] update attributes during inlining

2020-03-30 Thread Anna Thomas via Phabricator via cfe-commits
anna marked 3 inline comments as done.
anna added inline comments.



Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1175
+  continue;
+// Sanity check that the cloned return instruction exists and is a return
+// instruction itself.

anna wrote:
> reames wrote:
> > Ok, after staring at it a bit, I've convinced myself the code here is 
> > correct, just needlessly conservative.
> > 
> > What you're doing is:
> > If the callees return instruction and returned call both map to the same 
> > instructions once inlined, determine whether there's a possible exit 
> > between the inlined copy.
> > 
> > What you could be doing:
> > If the callee returns a call, check if *in the callee* there's a possible 
> > exit between call and return, then apply attribute to cloned call.
> > 
> > The key difference is when the caller directly returns the result vs uses 
> > it locally.  The result here is that your transform is much more narrow in 
> > applicability than it first appears.
> yes, thanks for pointing it out. I realized it after our offline discussion 
> :) 
> For now, I will add a FIXME testcase which showcases the difference in code 
> and handle that testcase in a followon change. 
> The key difference is when the caller directly returns the result vs uses it 
> locally. The result here is that your transform is much more narrow in 
> applicability than it first appears.
I tried multiple test cases to showcase the difference between the two ideas 
above but failed. Specifically, `simplifyInstruction` used during inlining the 
callee is not too great at optimizing the body. For example, see added testcase 
`test7`. 

I also tried the less restrictive version (check the safety of the optimization 
in the callee itself, and do the attribute update on the cloned instruction), 
but didn't see any testcases in clang that needed update. Of course, that 
doesn't mean anything :)





Comment at: llvm/test/Transforms/Inline/ret_attr_update.ll:112
+  ret i8* %s
+}

anna wrote:
> reames wrote:
> > There's a critical missing test case here:
> > - Callee and caller have the same attributes w/different values (i.e. deref)
> > 
> > And thinking through the code, I think there might be a bug here.  It's not 
> > a serious one, but the if the callee specifies a larger deref than the 
> > caller, it looks like the the smaller value is being written over the 
> > larger.
> > 
> > Actually, digging through the attribute code, I think I'm wrong about the 
> > bug.  However, you should definitely write the test to confirm and document 
> > merging behaviour!
> > 
> > If it does turn out I'm correct, I'm fine with this being addressed in a 
> > follow up patch provided that the test is added in this one and isn't a 
> > functional issue.  
> will check this.
added test case and documented merge behaviour. No bug in code, since we use 
the already existing value on attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76140



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


[PATCH] D76140: [InlineFunction] update attributes during inlining

2020-03-30 Thread Anna Thomas via Phabricator via cfe-commits
anna updated this revision to Diff 253704.
anna added a comment.

addressed review comments. Added two test cases: deref value being different, 
inlined callee body better optimized compared to callee.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76140

Files:
  clang/test/CodeGen/builtins-systemz-zvector.c
  clang/test/CodeGen/builtins-systemz-zvector2.c
  clang/test/CodeGen/movbe-builtins.c
  clang/test/CodeGen/rot-intrinsics.c
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Transforms/Inline/ret_attr_update.ll

Index: llvm/test/Transforms/Inline/ret_attr_update.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/ret_attr_update.ll
@@ -0,0 +1,159 @@
+; RUN: opt < %s -inline-threshold=0 -always-inline -S | FileCheck %s
+; RUN: opt < %s -passes=always-inline -S | FileCheck %s
+
+declare i8* @foo(i8*) argmemonly nounwind
+
+define i8* @callee(i8 *%p) alwaysinline {
+; CHECK: @callee(
+; CHECK: call i8* @foo(i8* noalias %p)
+  %r = call i8* @foo(i8* noalias %p)
+  ret i8* %r
+}
+
+define i8* @caller(i8* %ptr, i64 %x) {
+; CHECK-LABEL: @caller
+; CHECK: call nonnull i8* @foo(i8* noalias
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call nonnull i8* @callee(i8* %gep)
+  ret i8* %p
+}
+
+declare void @llvm.experimental.guard(i1,...)
+; Cannot add nonnull attribute to foo
+; because the guard is a throwing call
+define internal i8* @callee_with_throwable(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_throwable
+  %r = call i8* @foo(i8* %p)
+  %cond = icmp ne i8* %r, null
+  call void (i1, ...) @llvm.experimental.guard(i1 %cond) [ "deopt"() ]
+  ret i8* %r
+}
+
+declare i8* @bar(i8*) readonly nounwind
+; Here also we cannot add nonnull attribute to the call bar.
+define internal i8* @callee_with_explicit_control_flow(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_explicit_control_flow
+  %r = call i8* @bar(i8* %p)
+  %cond = icmp ne i8* %r, null
+  br i1 %cond, label %ret, label %orig
+
+ret:
+  ret i8* %r
+
+orig:
+  ret i8* %p
+}
+
+define i8* @caller2(i8* %ptr, i64 %x, i1 %cond) {
+; CHECK-LABEL: @caller2
+; CHECK: call i8* @foo
+; CHECK: call i8* @bar
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call nonnull i8* @callee_with_throwable(i8* %gep)
+  %q = call nonnull i8* @callee_with_explicit_control_flow(i8* %gep)
+  br i1 %cond, label %pret, label %qret
+
+pret:
+  ret i8* %p
+
+qret:
+  ret i8* %q
+}
+
+define internal i8* @callee3(i8 *%p) alwaysinline {
+; CHECK-NOT: callee3
+  %r = call noalias i8* @foo(i8* %p)
+  ret i8* %r
+}
+
+; add the deref attribute to the existing attributes on foo.
+define i8* @caller3(i8* %ptr, i64 %x) {
+; CHECK-LABEL: caller3
+; CHECK: call noalias dereferenceable_or_null(12) i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call dereferenceable_or_null(12) i8* @callee3(i8* %gep)
+  ret i8* %p
+}
+
+declare i8* @inf_loop_call(i8*) nounwind
+; We cannot propagate attributes to foo because we do not know whether inf_loop_call
+; will return execution.
+define internal i8* @callee_with_sideeffect_callsite(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_sideeffect_callsite
+  %r = call i8* @foo(i8* %p)
+  %v = call i8* @inf_loop_call(i8* %p)
+  ret i8* %r
+}
+
+; do not add deref attribute to foo
+define i8* @test4(i8* %ptr, i64 %x) {
+; CHECK-LABEL: test4
+; CHECK: call i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call dereferenceable_or_null(12) i8* @callee_with_sideeffect_callsite(i8* %gep)
+  ret i8* %p
+}
+
+declare i8* @baz(i8*) nounwind readonly
+define internal i8* @callee5(i8* %p) alwaysinline {
+; CHECK-NOT: callee5
+  %r = call i8* @foo(i8* %p)
+  %v = call i8* @baz(i8* %p)
+  ret i8* %r
+}
+
+; add the deref attribute to foo.
+define i8* @test5(i8* %ptr, i64 %x) {
+; CHECK-LABEL: test5
+; CHECK: call dereferenceable_or_null(12) i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %s = call dereferenceable_or_null(12) i8* @callee5(i8* %gep)
+  ret i8* %s
+}
+
+; deref attributes have different values on the callee and the call feeding into
+; the return.
+; AttrBuilder chooses the already existing value and does not overwrite it.
+define internal i8* @callee6(i8* %p) alwaysinline {
+; CHECK-NOT: callee6
+  %r = call dereferenceable_or_null(16) i8* @foo(i8* %p)
+  %v = call i8* @baz(i8* %p)
+  ret i8* %r
+}
+
+
+define i8* @test6(i8* %ptr, i64 %x) {
+; CHECK-LABEL: test6
+; CHECK: call dereferenceable_or_null(16) i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %s = call dereferenceable_or_null(12) i8* @callee6(i8* %gep)
+  ret i8* %s
+}
+
+; We add the attributes from the callee to both the calls below.
+define internal i8* @callee7(i8 *%ptr, i1 %cond) alwaysinline {
+; CHECK-NOT: @callee7(
+  br i1 %cond, label %pass, label %fail
+
+pass:
+  %r = call i8* @foo(i8* noalias %ptr)
+  ret i8* %r
+

[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

2020-03-30 Thread Thomas Lively via Phabricator via cfe-commits
tlively updated this revision to Diff 253703.
tlively marked 3 inline comments as done.
tlively added a comment.

- Prefix identifiers with `__`
- Use specific bit-widths where possible
- Do not change semantics under -funsigned-char


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76959

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/wasm_simd128.h

Index: clang/lib/Headers/wasm_simd128.h
===
--- /dev/null
+++ clang/lib/Headers/wasm_simd128.h
@@ -0,0 +1,1142 @@
+/*=== wasm_simd128.h - WebAssembly portable SIMD intrinsics ===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#pragma once
+
+#include 
+#include 
+
+// User-facing type
+typedef int32_t v128_t __attribute__((__vector_size__(16), __aligned__(16)));
+
+// Internal types determined by clang builtin definitions
+typedef int32_t __v128_u __attribute__((__vector_size__(16), __aligned__(1)));
+typedef char __i8x16 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef signed char __s8x16
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned char __u8x16
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef short __i16x8 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned short __u16x8
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef int __i32x4 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned int __u32x4
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef long long __i64x2 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));
+typedef float __f32x4 __attribute__((__vector_size__(16), __aligned__(16)));
+typedef double __f64x2 __attribute__((__vector_size__(16), __aligned__(16)));
+
+#define __DEFAULT_FN_ATTRS \
+  __attribute__((__always_inline__, __nodebug__, __target__("simd128"),\
+ __min_vector_width__(128)))
+
+#define __REQUIRE_CONSTANT(e)  \
+  _Static_assert(__builtin_constant_p(e), "Expected constant")
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_v128_load(const void *__mem) {
+  // UB-free unaligned access copied from xmmintrin.h
+  struct __wasm_v128_load_struct {
+__v128_u __v;
+  } __attribute__((__packed__, __may_alias__));
+  return ((const struct __wasm_v128_load_struct *)__mem)->__v;
+}
+
+#ifdef __wasm_unimplemented_simd128__
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v8x16_load_splat(const void *__mem) {
+  struct __wasm_v8x16_load_splat_struct {
+uint8_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint8_t __v = ((const struct __wasm_v8x16_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u8x16){__v, __v, __v, __v, __v, __v, __v, __v,
+   __v, __v, __v, __v, __v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v16x8_load_splat(const void *__mem) {
+  struct __wasm_v16x8_load_splat_struct {
+uint16_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint16_t __v = ((const struct __wasm_v16x8_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u16x8){__v, __v, __v, __v, __v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v32x4_load_splat(const void *__mem) {
+  struct __wasm_v32x4_load_splat_struct {
+uint32_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint32_t __v = ((const struct __wasm_v32x4_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u32x4){__v, __v, __v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_v64x2_load_splat(const void *__mem) {
+  struct __wasm_v64x2_load_splat_struct {
+uint64_t __v;
+  } __attribute__((__packed__, __may_alias__));
+  uint64_t __v = ((const struct __wasm_v64x2_load_splat_struct *)__mem)->__v;
+  return (v128_t)(__u64x2){__v, __v};
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_i16x8_load_8x8(const void *__mem) {
+  typedef int8_t __i8x8 __attribute__((__vector_size__(8), __aligned__(8)));
+  struct __wasm_i16x8_load_8x8_struct {
+__i8x8 __v;
+  } __attribute__((__packed__, __may_alias__));
+  __i8x8 __v = ((const struct __wasm_i16x8_load_8x8_struct *)__mem)->__v;
+  return (v128_t) __builtin_convertvector(__v, __i16x8);
+}
+
+static __inline__ v128_t __DEFAULT_FN_ATTRS
+wasm_u16x8_load_8x8(const void *__mem) {
+  typedef uint8_t __u8x8 __attribute__((__vector_size__(8), __aligned__(8)));
+  struct __wasm_u16x8_load_8x8_struct {
+__u8x8 __v;
+  } 

[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

2020-03-30 Thread Dimitri John Ledkov via Phabricator via cfe-commits
xnox added a comment.

Sorry for being too slow =) I like all the fixes done to move the define into a 
more natural config.h location, thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75914



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


[PATCH] D77077: [clang] CodeGen: Make getOrEmitProtocol public for Swift

2020-03-30 Thread Arnold Schwaighofer via Phabricator via cfe-commits
aschwaighofer added a comment.

> Can you explain the need for the callback? Are you expecting to use this for 
> Swift-declared protocols by synthesizing an ObjC protocol declaration for 
> them? I can see why you'd need a callback in that case.

The objective C protocol references other protocols in its inherited list. 
Swift has an internal list that keeps track of those protocols references and 
makes sure to initialized them for the debugger and also makes sure that the 
underlying protocol structure is emitted. The call back is to keep this list in 
sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77077



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


[clang] a4f74f3 - [OPENMP50]Do not imply lvalue as base expression in array shaping

2020-03-30 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-03-30T17:07:08-04:00
New Revision: a4f74f377b7c42d38a230a19996e3a7fa5a328c7

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

LOG: [OPENMP50]Do not imply lvalue as base expression in array shaping
expression.

We should not assume that the base expression in the array shaping
operation is an lvalue of some form, it may be an rvalue.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/OpenMP/task_codegen.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 4b913607c1db..ae98433acb48 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -5364,14 +5364,12 @@ std::pair 
CGOpenMPRuntime::emitDependClause(
 continue;
   const Expr *E = Dependencies[I].second;
   const auto *OASE = dyn_cast(E);
-  LValue Addr;
+  llvm::Value *Addr;
   if (OASE) {
-const Expr *Base = OASE->getBase()->IgnoreParenImpCasts();
-Addr =
-CGF.EmitLoadOfPointerLValue(CGF.EmitLValue(Base).getAddress(CGF),
-
Base->getType()->castAs());
+const Expr *Base = OASE->getBase();
+Addr = CGF.EmitScalarExpr(Base);
   } else {
-Addr = CGF.EmitLValue(E);
+Addr = CGF.EmitLValue(E).getPointer(CGF);
   }
   llvm::Value *Size;
   QualType Ty = E->getType();
@@ -5390,8 +5388,7 @@ std::pair 
CGOpenMPRuntime::emitDependClause(
 CGF.EmitOMPArraySectionExpr(ASE, /*IsLowerBound=*/false);
 llvm::Value *UpAddr = CGF.Builder.CreateConstGEP1_32(
 UpAddrLVal.getPointer(CGF), /*Idx0=*/1);
-llvm::Value *LowIntPtr =
-CGF.Builder.CreatePtrToInt(Addr.getPointer(CGF), CGM.SizeTy);
+llvm::Value *LowIntPtr = CGF.Builder.CreatePtrToInt(Addr, CGM.SizeTy);
 llvm::Value *UpIntPtr = CGF.Builder.CreatePtrToInt(UpAddr, CGM.SizeTy);
 Size = CGF.Builder.CreateNUWSub(UpIntPtr, LowIntPtr);
   } else {
@@ -5410,9 +5407,8 @@ std::pair 
CGOpenMPRuntime::emitDependClause(
   // deps[i].base_addr = &;
   LValue BaseAddrLVal = CGF.EmitLValueForField(
   Base, *std::next(KmpDependInfoRD->field_begin(), BaseAddr));
-  CGF.EmitStoreOfScalar(
-  CGF.Builder.CreatePtrToInt(Addr.getPointer(CGF), CGF.IntPtrTy),
-  BaseAddrLVal);
+  CGF.EmitStoreOfScalar(CGF.Builder.CreatePtrToInt(Addr, CGF.IntPtrTy),
+BaseAddrLVal);
   // deps[i].len = sizeof();
   LValue LenLVal = CGF.EmitLValueForField(
   Base, *std::next(KmpDependInfoRD->field_begin(), Len));

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 44f51f1a432d..1a18eab72527 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4804,6 +4804,9 @@ ExprResult Sema::ActOnOMPArrayShapingExpr(Expr *Base, 
SourceLocation LParenLoc,
   ArrayRef Brackets) {
   if (Base->getType()->isPlaceholderType()) {
 ExprResult Result = CheckPlaceholderExpr(Base);
+if (Result.isInvalid())
+  return ExprError();
+Result = DefaultLvalueConversion(Result.get());
 if (Result.isInvalid())
   return ExprError();
 Base = Result.get();

diff  --git a/clang/test/OpenMP/task_codegen.c 
b/clang/test/OpenMP/task_codegen.c
index 9376c375d5a8..9e4b3b59d6d5 100644
--- a/clang/test/OpenMP/task_codegen.c
+++ b/clang/test/OpenMP/task_codegen.c
@@ -56,7 +56,6 @@ int main() {
   // CHECK: store i64 4, i64* [[SIZE_ADDR]],
   // CHECK: [[FLAGS_ADDR:%.+]] = getelementptr inbounds 
%struct.kmp_depend_info, %struct.kmp_depend_info* [[VLA0]], i{{.+}} 0, i{{.+}} 2
   // CHECK: store i8 1, i8* [[FLAGS_ADDR]],
-  // CHECK: [[B_ADDR:%.+]] = load i32*, i32** %{{.+}},
   // CHECK: [[A:%.+]] = load i32, i32* [[A_ADDR]],
   // CHECK: [[A_CAST:%.+]] = sext i32 [[A]] to i64
   // CHECK: [[SZ1:%.+]] = mul nuw i64 3, [[A_CAST]]
@@ -65,7 +64,7 @@ int main() {
   // CHECK: [[SZ:%.+]] = mul nuw i64 [[SZ1]], [[A_CAST]]
   // CHECK: [[VLA1:%.+]] = getelementptr %struct.kmp_depend_info, 
%struct.kmp_depend_info* [[VLA]], i64 1
   // CHECK: [[BASE_ADDR:%.+]] = getelementptr inbounds 
%struct.kmp_depend_info, %struct.kmp_depend_info* [[VLA1]], i{{.+}} 0, i{{.+}} 0
-  // CHECK: [[B_ADDR_CAST:%.+]] = ptrtoint i32* [[B_ADDR]] to i64
+  // CHECK: [[B_ADDR_CAST:%.+]] = ptrtoint i32** %{{.+}} to i64
   // CHECK: store i64 [[B_ADDR_CAST]], i64* [[BASE_ADDR]],
   // CHECK: [[SIZE_ADDR:%.+]] = getelementptr inbounds 
%struct.kmp_depend_info, %struct.kmp_depend_info* [[VLA1]], i{{.+}} 0, i{{.+}} 1
   // CHECK: store i64 [[SZ]], i64* [[SIZE_ADDR]],
@@ -85,7 +84,7 @@ int main() {
   // 

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-03-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

This pretty much nulls out  (clang-tidy) Warn on invalid "case" configurations 
for readability-identifier-naming  I have 
moved that into a follow up patch that can be submitted now if you'd like.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77085



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


[PATCH] D76937: Fix infinite recursion in deferred diagnostic emitter

2020-03-30 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D76937#1950077 , @rjmccall wrote:

> Can you explain what exactly the emission/semantic model is for variables?  
> Normal code-generation absolutely triggers the emission of many variables 
> lazily (e.g. internal-linkage globals, C++ inline variables); and any 
> variable that's *not* being emitted lazily actually needs to be treated as a 
> potential root into the delayed-diagnostic graph.


Currently only in the following case a global variable can change the emission 
state of a function:

  int foobar3() { return 1; }
  #pragma omp declare target
  int (*C)() = 
  #pragma omp end declare target

The global variable needs to be in a omp declare target directive. Its 
initializer references a host function.

This will cause the function emitted in device compilation.

This is not transitive for variable declaration/references, e.g. the following 
case will not cause foobar3 to be emitted in device compilation, unless 
variable C itself is enclosed in omp declare target directive.

  int foobar3() { return 1; }
  int (*C)() = 
  #pragma omp declare target
  int (*D)() = C;
  #pragma omp end declare target

Since we logged all such variable declarations in DeclsToCheckForDeferredDiags, 
we only need to check variables decls in DeclsToCheckForDeferredDiags and do 
not need to check variable decls in DeclRefExpr.


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

https://reviews.llvm.org/D76937



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


[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-03-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
njames93 added a comment.
njames93 added a project: clang-tools-extra.

This pretty much nulls out  (clang-tidy) Warn on invalid "case" configurations 
for readability-identifier-naming  I have 
moved that into a follow up patch that can be submitted now if you'd like.


Adds support for `ClangTidyCheck::OptionsView` to deteremine:

- If an option is found in the configuration.
- If an integer option read from configuration is parsable to an integer.
- Parse and Serialize enum configuration options directly using a mapping from 
`llvm::StringRef` to `EnumType`.
- If an integer or enum option isn't parseable but there is a default value it 
will issue a warning to stderr that the config value hasn't been used.
- If an enum option isn't parsable it can provide a hint if the value was a 
typo.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77085

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -1,6 +1,8 @@
 #include "ClangTidyOptions.h"
-#include "gtest/gtest.h"
+#include "ClangTidyCheck.h"
+#include "ClangTidyDiagnosticConsumer.h"
 #include "llvm/ADT/StringExtras.h"
+#include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
@@ -97,6 +99,154 @@
 llvm::join(Options.ExtraArgsBefore->begin(),
Options.ExtraArgsBefore->end(), ","));
 }
+
+class TestCheck : public ClangTidyCheck {
+public:
+  TestCheck(ClangTidyContext *Context) : ClangTidyCheck("test", Context) {}
+
+  template  auto getLocal(Args &&... Arguments) {
+return Options.get(std::forward(Arguments)...);
+  }
+
+  template  auto getGlobal(Args &&... Arguments) {
+return Options.getLocalOrGlobal(std::forward(Arguments)...);
+  }
+
+  template 
+  auto getIntLocal(Args &&... Arguments) {
+return Options.get(std::forward(Arguments)...);
+  }
+
+  template 
+  auto getIntGlobal(Args &&... Arguments) {
+return Options.getLocalOrGlobal(std::forward(Arguments)...);
+  }
+};
+
+TEST(CheckOptionsValidation, MissingOptions) {
+  ClangTidyOptions Options;
+  ClangTidyContext Context(std::make_unique(
+  ClangTidyGlobalOptions(), Options));
+  TestCheck TestCheck();
+  EXPECT_TRUE(TestCheck.getLocal("Opt").errorIsA());
+  EXPECT_EQ(TestCheck.getLocal("Opt", "Unknown"), "Unknown");
+}
+
+#define CHECK_VAL(Value, Expected) \
+  do { \
+auto Item = Value; \
+ASSERT_TRUE(!!Item);   \
+EXPECT_EQ(*Item, Expected);\
+  } while (false)
+
+#define CHECK_ERROR(Value, ErrorType, ExpectedMessage) \
+  do { \
+auto Item = Value; \
+ASSERT_FALSE(Item);\
+ASSERT_TRUE(Item.errorIsA());   \
+ASSERT_FALSE(llvm::handleErrors(   \
+Item.takeError(), [&](const ErrorType ) -> llvm::Error {   \
+  EXPECT_EQ(Err.message(), ExpectedMessage);   \
+  return llvm::Error::success();   \
+}));   \
+  } while (false)
+
+TEST(CheckOptionsValidation, ValidIntOptions) {
+  ClangTidyOptions Options;
+  auto  = Options.CheckOptions;
+  CheckOptions["test.IntExpected1"] = "1";
+  CheckOptions["test.IntExpected2"] = "1WithMore";
+  CheckOptions["test.IntExpected3"] = "NoInt";
+  CheckOptions["GlobalIntExpected1"] = "1";
+  CheckOptions["GlobalIntExpected2"] = "NoInt";
+  CheckOptions["test.DefaultedIntInvalid"] = "NoInt";
+  CheckOptions["GlobalIntInvalid"] = "NoInt";
+
+  ClangTidyContext Context(std::make_unique(
+  ClangTidyGlobalOptions(), Options));
+  TestCheck TestCheck();
+
+#define CHECK_ERROR_INT(Name, Expected)\
+  CHECK_ERROR(Name, UnparseableIntegerOptionError, Expected)
+
+  CHECK_VAL(TestCheck.getIntLocal("IntExpected1"), 1);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalIntExpected1"), 1);
+  

[PATCH] D75903: [AArch64][CodeGen] Fixing stack alignment of HFA arguments on AArch64 PCS

2020-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D75903#1914715 , @ostannard wrote:

> This means that `stk1` should be passed as a copy with alignment 16 bytes, 
> putting it at `sp+16`. GCC does this, clang without this patch passes it at 
> `sp+8`, and clang with this patch passes it at `sp+32`.


MSVC puts it at `sp+8`: https://gcc.godbolt.org/z/aAku9j

They allegedly follow this same document. Either way, we need to remain 
compatible.

Given that neither GCC nor MSVC do things they way you are proposing, are you 
sure this is correct? What compiler does this change make us more compatible 
with?

I don't believe there is an issue from the user's standpoint, since Clang 
copies the entire argument into an appropriately aligned alloca. If it is 
address-taken and stored to, there will not be any faults.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75903



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


[PATCH] D77048: [Clang][CodeGen] Fixing mismatch between memory layout and const expressions for oversized bitfields

2020-03-30 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:613
+  
+  // Add padding bits in case of over-sized bit-field.
+  //   "The first sizeof(T)*8 bits are used to hold the value of the bit-field,

The existing code in ConstantAggregateBuilder::add should skip over padding.  
I'm not sure what you're trying to accomplish by adding more code dealing with 
padding here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77048



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


[clang] ced99a1 - Fix comment for CLANG_SYSTEMZ_DEFAULT_ARCH

2020-03-30 Thread Jonas Hahnfeld via cfe-commits

Author: Jonas Hahnfeld
Date: 2020-03-30T21:36:18+02:00
New Revision: ced99a1a6368b117384935957e2329ec4ba7784b

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

LOG: Fix comment for CLANG_SYSTEMZ_DEFAULT_ARCH

Also move up, next to the other *_DEFAULT_* configurations.

Added: 


Modified: 
clang/include/clang/Config/config.h.cmake

Removed: 




diff  --git a/clang/include/clang/Config/config.h.cmake 
b/clang/include/clang/Config/config.h.cmake
index a0f8b6b1b0da..26e9d5c4eb4d 100644
--- a/clang/include/clang/Config/config.h.cmake
+++ b/clang/include/clang/Config/config.h.cmake
@@ -35,6 +35,9 @@
 /* Default architecture for OpenMP offloading to Nvidia GPUs. */
 #define CLANG_OPENMP_NVPTX_DEFAULT_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}"
 
+/* Default architecture for SystemZ. */
+#define CLANG_SYSTEMZ_DEFAULT_ARCH "${CLANG_SYSTEMZ_DEFAULT_ARCH}"
+
 /* Multilib suffix for libdir. */
 #define CLANG_LIBDIR_SUFFIX "${CLANG_LIBDIR_SUFFIX}"
 
@@ -83,7 +86,4 @@
 /* Spawn a new process clang.exe for the CC1 tool invocation, when necessary */
 #cmakedefine01 CLANG_SPAWN_CC1
 
-/* Default  to all compiler invocations for --sysroot=. */
-#define CLANG_SYSTEMZ_DEFAULT_ARCH "${CLANG_SYSTEMZ_DEFAULT_ARCH}"
-
 #endif



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


[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

2020-03-30 Thread Vince Bridgers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdefd95ef4517: [analyzer] Fix StdLibraryFunctionsChecker 
NotNull Constraint Check (authored by vabridgers, committed by einvbri 
vince.a.bridg...@ericsson.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77012

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions.c


Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,14 @@
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+void test_fread_uninitialized(void) {
+  void *ptr;
+  size_t sz;
+  size_t nmem;
+  FILE *fp;
+  (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call 
argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@
 ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
   const Summary ) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;


Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,14 @@
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+void test_fread_uninitialized(void) {
+  void *ptr;
+  size_t sz;
+  size_t nmem;
+  FILE *fp;
+  (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@
 ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
   const Summary ) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

2020-03-30 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

This is needed for OpenMP as well. Does it make sense to include it in this 
patch or in another one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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


[PATCH] D77056: RFC: [Sema][SVE] Allow non-member operators for SVE types

2020-03-30 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> One option would to be wait until there's at least one type that 
> isSizelessBuiltinType but isn't an SVE type. Another would be to introduce it 
> when a feature seems too SVE-specific

Probably not a big deal either way, as long as we document the intent in the 
code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77056



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


[PATCH] D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract

2020-03-30 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I can understand why you might want the new intrinsics as a temporary measure, 
but I don't see the point of removing the already working support for 
llvm.sadd. etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77054



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


[PATCH] D76987: Rename options --cuda-gpu-arch and --no-cuda-gpu-arch

2020-03-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a reviewer: echristo.
tra accepted this revision.
tra added a subscriber: echristo.
tra added a comment.
This revision is now accepted and ready to land.

+ @echristo who OK'ed the idea conditional on the actual patch. :-)

LGTM overall.


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

https://reviews.llvm.org/D76987



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


[PATCH] D77070: [SemaObjC] Add a warning for declarations of BOOL:1 when BOOL is a signed char

2020-03-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

This feels like it's going to bite a lot of people, but maybe it's the best 
option.


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

https://reviews.llvm.org/D77070



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


[PATCH] D76594: [clang][AST] Support AST files larger than 512M

2020-03-30 Thread Dmitry Polukhin via Phabricator via cfe-commits
DmitryPolukhin marked an inline comment as done.
DmitryPolukhin added inline comments.



Comment at: clang/include/clang/Serialization/ASTBitCodes.h:220
 /// Source range/offset of a preprocessed entity.
 struct DeclOffset {
+  /// Raw source location. The unsigned i.e. 32-bit integer is enough for

sammccall wrote:
> Is there one of these for every decl in the module? It seems like we're 
> probably giving up a good fraction of the 4% increase for just using absolute 
> 64 bit offsets everywhere :-( Is there still a significant gain from using 
> section-relative elsewhere?
> 
> If there are indeed lots of these, giving up 4 bytes to padding (in addition 
> to the wide offset) seems unfortunate and because we memcpy the structs into 
> the AST file seems like a sad reason :-)
> Can we align this to 4 bytes instead?
> (e.g. by splitting into two fields and encapsulating the few direct accesses, 
> though there's probably a neater way)
Yes, it seems that all referenced decls should have an entry in this table. 4% 
increase was counted on previous diff before I discovered DeclOffsets and 
TypeOffsets. This diff uses relative offsets for previous cases and increase is 
only 2 additional 64-bit integers per file. DeclOffsets are about 4.7% and 
TypeOffsets are about 2.5% of the whole file. With this diff both number will 
be 2x, splitting DeclOffset in two separate arrays could make the increase only 
1.5x for DeclOffsets. If approach from this diff looks fine, I can split array 
of DeclOffset into 2 separate arrays. It could be a bit less efficient because 
of worse memory locality but will save some space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76594



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


[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

2020-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.
rnk added a reviewer: rsmith.
Herald added a project: clang.

DONOTSUBMIT: Uploading for feedback on approach, still have test failures:



Failing Tests (1):

  Clang :: CXX/class.access/p4.cpp

In the MS C++ ABI, complete destructors for classes with virtual bases
are emitted whereever they are needed. The complete destructor calls:

- the base dtor
- for each vbase, its base dtor

Even when a destructor is user-defined in another TU, clang needs to
mark the virtual base dtors referenced in TUs that call destructors.
This fixes PR38521.

FIXME: What about separating base and complete dtors? i.e. what about
when only base dtors are ref'd?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77081

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp

Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15998,10 +15998,19 @@
   } else if (CXXDestructorDecl *Destructor =
  dyn_cast(Func)) {
 Destructor = cast(Destructor->getFirstDecl());
-if (Destructor->isDefaulted() && !Destructor->isDeleted()) {
-  if (Destructor->isTrivial() && !Destructor->hasAttr())
-return;
-  DefineImplicitDestructor(Loc, Destructor);
+if (!Destructor->isDeleted()) {
+  if (Destructor->isDefaulted()) {
+if (Destructor->isTrivial() &&
+!Destructor->hasAttr())
+  return;
+DefineImplicitDestructor(Loc, Destructor);
+  } else if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+// In the MS ABI, the complete destructor is implicitly defined,
+// even if the base destructor is user defined.
+// FIXME: Need a way to do it only once. "setBody" seems wrong.
+if (Destructor->getParent()->getNumVBases() > 0)
+  DefineImplicitCompleteDestructor(Loc, Destructor);
+  }
 }
 if (Destructor->isVirtual() && getLangOpts().AppleKext)
   MarkVTableUsed(Loc, Destructor->getParent());
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13202,6 +13202,53 @@
   }
 }
 
+void Sema::DefineImplicitCompleteDestructor(SourceLocation CurrentLocation,
+CXXDestructorDecl *Destructor) {
+  if (Destructor->isInvalidDecl())
+return;
+
+  CXXRecordDecl *ClassDecl = Destructor->getParent();
+  assert(Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+ "implicit complete dtors unneeded outside MS ABI");
+  assert(ClassDecl->getNumVBases() > 0 &&
+ "complete dtor only exists for classes with vbases");
+
+  SynthesizedFunctionScope Scope(*this, Destructor);
+
+  // Add a context note for diagnostics produced after this point.
+  Scope.addContextNote(CurrentLocation);
+
+  // No need to resolve the exception specification of the base destructor or
+  // mark vtables referenced. That will be done when the base dtor is defined.
+
+  // Similar to what is done above in MarkBaseAndMemberDestructorsReferenced,
+  // but only for virtual bases.
+  for (const CXXBaseSpecifier  : ClassDecl->vbases()) {
+// Bases are always records in a well-formed non-dependent class.
+CXXRecordDecl *BaseRD = VBase.getType()->getAsCXXRecordDecl();
+
+// If our base class is invalid, we probably can't get its dtor anyway.
+if (!BaseRD || BaseRD->isInvalidDecl() || BaseRD->hasIrrelevantDestructor())
+  continue;
+
+CXXDestructorDecl *Dtor = LookupDestructor(BaseRD);
+assert(Dtor && "No dtor found for BaseRD!");
+if (CheckDestructorAccess(
+ClassDecl->getLocation(), Dtor,
+PDiag(diag::err_access_dtor_vbase)
+<< Context.getTypeDeclType(ClassDecl) << VBase.getType(),
+Context.getTypeDeclType(ClassDecl)) == AR_accessible) {
+  CheckDerivedToBaseConversion(
+  Context.getTypeDeclType(ClassDecl), VBase.getType(),
+  diag::err_access_dtor_vbase, 0, ClassDecl->getLocation(),
+  SourceRange(), DeclarationName(), nullptr);
+}
+
+MarkFunctionReferenced(Dtor->getLocation(), Dtor);
+DiagnoseUseOfDecl(Dtor, Dtor->getLocation());
+  }
+}
+
 /// Perform any semantic analysis which needs to be delayed until all
 /// pending class member declarations have been parsed.
 void Sema::ActOnFinishCXXMemberDecls() {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5559,6 +5559,11 @@
   void DefineImplicitDestructor(SourceLocation CurrentLocation,
 CXXDestructorDecl *Destructor);
 
+  

[PATCH] D77077: [clang] CodeGen: Make getOrEmitProtocol public for Swift

2020-03-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:148
+ llvm::function_ref
+ createProtocolReference);
 }  // end namespace CodeGen

I would call this `emitObjCProtocolObject` or something, and maybe say in the 
comment:

> Get a pointer to a protocol object for the given declaration, emitting it if 
> it hasn't already been emitted in this translation unit.  Note that the ABI 
> for emitting a protocol reference in code (e.g. for a `@protocol` expression) 
> in most runtimes is not as simple as just materializing a pointer to this 
> object.

Can you explain the need for the callback?   Are you expecting to use this for 
Swift-declared protocols by synthesizing an ObjC protocol declaration for them? 
 I can see why you'd need a callback in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77077



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


[clang] defd95e - [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

2020-03-30 Thread via cfe-commits

Author: Vince Bridgers
Date: 2020-03-30T14:13:08-05:00
New Revision: defd95ef45171252ee8491729d3f3c863bbfe530

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

LOG: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

Summary:
This check was causing a crash in a test case where the 0th argument was
uninitialized ('Assertion `T::isKind(*this)' at line SVals.h:104). This
was happening since the argument was actually undefined, but the castAs
assumes the value is DefinedOrUnknownSVal.

The fix appears to be simply to check for an undefined value and skip
the check allowing the uninitalized value checker to detect the error.

I included a test case that I verified to produce the negative case
prior to the fix, and passes with the fix.

Reviewers: martong, NoQ

Subscribers: xazax.hun, szepet, rnkovacs, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, Charusso, ASDenysPetrov, baloghadamsoftware, dkrupp, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index cd257ffdc939..6e5f5f8b5874 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -190,6 +190,9 @@ class StdLibraryFunctionsChecker
 ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
   const Summary ) const override {
   SVal V = getArgSVal(Call, getArgNo());
+  if (V.isUndef())
+return State;
+
   DefinedOrUnknownSVal L = V.castAs();
   if (!L.getAs())
 return State;

diff  --git a/clang/test/Analysis/std-c-library-functions.c 
b/clang/test/Analysis/std-c-library-functions.c
index 3f700a7c39a4..a275ea6720ad 100644
--- a/clang/test/Analysis/std-c-library-functions.c
+++ b/clang/test/Analysis/std-c-library-functions.c
@@ -89,6 +89,14 @@ void test_fread_fwrite(FILE *fp, int *buf) {
   clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
+void test_fread_uninitialized(void) {
+  void *ptr;
+  size_t sz;
+  size_t nmem;
+  FILE *fp;
+  (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call 
argument is an uninitialized value}}
+}
+
 ssize_t getline(char **, size_t *, FILE *);
 void test_getline(FILE *fp) {
   char *line = 0;



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


[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

2020-03-30 Thread Eli Friedman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG24485aec4750: [clang analysis] Make mutex guard detection 
more reliable. (authored by efriedma).

Changed prior to commit:
  https://reviews.llvm.org/D76943?vs=253253=253657#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76943

Files:
  clang/lib/Analysis/ThreadSafety.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp


Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5648,6 +5648,22 @@
 auto ptr = get();
 return ptr->f();
   }
+  void use_constructor() {
+auto ptr = ReadLockedPtr(nullptr);
+ptr->f();
+auto ptr2 = ReadLockedPtr{nullptr};
+ptr2->f();
+auto ptr3 = (ReadLockedPtr{nullptr});
+ptr3->f();
+  }
+  struct Convertible {
+Convertible();
+operator ReadLockedPtr();
+  };
+  void use_conversion() {
+ReadLockedPtr ptr = Convertible();
+ptr->f();
+  }
 }
 
 namespace PR38640 {
Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -2139,12 +2139,14 @@
 
   // handle constructors that involve temporaries
   if (auto *EWC = dyn_cast(E))
-E = EWC->getSubExpr();
-  if (auto *ICE = dyn_cast(E))
-if (ICE->getCastKind() == CK_NoOp)
-  E = ICE->getSubExpr();
+E = EWC->getSubExpr()->IgnoreParens();
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_NoOp ||
+CE->getCastKind() == CK_ConstructorConversion ||
+CE->getCastKind() == CK_UserDefinedConversion)
+  E = CE->getSubExpr()->IgnoreParens();
   if (auto *BTE = dyn_cast(E))
-E = BTE->getSubExpr();
+E = BTE->getSubExpr()->IgnoreParens();
 
   if (const auto *CE = dyn_cast(E)) {
 const auto *CtorD = dyn_cast_or_null(CE->getConstructor());


Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5648,6 +5648,22 @@
 auto ptr = get();
 return ptr->f();
   }
+  void use_constructor() {
+auto ptr = ReadLockedPtr(nullptr);
+ptr->f();
+auto ptr2 = ReadLockedPtr{nullptr};
+ptr2->f();
+auto ptr3 = (ReadLockedPtr{nullptr});
+ptr3->f();
+  }
+  struct Convertible {
+Convertible();
+operator ReadLockedPtr();
+  };
+  void use_conversion() {
+ReadLockedPtr ptr = Convertible();
+ptr->f();
+  }
 }
 
 namespace PR38640 {
Index: clang/lib/Analysis/ThreadSafety.cpp
===
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -2139,12 +2139,14 @@
 
   // handle constructors that involve temporaries
   if (auto *EWC = dyn_cast(E))
-E = EWC->getSubExpr();
-  if (auto *ICE = dyn_cast(E))
-if (ICE->getCastKind() == CK_NoOp)
-  E = ICE->getSubExpr();
+E = EWC->getSubExpr()->IgnoreParens();
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_NoOp ||
+CE->getCastKind() == CK_ConstructorConversion ||
+CE->getCastKind() == CK_UserDefinedConversion)
+  E = CE->getSubExpr()->IgnoreParens();
   if (auto *BTE = dyn_cast(E))
-E = BTE->getSubExpr();
+E = BTE->getSubExpr()->IgnoreParens();
 
   if (const auto *CE = dyn_cast(E)) {
 const auto *CtorD = dyn_cast_or_null(CE->getConstructor());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76452: Use LLD by default for Android.

2020-03-30 Thread Dan Albert via Phabricator via cfe-commits
danalbert added a comment.

In D76452#1945084 , @srhines wrote:

> In D76452#1945029 , @MaskRay wrote:
>
> > To cross build ELF object on macOS, another alternative is a wrapper named 
> > `ld` which invokes `lld -flavor gnu "$@"`
>
>
> @danalbert Would this kind of idea work with your other reverted patch? I'm 
> not sure exactly what broke on those builds.


Not really. Windows complicates things here, since it operates based on file 
extensions rather than shebangs. `ld.exe` is not `ld.cmd`, and `ld` (with no 
extension) doesn't work. Any system that expects one form won't work with the 
other. Beyond that, process creation is expensive on Windows so we should be 
avoiding as many superfluous processes as we can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76452



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


[PATCH] D76140: [InlineFunction] update attributes during inlining

2020-03-30 Thread Anna Thomas via Phabricator via cfe-commits
anna marked 3 inline comments as done.
anna added inline comments.



Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1159
+
+  auto MayContainThrowingOrExitingCall = [&](Instruction *RVal,
+ Instruction *RInst) {

reames wrote:
> Pull this out as a static helper instead of a lambda, add an assert 
> internally that the two instructions are in the same block.  
> 
> Why?  Because I'm 80% sure the state capture on the lambda isn't needed, and 
> having it as a separate function forces that discipline.  
agreed. I'll do that in this change itself before landing. I am using this 
static helper in followon change D76792.



Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1175
+  continue;
+// Sanity check that the cloned return instruction exists and is a return
+// instruction itself.

reames wrote:
> Ok, after staring at it a bit, I've convinced myself the code here is 
> correct, just needlessly conservative.
> 
> What you're doing is:
> If the callees return instruction and returned call both map to the same 
> instructions once inlined, determine whether there's a possible exit between 
> the inlined copy.
> 
> What you could be doing:
> If the callee returns a call, check if *in the callee* there's a possible 
> exit between call and return, then apply attribute to cloned call.
> 
> The key difference is when the caller directly returns the result vs uses it 
> locally.  The result here is that your transform is much more narrow in 
> applicability than it first appears.
yes, thanks for pointing it out. I realized it after our offline discussion :) 
For now, I will add a FIXME testcase which showcases the difference in code and 
handle that testcase in a followon change. 



Comment at: llvm/test/Transforms/Inline/ret_attr_update.ll:112
+  ret i8* %s
+}

reames wrote:
> There's a critical missing test case here:
> - Callee and caller have the same attributes w/different values (i.e. deref)
> 
> And thinking through the code, I think there might be a bug here.  It's not a 
> serious one, but the if the callee specifies a larger deref than the caller, 
> it looks like the the smaller value is being written over the larger.
> 
> Actually, digging through the attribute code, I think I'm wrong about the 
> bug.  However, you should definitely write the test to confirm and document 
> merging behaviour!
> 
> If it does turn out I'm correct, I'm fine with this being addressed in a 
> follow up patch provided that the test is added in this one and isn't a 
> functional issue.  
will check this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76140



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


[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

2020-03-30 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D68165#1950451 , @ldionne wrote:

> Always with me? Maybe you should double-check that you don't have something 
> weird in your git config, aliases or other? I don't know *why* it would be me 
> in particular, but the fact that it is consistently the same person is kind 
> of suspicious.


Will do. Sorry for the trouble!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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


[PATCH] D77056: RFC: [Sema][SVE] Allow non-member operators for SVE types

2020-03-30 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm added a comment.

In D77056#1950358 , @efriedma wrote:

> I'm concerned we're going to run into trouble if two people define different 
> SVE "libraries", and you try to link both of them into the same program.  If 
> you're not careful, you end up with ODR violations.  The scope of this is 
> sort of restricted in normal C++: class and enumerated types sort of have an 
> informal "owner", and people tend to respect that.  I mean, you could say 
> it's the user's own fault if they break ODR, but we're essentially making a 
> trap here.  Maybe it would make sense to require that SVE operators are 
> defined in a namespace?  That would make the user think about the issue.


Thanks for the suggestion.  Requiring  a namespace sounds like a good plan -- 
will give it some more thought.

> We're also basically committing to never building these operators into the 
> compiler, for all sizeless types for all targets.

I guess this brings up the question of when we should introduce a specific 
isSizelessSVEBuiltinType() query.  One option would to be wait until there's at 
least one type that isSizelessBuiltinType but isn't an SVE type.  Another would 
be to introduce  it when a feature seems too SVE-specific.  If you think this 
qualifies for the latter then I can add the query now.

For AArch64, it would always be possible in future to provide a standard header 
file that defines certain operators for SVE types.  One way of implementing the 
header file would be to use magic attributes that get the compiler to fill in 
the implementation.  (Or it could perhaps just use a pragma to "enable" 
built-in operators, but that sounds riskier.)

> It would probably make sense to send this to cfe-dev, to get more 
> perspectives on the language change.

OK.  I'll address your comments first and then post there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77056



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


[clang] 24485ae - [clang analysis] Make mutex guard detection more reliable.

2020-03-30 Thread Eli Friedman via cfe-commits

Author: Eli Friedman
Date: 2020-03-30T11:46:02-07:00
New Revision: 24485aec4750255574a9a8211b3aef1ce00e83b6

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

LOG: [clang analysis] Make mutex guard detection more reliable.

-Wthread-safety was failing to detect certain AST patterns it should
detect. Make the pattern detection a bit more comprehensive.

Due to an unrelated bug involving template instantiation, this showed up
as a regression in 10.0 vs. 9.0 in the original bug report. The included
testcase fails on older versions of clang, though.

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

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

Added: 


Modified: 
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Removed: 




diff  --git a/clang/lib/Analysis/ThreadSafety.cpp 
b/clang/lib/Analysis/ThreadSafety.cpp
index 252083f377d9..e0ff23df5ab4 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -2139,12 +2139,14 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
 
   // handle constructors that involve temporaries
   if (auto *EWC = dyn_cast(E))
-E = EWC->getSubExpr();
-  if (auto *ICE = dyn_cast(E))
-if (ICE->getCastKind() == CK_NoOp)
-  E = ICE->getSubExpr();
+E = EWC->getSubExpr()->IgnoreParens();
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_NoOp ||
+CE->getCastKind() == CK_ConstructorConversion ||
+CE->getCastKind() == CK_UserDefinedConversion)
+  E = CE->getSubExpr()->IgnoreParens();
   if (auto *BTE = dyn_cast(E))
-E = BTE->getSubExpr();
+E = BTE->getSubExpr()->IgnoreParens();
 
   if (const auto *CE = dyn_cast(E)) {
 const auto *CtorD = dyn_cast_or_null(CE->getConstructor());

diff  --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp 
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 23255a53eae7..cdb22cd22a99 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5648,6 +5648,22 @@ namespace ReturnScopedLockable {
 auto ptr = get();
 return ptr->f();
   }
+  void use_constructor() {
+auto ptr = ReadLockedPtr(nullptr);
+ptr->f();
+auto ptr2 = ReadLockedPtr{nullptr};
+ptr2->f();
+auto ptr3 = (ReadLockedPtr{nullptr});
+ptr3->f();
+  }
+  struct Convertible {
+Convertible();
+operator ReadLockedPtr();
+  };
+  void use_conversion() {
+ReadLockedPtr ptr = Convertible();
+ptr->f();
+  }
 }
 
 namespace PR38640 {



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


Re: [clang] 857bf5d - [FIX] Do not copy an llvm::function_ref if it has to be reused

2020-03-30 Thread David Blaikie via cfe-commits
On Fri, Mar 27, 2020 at 5:16 PM Arthur O'Dwyer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Richard: Okay, filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94376 !
>
> David: You are correct, the bug in function_ref appeared only when
> constructing from `const function_ref&&`. When I said that the constructor
> template suppressed the copy constructor, I was wrong. The implicitly
> generated copy constructor is still there, and it's the best match for
> `const function_ref&` but not for `const function_ref&&`.  The bug would
> also appear with construction from `volatile function_ref&`, but, let's not
> go there. :)
>
> IMHO (YMMV), it would be worth *additionally* adding a complex but
> "realistic" test case that depends on the GCC bug, as opposed to your own
> test's simple but "unrealistic" cast to `const function_ref&&`. Here's a
> test case that I believe would have dereferenced null on GCC but would have
> worked fine on Clang—
>
> TEST(FunctionRefTest, BadCopyGCCBug) {
>
>   auto A = [] { return 1; };
>
>   function_ref W = A;
>
>   auto LW = [=]() {  // captures a data member 'const function_ref W'
>
> return [=]() {
>
>   return W();
>
> };
>
>   }();
>
>   auto LX = std::move(LW);  // calls function_ref's 'const&&' constructor
>
>   W = nullptr;
>
>   ASSERT_EQ(LX(), 1);
>
> }
>

Yeah, I tend to err on the side of fairly isolated testing. I guess a
fairly closer-to-practical, though smaller (in terms of feature
interactions, not necessarily in lines of code) example might be to
std::move on a const ref, rather than the raw cast - the next step beyond
that would be to use a template to make it more realistic as to why you'd
be moving a const ref, etc.

But I think the narrower test is alright.

- Dave


>
> –Arthur
>
>
> On Fri, Mar 27, 2020 at 7:48 PM Richard Smith 
> wrote:
>
>> On Thu, 26 Mar 2020 at 21:50, Arthur O'Dwyer via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> [...]
>>> Argh! That is insanely sneaky, and it is quite probable IMHO that this
>>> is a core-language bug in GCC, because GCC behaves differently from EDG and
>>> Clang here.
>>> https://godbolt.org/z/oCvLpv
>>> On GCC, when you have a lambda nested within a lambda, and you
>>> capture-by-copy a variable which was captured-by-copy using [=] or [i] (but
>>> not C++14 init-capture [i=i]), then your data member for that capture will
>>> have type `const I`, not just `I`.
>>> On Clang and EDG, [=], [i], and [i=i] all behave equivalently, and
>>> capture a data member with type `I`.
>>>
>>> I don't see anything about this on
>>> https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=lambda%20capture%20const
>>> — Richard, you might be best able to describe the issue correctly ;) but if
>>> you want me to file it, I will.  (Might be a corollary of bug 86697
>>> .)
>>>
>>
>> Oh wow, so this would only fail with a combination of libstdc++ (which
>> makes a copy of the lambda and destroy the original before copying the
>> lambda) and GCC (which has a bug in how it determines the type of a nested
>> capture)? Fascinating! :)
>>
>> [expr.prim.lambda.capture]p10 is the relevant rule: "The type of such a
>> data member is the referenced type if the entity is a reference to an
>> object, an lvalue reference to the referenced function type if the entity
>> is a reference to a function, or the type of the corresponding captured
>> entity otherwise."
>>
>> Regardless of whether you think the captured entity is the original
>> variable or the member of the outer closure type, the type of that entity
>> is not const-qualified. So the inner capture should not have a
>> const-qualified type.
>>
>> Given that you identified the GCC capture bug, I think you should do the
>> honors :)
>>
>>
>>> Regardless, llvm::function_ref's SFINAE should still be fixed. This is a
>>> bug in llvm::function_ref, exposed by a bug in GCC. (Or vice versa.)
>>>
>>
>> Yup.
>>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

2020-03-30 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

In D68165#1950357 , @Szelethus wrote:

> Ugh, I squashed my local commits and pushed, it seems like for some reason it 
> made you the author... it has happened to me before (always with you, for 
> some reason), but was able to notice and correct it.


Always with me? Maybe you should double-check that you don't have something 
weird in your git config, aliases or other? I don't know *why* it would be me 
in particular, but the fact that it is consistently the same person is kind of 
suspicious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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


[PATCH] D77077: [clang] CodeGen: Make getOrEmitProtocol public for Swift

2020-03-30 Thread Arnold Schwaighofer via Phabricator via cfe-commits
aschwaighofer created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Swift would like to use clang's abis to emit protocol declarations.

It needs to a hook to register when inherited protocols are emitted.

This commits adds the public API:

emitProtocolDecl(CodeGenModule , const ObjCProtocolDecl *p,

  llvm::function_ref
  createProtocolReference);

rdar://60888524


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77077

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.cpp
  clang/lib/CodeGen/CGObjCRuntime.h

Index: clang/lib/CodeGen/CGObjCRuntime.h
===
--- clang/lib/CodeGen/CGObjCRuntime.h
+++ clang/lib/CodeGen/CGObjCRuntime.h
@@ -211,6 +211,16 @@
   /// implementations.
   virtual void GenerateProtocol(const ObjCProtocolDecl *OPD) = 0;
 
+  /// getOrEmitProtocol - Get the protocol object for the given
+  /// declaration, emitting it if necessary. The return value has type
+  /// ProtocolPtrTy.
+  /// \p getProtocolReference is called by the implementation when a
+  /// reference to another protocol is needed.
+  virtual llvm::Constant *getOrEmitProtocol(
+  const ObjCProtocolDecl *OPD,
+  llvm::function_ref
+  getProtocolReference) = 0;
+
   /// Generate a function preamble for a method with the specified
   /// types.
 
Index: clang/lib/CodeGen/CGObjCRuntime.cpp
===
--- clang/lib/CodeGen/CGObjCRuntime.cpp
+++ clang/lib/CodeGen/CGObjCRuntime.cpp
@@ -13,14 +13,15 @@
 //===--===//
 
 #include "CGObjCRuntime.h"
-#include "CGCleanup.h"
 #include "CGCXXABI.h"
+#include "CGCleanup.h"
 #include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
@@ -383,3 +384,11 @@
 CGM.getTypes().GetFunctionType(argsInfo)->getPointerTo();
   return MessageSendInfo(argsInfo, signatureType);
 }
+
+llvm::Constant *clang::CodeGen::emitProtocolDecl(
+CodeGenModule , const ObjCProtocolDecl *protocol,
+llvm::function_ref
+createProtocolReference) {
+  return CGM.getObjCRuntime().getOrEmitProtocol(protocol,
+createProtocolReference);
+}
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1110,7 +1110,7 @@
   /// GetOrEmitProtocol - Get the protocol object for the given
   /// declaration, emitting it if necessary. The return value has type
   /// ProtocolPtrTy.
-  virtual llvm::Constant *GetOrEmitProtocol(const ObjCProtocolDecl *PD)=0;
+  llvm::Constant *GetOrEmitProtocol(const ObjCProtocolDecl *PD);
 
   /// GetOrEmitProtocolRef - Get a forward reference to the protocol
   /// object for the given declaration, emitting it if needed. These
@@ -1288,10 +1288,15 @@
   llvm::Constant *emitMethodList(Twine Name, MethodListType MLT,
  ArrayRef Methods);
 
-  /// GetOrEmitProtocol - Get the protocol object for the given
+  /// getOrEmitProtocol - Get the protocol object for the given
   /// declaration, emitting it if necessary. The return value has type
   /// ProtocolPtrTy.
-  llvm::Constant *GetOrEmitProtocol(const ObjCProtocolDecl *PD) override;
+  /// \p getProtocolReference is called by the implementation when a
+  /// reference to another protocol is needed.
+  llvm::Constant *getOrEmitProtocol(
+  const ObjCProtocolDecl *OPD,
+  llvm::function_ref
+  getProtocolReference) override;
 
   /// GetOrEmitProtocolRef - Get a forward reference to the protocol
   /// object for the given declaration, emitting it if needed. These
@@ -1309,9 +1314,11 @@
 
   /// EmitProtocolList - Generate the list of referenced
   /// protocols. The return value has type ProtocolListPtrTy.
-  llvm::Constant *EmitProtocolList(Twine Name,
-   ObjCProtocolDecl::protocol_iterator begin,
-   ObjCProtocolDecl::protocol_iterator end);
+  llvm::Constant *EmitProtocolList(
+  Twine Name, ObjCProtocolDecl::protocol_iterator begin,
+  ObjCProtocolDecl::protocol_iterator end,
+  llvm::function_ref
+  getProtocolRef);
 
   /// EmitSelector - Return a Value*, of type ObjCTypes.SelectorPtrTy,
   /// for the given selector.
@@ -1471,10 +1478,15 @@
 const ObjCIvarDecl *Ivar,
 unsigned long int offset);
 
-  /// GetOrEmitProtocol - Get the protocol object 

[PATCH] D76140: [InlineFunction] update attributes during inlining

2020-03-30 Thread Philip Reames via Phabricator via cfe-commits
reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM, but with two specific required follow ups.  If you're not comfortable 
committing to both, please don't land this one.




Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:93
+static cl::opt MaxInstCheckedForThrow(
+"max-inst-checked-for-throw-during-inlining", cl::Hidden,
+cl::desc("the maximum number of instructions analyzed for may throw during 
"

I'd suggest a name change here.  Maybe: "inliner-attribute-window"?



Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1159
+
+  auto MayContainThrowingOrExitingCall = [&](Instruction *RVal,
+ Instruction *RInst) {

Pull this out as a static helper instead of a lambda, add an assert internally 
that the two instructions are in the same block.  

Why?  Because I'm 80% sure the state capture on the lambda isn't needed, and 
having it as a separate function forces that discipline.  



Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1175
+  continue;
+// Sanity check that the cloned return instruction exists and is a return
+// instruction itself.

Ok, after staring at it a bit, I've convinced myself the code here is correct, 
just needlessly conservative.

What you're doing is:
If the callees return instruction and returned call both map to the same 
instructions once inlined, determine whether there's a possible exit between 
the inlined copy.

What you could be doing:
If the callee returns a call, check if *in the callee* there's a possible exit 
between call and return, then apply attribute to cloned call.

The key difference is when the caller directly returns the result vs uses it 
locally.  The result here is that your transform is much more narrow in 
applicability than it first appears.



Comment at: llvm/test/Transforms/Inline/ret_attr_update.ll:112
+  ret i8* %s
+}

There's a critical missing test case here:
- Callee and caller have the same attributes w/different values (i.e. deref)

And thinking through the code, I think there might be a bug here.  It's not a 
serious one, but the if the callee specifies a larger deref than the caller, it 
looks like the the smaller value is being written over the larger.

Actually, digging through the attribute code, I think I'm wrong about the bug.  
However, you should definitely write the test to confirm and document merging 
behaviour!

If it does turn out I'm correct, I'm fine with this being addressed in a follow 
up patch provided that the test is added in this one and isn't a functional 
issue.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76140



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


[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

2020-03-30 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments.



Comment at: clang/CMakeLists.txt:311
+  "SystemZ Default Arch")
+add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}")
+

thakis wrote:
> Hahnfeld wrote:
> > Passing values like this is unusual for CMake and causes all source files 
> > to be recompiled if the value is changed. Instead could we add this to 
> > `include/clang/Config/config.h.cmake` like other `CLANG_DEFAULT_*` options?
> +1
Also, this is used in one cpp file; passing the define to every compilation 
unit seems a bit aggressive.

I'll try to land a patch to fix this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75914



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


[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

2020-03-30 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments.



Comment at: clang/CMakeLists.txt:311
+  "SystemZ Default Arch")
+add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}")
+

thakis wrote:
> thakis wrote:
> > Hahnfeld wrote:
> > > Passing values like this is unusual for CMake and causes all source files 
> > > to be recompiled if the value is changed. Instead could we add this to 
> > > `include/clang/Config/config.h.cmake` like other `CLANG_DEFAULT_*` 
> > > options?
> > +1
> Also, this is used in one cpp file; passing the define to every compilation 
> unit seems a bit aggressive.
> 
> I'll try to land a patch to fix this.
Done in c506adcdf2ca3ba6459e52e09c55868e3b57af46. (This only changes the 
implementation of the feature; the way people who build clang tell cmake what 
they want didn't change.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75914



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


[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

2020-03-30 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: llvm/include/llvm/MC/MCDirectives.h:47
+  MCSA_WeakReference,   ///< .weak_reference (MachO)
+  MCSA_WeakDefAutoPrivate   ///< .weak_def_can_be_hidden (MachO)
 };

DiggerLin wrote:
> @hubert.reinterpretcast , @jasonliu , do we need to create a NFC patch for 
> the clang format problem of the above first ?
I think it would help; yes. Please drop one at your earliest convenience and 
then rebase this patch on top of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76932



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


[PATCH] D77022: [analyzer] Use IgnoreImpCasts() instead of reimplementing it.

2020-03-30 Thread Nico Weber via Phabricator via cfe-commits
thakis marked an inline comment as done.
thakis added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:512
+  return E->IgnoreImpCasts();
 }
 

NoQ wrote:
> Charusso wrote:
> > I think it would make sense to remove the helper-function completely. 
> > (Being used 2 times.)
> Yup.
I didn't do that because I liked the comment that this is for the _Nonnull 
implicit ARC casts – if I inline the function I have to duplicate the comment.


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

https://reviews.llvm.org/D77022



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


[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH

2020-03-30 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments.



Comment at: clang/CMakeLists.txt:311
+  "SystemZ Default Arch")
+add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}")
+

Hahnfeld wrote:
> Passing values like this is unusual for CMake and causes all source files to 
> be recompiled if the value is changed. Instead could we add this to 
> `include/clang/Config/config.h.cmake` like other `CLANG_DEFAULT_*` options?
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75914



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


[PATCH] D76996: [analyzer] Improve PlacementNewChecker

2020-03-30 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat updated this revision to Diff 253642.
f00kat marked an inline comment as done.
f00kat added a comment.

test fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76996

Files:
  clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
  clang/test/Analysis/placement-new.cpp

Index: clang/test/Analysis/placement-new.cpp
===
--- clang/test/Analysis/placement-new.cpp
+++ clang/test/Analysis/placement-new.cpp
@@ -155,3 +155,147 @@
   (void)dp;
 }
 } // namespace testHierarchy
+
+namespace testArrayTypesAllocation {
+void f1() {
+  struct S {
+short a;
+  };
+
+  // bad (not enough space).
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer1[sizeof(S) * N]; // expected-note {{'buffer1' initialized here}}
+  ::new (buffer1) S[N];// expected-warning{{Storage provided to placement new is only 64 bytes, whereas the allocated array type requires more space for internal needs}} expected-note 1 {{}}
+}
+
+void f2() {
+  struct S {
+short a;
+  };
+
+  // maybe ok but we need to warn.
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)]; // expected-note {{'buffer2' initialized here}}
+  ::new (buffer2) S[N];  // expected-warning{{68 bytes is possibly not enough for array allocation which requires 64 bytes. Current overhead requires the size of 4 bytes}} expected-note 1 {{}}
+}
+} // namespace testArrayTypesAllocation
+
+namespace testStructAlign {
+void f1() {
+  struct X {
+char a[9];
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // bad (struct X is aligned to char).
+  ::new () long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f2() {
+  struct X {
+char a;
+char b;
+long c;
+  } Xi;
+
+  // ok (struct X is aligned to long).
+  ::new () long;
+}
+
+void f3() {
+  struct X {
+char a;
+char b;
+long c;
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // bad (struct X is aligned to long but field 'b' is aligned to 1 because of its offset)
+  ::new () long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f4() {
+  struct X {
+char a;
+struct alignas(alignof(short)) Y {
+  char b;
+  char c;
+} y;
+long d;
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // bad. 'b' is aligned to short
+  ::new () long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f5() {
+  short b[10]; // expected-note {{'b' initialized here}}
+
+  ::new () long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f6() {
+  short b[10]; // expected-note {{'b' initialized here}}
+
+  // bad (same as previous but checks ElementRegion case)
+  ::new ([0]) long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f7() {
+  short b[10];
+
+  // ok. 2(short align) + 3*2(index '1' offset)
+  ::new ([3]) long;
+}
+
+void f8() {
+  short b[10]; // expected-note {{'b' initialized here}}
+
+  // bad. 2(short align) + 2*2(index '2' offset)
+  ::new ([2]) long; // expected-warning{{Storage type is aligned to 6 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f9() {
+  struct X {
+char a;
+alignas(alignof(short)) char b[20];
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // ok 2(custom align) + 6(index '2' offset)
+  ::new ([6]) long;
+
+  // bad 2(custom align) + 1(index '2' offset)
+  ::new ([1]) long; // expected-warning{{Storage type is aligned to 3 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f10() {
+  struct X {
+char a[8];
+alignas(2) char b;
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // bad (struct X is aligned to 2).
+  ::new () long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f11() {
+  struct X {
+char a;
+char b;
+struct Y {
+  long c;
+} d;
+  } Xi;
+
+  // ok (struct X is aligned to long).
+  ::new () long;
+}
+
+void f12() {
+  struct alignas(alignof(long)) X {
+char a;
+char b;
+  } Xi;
+
+  // ok (struct X is aligned to long).
+  ::new () long;
+}
+} // namespace testStructAlign
Index: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -25,22 +25,31 @@
   void checkPreStmt(const 

[clang] c506adc - Move CLANG_SYSTEMZ_DEFAULT_ARCH to config.h.

2020-03-30 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2020-03-30T14:16:17-04:00
New Revision: c506adcdf2ca3ba6459e52e09c55868e3b57af46

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

LOG: Move CLANG_SYSTEMZ_DEFAULT_ARCH to config.h.

Instead of using a global define; see comments on D75914.

While here, port 9c9d88d8b1b to the GN build.

Added: 


Modified: 
clang/CMakeLists.txt
clang/include/clang/Config/config.h.cmake
clang/lib/Driver/ToolChains/Arch/SystemZ.cpp
llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index c9e76c5e4518..88e556fd88a0 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -306,9 +306,7 @@ if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 35)
 "Default architecture for OpenMP offloading to Nvidia GPUs." FORCE)
 endif()
 
-set(CLANG_SYSTEMZ_DEFAULT_ARCH "z10" CACHE STRING
-  "SystemZ Default Arch")
-add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}")
+set(CLANG_SYSTEMZ_DEFAULT_ARCH "z10" CACHE STRING "SystemZ Default Arch")
 
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")

diff  --git a/clang/include/clang/Config/config.h.cmake 
b/clang/include/clang/Config/config.h.cmake
index 261b3841b86f..a0f8b6b1b0da 100644
--- a/clang/include/clang/Config/config.h.cmake
+++ b/clang/include/clang/Config/config.h.cmake
@@ -83,4 +83,7 @@
 /* Spawn a new process clang.exe for the CC1 tool invocation, when necessary */
 #cmakedefine01 CLANG_SPAWN_CC1
 
+/* Default  to all compiler invocations for --sysroot=. */
+#define CLANG_SYSTEMZ_DEFAULT_ARCH "${CLANG_SYSTEMZ_DEFAULT_ARCH}"
+
 #endif

diff  --git a/clang/lib/Driver/ToolChains/Arch/SystemZ.cpp 
b/clang/lib/Driver/ToolChains/Arch/SystemZ.cpp
index b263fb7df09e..f81bf68172de 100644
--- a/clang/lib/Driver/ToolChains/Arch/SystemZ.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/SystemZ.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "SystemZ.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"

diff  --git a/llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn 
b/llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
index cc2c4e19ad49..7fbfb46a41c5 100644
--- a/llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
@@ -36,6 +36,7 @@ write_cmake_config("Config") {
 "ENABLE_X86_RELAX_RELOCATIONS=",
 "ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=",
 "CLANG_ENABLE_OBJC_REWRITER=1",  # FIXME: flag?
+"CLANG_SYSTEMZ_DEFAULT_ARCH=z10",
   ]
 
   if (clang_enable_arcmt) {



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


[PATCH] D76546: [Hexagon] MaxAtomicPromoteWidth, MaxAtomicInlineWidth are not getting set.

2020-03-30 Thread Sid Manning via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG81194bfeea7b: [Hexagon] MaxAtomicPromoteWidth and 
MaxAtomicInlineWidth are not getting set. (authored by sidneym).

Changed prior to commit:
  https://reviews.llvm.org/D76546?vs=251896=253640#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76546

Files:
  clang/lib/Basic/Targets/Hexagon.h
  clang/test/Preprocessor/hexagon-predefines.c


Index: clang/test/Preprocessor/hexagon-predefines.c
===
--- clang/test/Preprocessor/hexagon-predefines.c
+++ clang/test/Preprocessor/hexagon-predefines.c
@@ -113,3 +113,18 @@
 // CHECK-LINUX: #define __unix__ 1
 // CHECK-LINUX: #define linux 1
 // CHECK-LINUX: #define unix 1
+
+// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-linux-musl \
+// RUN: -target-cpu hexagonv67 -target-feature +hvxv67 \
+// RUN: -target-feature +hvx-length128b %s | FileCheck \
+// RUN: %s -check-prefix CHECK-ATOMIC
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_INT_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_LONG_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 2
Index: clang/lib/Basic/Targets/Hexagon.h
===
--- clang/lib/Basic/Targets/Hexagon.h
+++ clang/lib/Basic/Targets/Hexagon.h
@@ -57,6 +57,7 @@
 LargeArrayAlign = 64;
 UseBitFieldTypeAlignment = true;
 ZeroLengthBitfieldBoundary = 32;
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
 
 // These are the default values anyway, but explicitly make sure
 // that the size of the boolean type is 8 bits. Bool vectors are used


Index: clang/test/Preprocessor/hexagon-predefines.c
===
--- clang/test/Preprocessor/hexagon-predefines.c
+++ clang/test/Preprocessor/hexagon-predefines.c
@@ -113,3 +113,18 @@
 // CHECK-LINUX: #define __unix__ 1
 // CHECK-LINUX: #define linux 1
 // CHECK-LINUX: #define unix 1
+
+// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-linux-musl \
+// RUN: -target-cpu hexagonv67 -target-feature +hvxv67 \
+// RUN: -target-feature +hvx-length128b %s | FileCheck \
+// RUN: %s -check-prefix CHECK-ATOMIC
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_INT_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_LONG_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 2
Index: clang/lib/Basic/Targets/Hexagon.h
===
--- clang/lib/Basic/Targets/Hexagon.h
+++ clang/lib/Basic/Targets/Hexagon.h
@@ -57,6 +57,7 @@
 LargeArrayAlign = 64;
 UseBitFieldTypeAlignment = true;
 ZeroLengthBitfieldBoundary = 32;
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
 
 // These are the default values anyway, but explicitly make sure
 // that the size of the boolean type is 8 bits. Bool vectors are used
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D76987: Rename options --cuda-gpu-arch and --no-cuda-gpu-arch

2020-03-30 Thread Greg Rodgers via Phabricator via cfe-commits
gregrodgers added a comment.

This was discussed on llvm-dev three years ago.  Here is the thread.

http://lists.llvm.org/pipermail/llvm-dev/2017-February/109930.html

The last name discussed was "-- offload-arch".   I don't believe we need a list 
option anymore.   So ignore the very old request for --offload-archs.

I am ok with the patch the way it is.   In the future, we should consider 
renaming the CudaArch class to OffloadArch class .  Also the GpuArchList is 
currently only initialized in CudaActionBuilder.   Eventually this is will have 
to be done for HIPActionBuilder and OpenMPActionBuilder.   Could you consider 
creating a function to  InitializeGpuArchList ?


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

https://reviews.llvm.org/D76987



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


[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

2020-03-30 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D68165#1949954 , @ldionne wrote:

> > Closed by commit rG703a1b8caf09 
> > : 
> > [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy 
> > CallDescriptionMap (authored by ldionne, committed by Szelethus). · Explain 
> > Why
>
> This is really strange -- it looks like 703a1b8caf09 
>  was 
> attributed to me in git, but I have nothing to do with this change. How did 
> you commit the patch? Is that some Phabricator bug?


Ugh, I squashed my local commits and pushed, it seems like for some reason it 
made you the author... it has happened to me before (always with you, for some 
reason), but was able to notice and correct it. There is little we can do about 
this, right?

In D68165#1950212 , @NoQ wrote:

> Not a phabricator bug. Maybe @Szelethus's local git somehow got in a weird 
> state(?)


Its been a source of endless misery :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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


[PATCH] D77056: RFC: [Sema][SVE] Allow non-member operators for SVE types

2020-03-30 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I'm concerned we're going to run into trouble if two people define different 
SVE "libraries", and you try to link both of them into the same program.  If 
you're not careful, you end up with ODR violations.  The scope of this is sort 
of restricted in normal C++: class and enumerated types sort of have an 
informal "owner", and people tend to respect that.  I mean, you could say it's 
the user's own fault if they break ODR, but we're essentially making a trap 
here.  Maybe it would make sense to require that SVE operators are defined in a 
namespace?  That would make the user think about the issue.

We're also basically committing to never building these operators into the 
compiler, for all sizeless types for all targets.

It would probably make sense to send this to cfe-dev, to get more perspectives 
on the language change.




Comment at: clang/include/clang/AST/Type.h:6833
 inline bool Type::isOverloadableType() const {
-  return isDependentType() || isRecordType() || isEnumeralType();
+  return (isDependentType() || isRecordType() || isEnumeralType() ||
+  isSizelessBuiltinType());

Unnecessary parentheses



Comment at: clang/test/SemaCXX/sizeless-1.cpp:659
+svint8_t (svint8_t &, int);
+int operator,(svint8_t, svint8_t);
+

Need some testcases with template operators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77056



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


[clang] 7842e7e - [OPENMP50]Add codegen support for array shaping expression in depend

2020-03-30 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-03-30T13:37:21-04:00
New Revision: 7842e7ebbf3b68ebe52592d51aaf7a20f94d047b

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

LOG: [OPENMP50]Add codegen support for array shaping expression in depend
clauses.

Implemented codegen for array shaping operation in depend clauses. The
begin of the expression is the pointer itself, while the size of the
dependence data is the mukltiplacation of all dimensions in the array
shaping expression.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/depobj_ast_print.cpp
clang/test/OpenMP/depobj_codegen.cpp
clang/test/OpenMP/task_codegen.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 1bb001ced31a..4b913607c1db 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -5363,11 +5363,29 @@ std::pair 
CGOpenMPRuntime::emitDependClause(
   if (Dependencies[I].first == OMPC_DEPEND_depobj)
 continue;
   const Expr *E = Dependencies[I].second;
-  LValue Addr = CGF.EmitLValue(E);
+  const auto *OASE = dyn_cast(E);
+  LValue Addr;
+  if (OASE) {
+const Expr *Base = OASE->getBase()->IgnoreParenImpCasts();
+Addr =
+CGF.EmitLoadOfPointerLValue(CGF.EmitLValue(Base).getAddress(CGF),
+
Base->getType()->castAs());
+  } else {
+Addr = CGF.EmitLValue(E);
+  }
   llvm::Value *Size;
   QualType Ty = E->getType();
-  if (const auto *ASE =
-  dyn_cast(E->IgnoreParenImpCasts())) {
+  if (OASE) {
+Size = llvm::ConstantInt::get(CGF.SizeTy,/*V=*/1);
+for (const Expr *SE : OASE->getDimensions()) {
+   llvm::Value *Sz = CGF.EmitScalarExpr(SE);
+   Sz = CGF.EmitScalarConversion(Sz, SE->getType(),
+CGF.getContext().getSizeType(),
+SE->getExprLoc());
+   Size = CGF.Builder.CreateNUWMul(Size, Sz);
+}
+  } else if (const auto *ASE =
+ dyn_cast(E->IgnoreParenImpCasts())) {
 LValue UpAddrLVal =
 CGF.EmitOMPArraySectionExpr(ASE, /*IsLowerBound=*/false);
 llvm::Value *UpAddr = CGF.Builder.CreateConstGEP1_32(

diff  --git a/clang/test/OpenMP/depobj_ast_print.cpp 
b/clang/test/OpenMP/depobj_ast_print.cpp
index 8b6586ca2b26..f3076646a25e 100644
--- a/clang/test/OpenMP/depobj_ast_print.cpp
+++ b/clang/test/OpenMP/depobj_ast_print.cpp
@@ -17,17 +17,20 @@ void foo() {}
 template 
 T tmain(T argc) {
   static T a;
-#pragma omp depobj(a) depend(in:argc)
+  int *b;
+#pragma omp depobj(a) depend(in:argc, ([4][*b][4])b)
 #pragma omp depobj(argc) destroy
 #pragma omp depobj(argc) update(inout)
   return argc;
 }
 // CHECK:  static T a;
-// CHECK-NEXT: #pragma omp depobj (a) depend(in : argc){{$}}
+// CHECK-NEXT: int *b;
+// CHECK-NEXT: #pragma omp depobj (a) depend(in : argc,([4][*b][4])b){{$}}
 // CHECK-NEXT: #pragma omp depobj (argc) destroy{{$}}
 // CHECK-NEXT: #pragma omp depobj (argc) update(inout){{$}}
 // CHECK:  static void *a;
-// CHECK-NEXT: #pragma omp depobj (a) depend(in : argc){{$}}
+// CHECK-NEXT: int *b;
+// CHECK-NEXT: #pragma omp depobj (a) depend(in : argc,([4][*b][4])b){{$}}
 // CHECK-NEXT: #pragma omp depobj (argc) destroy{{$}}
 // CHECK-NEXT: #pragma omp depobj (argc) update(inout){{$}}
 

diff  --git a/clang/test/OpenMP/depobj_codegen.cpp 
b/clang/test/OpenMP/depobj_codegen.cpp
index 61b97d0b8479..2c7509babc17 100644
--- a/clang/test/OpenMP/depobj_codegen.cpp
+++ b/clang/test/OpenMP/depobj_codegen.cpp
@@ -22,7 +22,7 @@ template 
 T tmain(T argc) {
   static T a;
   void *argv;
-#pragma omp depobj(a) depend(in:argv)
+#pragma omp depobj(a) depend(in:argv, ([3][*(int*)argv][4])argv)
 #pragma omp depobj(argc) destroy
 #pragma omp depobj(argc) update(inout)
   return argc;
@@ -87,19 +87,30 @@ int main(int argc, char **argv) {
 // CHECK-LABEL: tmain
 // CHECK: [[ARGC_ADDR:%.+]] = alloca i8*,
 // CHECK: [[GTID:%.+]] = call i32 @__kmpc_global_thread_num(
-// CHECK: [[DEP_ADDR_VOID:%.+]] = call i8* @__kmpc_alloc(i32 [[GTID]], i64 48, 
i8* null)
-// CHECK: [[DEP_ADDR:%.+]] = bitcast i8* [[DEP_ADDR_VOID]] to [2 x 
%struct.kmp_depend_info]*
-// CHECK: [[BASE_ADDR:%.+]] = getelementptr inbounds [2 x 
%struct.kmp_depend_info], [2 x %struct.kmp_depend_info]* [[DEP_ADDR]], i{{.+}} 
0, i{{.+}} 0
+// CHECK: [[DEP_ADDR_VOID:%.+]] = call i8* @__kmpc_alloc(i32 [[GTID]], i64 72, 
i8* null)
+// CHECK: [[DEP_ADDR:%.+]] = bitcast i8* [[DEP_ADDR_VOID]] to [3 x 
%struct.kmp_depend_info]*
+// CHECK: [[BASE_ADDR:%.+]] = getelementptr inbounds [3 x 
%struct.kmp_depend_info], [3 x 

[clang] 81194bf - [Hexagon] MaxAtomicPromoteWidth and MaxAtomicInlineWidth are not getting set.

2020-03-30 Thread Sid Manning via cfe-commits

Author: Sid Manning
Date: 2020-03-30T12:33:51-05:00
New Revision: 81194bfeea7b40b6e8c543bb4ae49b59f590475b

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

LOG: [Hexagon] MaxAtomicPromoteWidth and MaxAtomicInlineWidth are not getting 
set.

Noticed when building llvm's c++ library.

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

Added: 


Modified: 
clang/lib/Basic/Targets/Hexagon.h
clang/test/Preprocessor/hexagon-predefines.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/Hexagon.h 
b/clang/lib/Basic/Targets/Hexagon.h
index 89e3fa3fa6bf..7e173df81683 100644
--- a/clang/lib/Basic/Targets/Hexagon.h
+++ b/clang/lib/Basic/Targets/Hexagon.h
@@ -57,6 +57,7 @@ class LLVM_LIBRARY_VISIBILITY HexagonTargetInfo : public 
TargetInfo {
 LargeArrayAlign = 64;
 UseBitFieldTypeAlignment = true;
 ZeroLengthBitfieldBoundary = 32;
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
 
 // These are the default values anyway, but explicitly make sure
 // that the size of the boolean type is 8 bits. Bool vectors are used

diff  --git a/clang/test/Preprocessor/hexagon-predefines.c 
b/clang/test/Preprocessor/hexagon-predefines.c
index 54013ceffa64..7979d567134b 100644
--- a/clang/test/Preprocessor/hexagon-predefines.c
+++ b/clang/test/Preprocessor/hexagon-predefines.c
@@ -113,3 +113,18 @@
 // CHECK-LINUX: #define __unix__ 1
 // CHECK-LINUX: #define linux 1
 // CHECK-LINUX: #define unix 1
+
+// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-linux-musl \
+// RUN: -target-cpu hexagonv67 -target-feature +hvxv67 \
+// RUN: -target-feature +hvx-length128b %s | FileCheck \
+// RUN: %s -check-prefix CHECK-ATOMIC
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_INT_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_LONG_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 2
+// CHECK-ATOMIC: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 2



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


[PATCH] D77074: [FPEnv][AArch64] Platform-specific builtin constrained FP enablement

2020-03-30 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn marked 3 inline comments as done.
kpn added a comment.

Note that the AArch64 backend isn't ready for some of these changes. That's why 
the test is marked XFAIL.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5706
+Function *F;
+//exit(2); // XXX
+if (Builder.getIsFPConstrained())

These exit() calls tie back into my test matrix. They'll be removed when I 
eventually push.



Comment at: clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c:288
+
+// XXX FIXME do we need to check for both w and x registers?
+// COMMON-LABEL: test_vceq_f64

Anyone? I'm not an ARM expert.



Comment at: clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c:889
+
+// FIXME why the unused bitcast? There are several of them!
+// COMMON-LABEL: test_vrnda_f64

???


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77074



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


[PATCH] D76996: [analyzer] Improve PlacementNewChecker

2020-03-30 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat updated this revision to Diff 253628.
f00kat marked 3 inline comments as done.
f00kat added a comment.

1. Removed code and tests for ConcreteInt cases
2. Fixed FieldRegion check
3. Added handling for ElementRegion cases such as

  void f7() {
short b[10];
  
// ok. 2(short align) + 3*2(index '1' offset)
::new ([3]) long;
  }

4. Fixed align error message
5. Maybe fixed lint warnings


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76996

Files:
  clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
  clang/test/Analysis/placement-new.cpp

Index: clang/test/Analysis/placement-new.cpp
===
--- clang/test/Analysis/placement-new.cpp
+++ clang/test/Analysis/placement-new.cpp
@@ -155,3 +155,147 @@
   (void)dp;
 }
 } // namespace testHierarchy
+
+namespace testArrayTypesAllocation {
+void f1() {
+  struct S {
+short a;
+  };
+
+  // bad (not enough space).
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer1[sizeof(S) * N]; // expected-note {{'buffer1' initialized here}}
+  ::new (buffer1) S[N];// expected-warning{{Storage provided to placement new is only 64 bytes, whereas the allocated array type requires more space for internal needs}} expected-note 1 {{}}
+}
+
+void f2() {
+  struct S {
+short a;
+  };
+
+  // maybe ok but we need to warn.
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)]; // expected-note {{'buffer2' initialized here}}
+  ::new (buffer2) S[N];  // expected-warning{{Possibly not enough 68 bytes for array allocation which requires 64 bytes. Current overhead requires the size of 4 bytes}} expected-note 1 {{}}
+}
+} // namespace testArrayTypesAllocation
+
+namespace testStructAlign {
+void f1() {
+  struct X {
+char a[9];
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // bad (struct X is aligned to char).
+  ::new () long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f2() {
+  struct X {
+char a;
+char b;
+long c;
+  } Xi;
+
+  // ok (struct X is aligned to long).
+  ::new () long;
+}
+
+void f3() {
+  struct X {
+char a;
+char b;
+long c;
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // bad (struct X is aligned to long but field 'b' is aligned to 1 because of its offset)
+  ::new () long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f4() {
+  struct X {
+char a;
+struct alignas(alignof(short)) Y {
+  char b;
+  char c;
+} y;
+long d;
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // bad. 'b' is aligned to short
+  ::new () long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f5() {
+  short b[10]; // expected-note {{'b' initialized here}}
+
+  ::new () long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f6() {
+  short b[10]; // expected-note {{'b' initialized here}}
+
+  // bad (same as previous but checks ElementRegion case)
+  ::new ([0]) long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f7() {
+  short b[10];
+
+  // ok. 2(short align) + 3*2(index '1' offset)
+  ::new ([3]) long;
+}
+
+void f8() {
+  short b[10]; // expected-note {{'b' initialized here}}
+
+  // bad. 2(short align) + 2*2(index '2' offset)
+  ::new ([2]) long; // expected-warning{{Storage type is aligned to 6 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f9() {
+  struct X {
+char a;
+alignas(alignof(short)) char b[20];
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // ok 2(custom align) + 6(index '2' offset)
+  ::new ([6]) long;
+
+  // bad 2(custom align) + 1(index '2' offset)
+  ::new ([1]) long; // expected-warning{{Storage type is aligned to 3 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f10() {
+  struct X {
+char a[8];
+alignas(2) char b;
+  } Xi; // expected-note {{'Xi' initialized here}}
+
+  // bad (struct X is aligned to 2).
+  ::new () long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f11() {
+  struct X {
+char a;
+char b;
+struct Y {
+  long c;
+} d;
+  } Xi;
+
+  // ok (struct X is aligned to long).
+  ::new () long;
+}
+
+void f12() {
+  struct alignas(alignof(long)) X {
+char a;
+char b;
+  } Xi;
+
+  // ok (struct X is aligned to long).
+  ::new () long;
+}
+} // namespace testStructAlign
Index: 

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

2020-03-30 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo updated this revision to Diff 253623.
oontvoo added a comment.

Rebaase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/Inputs/dummy_pragma_once.h
  clang/test/Modules/Inputs/dummy_textual_header.h
  clang/test/Modules/Inputs/header_in_imported_module.h
  clang/test/Modules/Inputs/imported_module.cppm
  clang/test/Modules/Inputs/module.map
  clang/test/Modules/header-in-imported-module.c
  clang/test/Modules/import-pragma-once.c

Index: clang/test/Modules/import-pragma-once.c
===
--- /dev/null
+++ clang/test/Modules/import-pragma-once.c
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x c %s
+// expected-no-diagnostics
+#include "dummy_pragma_once.h"
+#include "dummy_textual_header.h"
+
+void *p = 
+void *q = 
Index: clang/test/Modules/header-in-imported-module.c
===
--- /dev/null
+++ clang/test/Modules/header-in-imported-module.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify  -x c++ %s
+//
+// RUN: %clang_cc1 -x c++ -fmodules-ts --precompile -o %t/ModuleB39206.pcm %S/Inputs/imported_module.cppm
+// RUN: %clang_cc1 -x c++ -fmodules-ts -c %t/ModuleB39206.pcm -o %t/ModuleB39206.o
+// RUN: %clang_cc1 -x c++ -fmodules-ts -fmodule-file=%t/ModuleB39206.pcm -verify  -c %s
+// expected-no-diagnostics
+
+// Bug 39206
+
+#include "header_in_imported_module.h"
+module ModuleB39206;
+
+#ifndef ALL_GOOD
+#error "missing macro in impl"
+#endif
Index: clang/test/Modules/Inputs/module.map
===
--- clang/test/Modules/Inputs/module.map
+++ clang/test/Modules/Inputs/module.map
@@ -282,6 +282,11 @@
   header "dummy.h"
 }
 
+module dummy_pragma_once {
+  header "dummy_pragma_once.h"
+  textual header "dummy_textual_header.h"
+}
+
 module builtin {
   header "builtin.h"
   explicit module sub {
Index: clang/test/Modules/Inputs/imported_module.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/imported_module.cppm
@@ -0,0 +1,5 @@
+export module ModuleB39206;
+#include "header.h"
+
+#ifndef ALL_GOOD
+#error "Missing macro in module decl"
\ No newline at end of file
Index: clang/test/Modules/Inputs/header_in_imported_module.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/header_in_imported_module.h
@@ -0,0 +1,3 @@
+#pragma once
+
+#define ALL_GOOD
Index: clang/test/Modules/Inputs/dummy_textual_header.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_textual_header.h
@@ -0,0 +1,2 @@
+#pragma once
+int y = 6;
Index: clang/test/Modules/Inputs/dummy_pragma_once.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_pragma_once.h
@@ -0,0 +1,3 @@
+#include "dummy_textual_header.h"
+
+int x = 5;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1621,7 +1621,7 @@
   endian::Writer LE(Out, little);
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
   LE.write(KeyLen);
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 2 + 4 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1678,6 +1678,9 @@
   }
   LE.write(Offset);
 
+  // Write this file UID.
+  LE.write(Data.HFI.UID);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1705,7 +1708,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch ) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch ) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator Generator;
   SmallVector SavedStrings;
@@ -1783,8 +1786,7 @@
 // changed since it was loaded. Also skip it if it's 

[PATCH] D76887: AMDGPU: Make HIPToolChain a subclass of ROCMToolChain

2020-03-30 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


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

https://reviews.llvm.org/D76887



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


[PATCH] D76365: [cuda][hip] Add CUDA builtin surface/texture reference support.

2020-03-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D76365#1947479 , @hliao wrote:

>


Nice! I'll file a bug with NVIDIA.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76365



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


[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

2020-03-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

**`$ git show --pretty=full 703a1b8caf09`**

  commit 703a1b8caf09a5262a45c2179b8131922f71cf25
  Author: Louis Dionne 
  Commit: Kirstóf Umann 
  
  [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy 
CallDescriptionMap
  
  ...

Not a phabricator bug. Maybe @Szelethus's local git somehow got in a weird 
state(?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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


[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

2020-03-30 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon updated this revision to Diff 253615.
Fznamznon added a comment.
Herald added a reviewer: jdoerfert.

Rebased to fresh version. Applied fixes after https://reviews.llvm.org/D70172


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaAvailability.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaSYCL/float128.cpp

Index: clang/test/SemaSYCL/float128.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/float128.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -triple spir64 -fsycl -fsycl-is-device -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsycl -fsycl-is-device -fsyntax-only %s
+
+template 
+class Z {
+public:
+  // TODO: If T is __float128 This might be a problem
+  T field;
+  //expected-error@+1 2{{'__float128' is not supported on this target}}
+  __float128 field1;
+};
+
+void host_ok(void) {
+  __float128 A;
+  int B = sizeof(__float128);
+  Z<__float128> C;
+  C.field1 = A;
+}
+
+void usage() {
+  //expected-error@+1 5{{'__float128' is not supported on this target}}
+  __float128 A;
+  Z<__float128> C;
+  //expected-error@+2 {{'A' is unavailable}}
+  //expected-error@+1 {{'field1' is unavailable}}
+  C.field1 = A;
+  //expected-error@+1 {{'A' is unavailable}}
+  decltype(A) D;
+
+  //expected-error@+1 {{'A' is unavailable}}
+  auto foo1 = [=]() {
+//expected-error@+1 {{'__float128' is not supported on this target}}
+__float128 AA;
+//expected-error@+1 {{'A' is unavailable}}
+auto BB = A;
+BB += 1;
+  };
+
+  //expected-note@+1 {{called by 'usage'}}
+  foo1();
+}
+
+template 
+void foo2(){};
+
+//expected-error@+1 {{'__float128 (__float128)' is not supported on this target}}
+__float128 foo(__float128 P) { return P; }
+
+template 
+__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
+  //expected-note@+1 5{{called by 'kernel}}
+  kernelFunc();
+  //expected-error@+1 {{'__float128' is not supported on this target}}
+  __float128 A;
+}
+
+int main() {
+  //expected-error@+1 2{{'__float128' is not supported on this target}}
+  __float128 CapturedToDevice = 1;
+  host_ok();
+  kernel([=]() {
+//expected-error@+1 {{'CapturedToDevice' is unavailable}}
+decltype(CapturedToDevice) D;
+//expected-error@+1 {{'CapturedToDevice' is unavailable}}
+auto C = CapturedToDevice;
+//expected-error@+1 {{'__float128' is not supported on this target}}
+__float128 ;
+Z<__float128> S;
+//expected-error@+1 {{'field1' is unavailable}}
+S.field1 += 1;
+S.field = 1;
+  });
+
+  kernel([=]() {
+//expected-note@+1 2{{called by 'operator()'}}
+usage();
+// expected-error@+1 2{{'__float128' is not supported on this target}}
+__float128 ;
+// expected-error@+2 {{'' is unavailable}}
+// expected-error@+1 {{'foo' is unavailable}}
+auto A = foo();
+  });
+
+  kernel([=]() {
+Z<__float128> S;
+foo2<__float128>();
+// TODO: this shouldn't be diagnosed
+// expected-error@+1 {{'__float128' is not supported on this target}}
+int E = sizeof(__float128);
+  });
+
+  return 0;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1517,10 +1517,21 @@
 break;
   case DeclSpec::TST_float128:
 if (!S.Context.getTargetInfo().hasFloat128Type() &&
+!S.getLangOpts().SYCLIsDevice &&
 !(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__float128";
 Result = Context.Float128Ty;
+if (!S.Context.getTargetInfo().hasFloat128Type() &&
+S.getLangOpts().SYCLIsDevice &&
+S.DelayedDiagnostics.shouldDelayDiagnostics()) {
+  S.DelayedDiagnostics.add(sema::DelayedDiagnostic::makeForbiddenType(
+  DS.getTypeSpecTypeLoc(), diag::err_type_unsupported, Result,
+  /*ignored*/ 0));
+  S.SYCLDiagIfDeviceCode(DS.getTypeSpecTypeLoc(),
+ diag::err_type_unsupported)
+  << Result;
+}
 break;
   case DeclSpec::TST_bool: Result = Context.BoolTy; break; // _Bool or bool
 break;
Index: clang/lib/Sema/SemaSYCL.cpp
===
--- /dev/null
+++ clang/lib/Sema/SemaSYCL.cpp
@@ -0,0 +1,54 @@
+//===- SemaSYCL.cpp - Semantic Analysis for SYCL constructs ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// 

[PATCH] D77070: [SemaObjC] Add a warning for declarations of BOOL:1 when BOOL is a signed char

2020-03-30 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, steven_wu.
Herald added subscribers: ributzka, dexonsmith, jkorous.

Instead, recommend using a real boolean type. Its possible to get away with 
BOOL:1, if you only read from it in a context where its contextually converted 
to bool, but its still broken if you, for instance, store it into a BOOL 
variable or try to compare it with another BOOL. Given that, I don't think 
there is any good reason to use it when there are better alternatives available.

rdar://29707989


https://reviews.llvm.org/D77070

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaObjC/signed-char-bool-bitfield-fixit.m
  clang/test/SemaObjC/signed-char-bool-bitfield.m

Index: clang/test/SemaObjC/signed-char-bool-bitfield.m
===
--- /dev/null
+++ clang/test/SemaObjC/signed-char-bool-bitfield.m
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify %s -Wobjc-signed-char-bool
+// RUN: %clang_cc1 -xobjective-c++ -verify %s -Wobjc-signed-char-bool
+
+typedef signed char BOOL;
+
+struct S {
+  BOOL b : 1; // expected-warning {{BOOL bit-field of width 1 cannot represent YES when BOOL is a signed type}}
+  BOOL b2 : 2;
+};
+
+@interface X {
+  BOOL b : 1; // expected-warning {{BOOL bit-field of width 1 cannot represent YES when BOOL is a signed type}}
+  BOOL b2 : 2;
+}
+@end
Index: clang/test/SemaObjC/signed-char-bool-bitfield-fixit.m
===
--- /dev/null
+++ clang/test/SemaObjC/signed-char-bool-bitfield-fixit.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -Wobjc-signed-char-bool %s -fdiagnostics-parseable-fixits 2>&1 | FileCheck %s
+
+typedef signed char BOOL;
+
+struct S {
+  BOOL bf : 1;
+  // CHECK: fix-it:"{{.*}}.m":{6:3-6:7}:"bool"
+};
+
+@interface X {
+  BOOL bf : 1;
+  // CHECK: fix-it:"{{.*}}.m":{11:3-11:7}:"bool"
+}
+@end
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16169,6 +16169,7 @@
 
 // Note that FieldName may be null for anonymous bitfields.
 ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
+SourceRange TypeRange,
 IdentifierInfo *FieldName,
 QualType FieldTy, bool IsMsStruct,
 Expr *BitWidth, bool *ZeroWidth) {
@@ -16259,6 +16260,16 @@
 Diag(FieldLoc, diag::warn_anon_bitfield_width_exceeds_type_width)
 << (unsigned)Value.getZExtValue() << (unsigned)TypeWidth;
 }
+
+if (getLangOpts().ObjC && NSAPIObj->isObjCBOOLType(FieldTy) &&
+FieldTy->isSpecificBuiltinType(BuiltinType::SChar) &&
+Value == 1) {
+  auto Builder = Diag(FieldLoc, diag::warn_objc_signed_char_bool_bitfield);
+  // The Foundation headers that define BOOL include , so its
+  // reasonable to assume that 'bool' is available.
+  if (TypeRange.isValid())
+Builder << FixItHint::CreateReplacement(TypeRange, "bool");
+}
   }
 
   return BitWidth;
@@ -16485,7 +16496,8 @@
 BitWidth = nullptr;
   // If this is declared as a bit-field, check the bit-field.
   if (BitWidth) {
-BitWidth = VerifyBitField(Loc, II, T, Record->isMsStruct(Context), BitWidth,
+BitWidth = VerifyBitField(Loc, TInfo->getTypeLoc().getSourceRange(),
+  II, T, Record->isMsStruct(Context), BitWidth,
   ).get();
 if (!BitWidth) {
   InvalidDecl = true;
@@ -16676,7 +16688,8 @@
 
   if (BitWidth) {
 // 6.7.2.1p3, 6.7.2.1p4
-BitWidth = VerifyBitField(Loc, II, T, /*IsMsStruct*/false, BitWidth).get();
+BitWidth = VerifyBitField(Loc, TInfo->getTypeLoc().getSourceRange(),
+  II, T, /*IsMsStruct*/false, BitWidth).get();
 if (!BitWidth)
   D.setInvalidType();
   } else {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -11401,9 +11401,10 @@
 
   /// VerifyBitField - verifies that a bit field expression is an ICE and has
   /// the correct width, and that the field type is valid.
-  /// Returns false on success.
+  /// Returns BitWidth on success.
   /// Can optionally return whether the bit-field is of width 0
-  ExprResult VerifyBitField(SourceLocation FieldLoc, IdentifierInfo *FieldName,
+  ExprResult VerifyBitField(SourceLocation FieldLoc, SourceRange TypeRange,
+IdentifierInfo *FieldName,
 QualType FieldTy, bool IsMsStruct,
 Expr *BitWidth, bool *ZeroWidth = nullptr);
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

2020-03-30 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:12248
+  ///   SYCLDiagIfDeviceCode(Loc, diag::err_type_unsupported) << "__float128";
+  DeviceDiagBuilder SYCLDiagIfDeviceCode(SourceLocation Loc, unsigned DiagID);
+

rjmccall wrote:
> Will this collect notes associated with the diagnostic correctly?
Could you please make your question a bit more concrete?
This function is supposed to work in the same way as 
`Sema::CUDADiagIfDeviceCode` and `Sema::diagIfOpenMPDeviceCode` . It emits 
given diagnostic if the current context is known as "device code" and makes 
this diagnostic deferred otherwise. It uses the `DeviceDiagBuilder` which was 
implemented earlier. This `DeviceDiagBuilder` also tries to emit callstack 
notes for the given diagnostics. Do you mean these callstack notes or something 
else?



Comment at: clang/lib/Sema/SemaAvailability.cpp:479
+case UnavailableAttr::IR_SYCLForbiddenType:
+  diag_available_here = diag::err_type_unsupported;
+  break;

rjmccall wrote:
> All of the other cases are setting this to a note, not an error, so I suspect 
> this will read wrong.
Yes, this is not a note. For such samples:

```
int main() {
  __float128 CapturedToDevice = 1;
  kernel([=]() {
decltype(CapturedToDevice) D;
  });
}
```
It looks like this:
```
float128.cpp:63:14: error: 'CapturedToDevice' is unavailable
decltype(CapturedToDevice) D;
 ^
float128.cpp:59:14: error: '__float128' is not supported on this target   /// 
This emitted instead of note 
  __float128 CapturedToDevice = 1;
 ^
```
I had feeling that it should probably be a note. But there is no implemented 
note for unsupported types. I think I can add a new one if it will make it 
better. Should I?



Comment at: clang/lib/Sema/SemaAvailability.cpp:534
+if (S.getLangOpts().SYCLIsDevice)
+  S.SYCLDiagIfDeviceCode(Loc, diag) << ReferringDecl;
+else

rjmccall wrote:
> Are you sure you want to be applying this to all of the possible diagnostics 
> here, rather than just for SYCLForbiddenType unavailable attributes?
I suppose it is reasonable if we want to reuse unavaliable attribute for other 
SYCL use cases. Plus, In SYCL we don't know where is device code until we 
instantiate templates, it happens late, so we have to defer any diagnostic 
while compiling for device, otherwise we can point to host code where much more 
is allowed.



Comment at: clang/lib/Sema/SemaDecl.cpp:18030
+  if (LangOpts.SYCLIsDevice && FD->hasAttr())
+return FunctionEmissionStatus::Emitted;
+

rjmccall wrote:
> So you want to emit it for the definition in addition to emitting it for 
> specific specializations?
Somehow diagnostics are emitted only for the definitions. 
Without this change diagnostics aren't emitted at all.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7771
+return true;
+  }
+

rjmccall wrote:
> I wonder if it's reasonable to treat all forbidden types the same here or if 
> we want different functions for the ARC and SYCL use cases.
I think it could be reasonable if we will have forbidden type cases for SYCL 
sometime. For now, I don't see the purpose in a separate function for SYCL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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


[PATCH] D77061: [analyzer] Add core.CallAndMessage to StdCLibraryFunctionArgsChecker's dependency

2020-03-30 Thread Gabor Marton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a1bb876dba4: [analyzer] Add core.CallAndMessage to 
StdCLibraryFunctionArgsCheckers… (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77061

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/test/Analysis/analyzer-enabled-checkers.c


Index: clang/test/Analysis/analyzer-enabled-checkers.c
===
--- clang/test/Analysis/analyzer-enabled-checkers.c
+++ clang/test/Analysis/analyzer-enabled-checkers.c
@@ -7,11 +7,11 @@
 // CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: apiModeling.StdCLibraryFunctionArgs
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
-// CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
 // CHECK-NEXT: core.NonNullParamChecker
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -299,7 +299,7 @@
   HelpText<"Check constraints of arguments of C standard library functions, "
"such as whether the parameter of isalpha is in the range [0, 255] "
"or is EOF.">,
-  Dependencies<[StdCLibraryFunctionsChecker]>,
+  Dependencies<[StdCLibraryFunctionsChecker, CallAndMessageChecker]>,
   Documentation;
 
 def TrustNonnullChecker : Checker<"TrustNonnull">,


Index: clang/test/Analysis/analyzer-enabled-checkers.c
===
--- clang/test/Analysis/analyzer-enabled-checkers.c
+++ clang/test/Analysis/analyzer-enabled-checkers.c
@@ -7,11 +7,11 @@
 // CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: apiModeling.StdCLibraryFunctionArgs
 // CHECK-NEXT: apiModeling.TrustNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
-// CHECK-NEXT: core.CallAndMessage
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
 // CHECK-NEXT: core.NonNullParamChecker
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -299,7 +299,7 @@
   HelpText<"Check constraints of arguments of C standard library functions, "
"such as whether the parameter of isalpha is in the range [0, 255] "
"or is EOF.">,
-  Dependencies<[StdCLibraryFunctionsChecker]>,
+  Dependencies<[StdCLibraryFunctionsChecker, CallAndMessageChecker]>,
   Documentation;
 
 def TrustNonnullChecker : Checker<"TrustNonnull">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D77068: [XCore] fix crash on unused inline in EmitTargetMetadata

2020-03-30 Thread Nigel Perks via Phabricator via cfe-commits
nigelp-xmos created this revision.
nigelp-xmos added a reviewer: rsmith.
nigelp-xmos added a project: clang.

EmitTargetMetadata passes to emitTargetMD a null pointer as returned from 
GetGlobalValue for an unused inline function which has been removed from the 
module at that point.

Richard Smith comments in CodeGenModule.cpp that the calling code in 
EmitTargetMetadata should be moved into the one target that needs it (XCore). I 
thought it best to start with the quicker change, restricted to XCore code, 
checking for null, to prevent the crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77068

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/xcore-unused-inline.c


Index: clang/test/CodeGen/xcore-unused-inline.c
===
--- /dev/null
+++ clang/test/CodeGen/xcore-unused-inline.c
@@ -0,0 +1,4 @@
+// REQUIRES: xcore-registered-target
+// RUN: %clang_cc1 -triple xcore-unknown-unknown -emit-llvm -o - %s
+
+inline void dead_function(void) {}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -9354,6 +9354,8 @@
 /// XCore uses emitTargetMD to emit TypeString metadata for global symbols.
 void XCoreTargetCodeGenInfo::emitTargetMD(const Decl *D, llvm::GlobalValue *GV,
   CodeGen::CodeGenModule ) const {
+  if (!GV)
+return;
   SmallStringEnc Enc;
   if (getTypeString(Enc, D, CGM, TSC)) {
 llvm::LLVMContext  = CGM.getModule().getContext();


Index: clang/test/CodeGen/xcore-unused-inline.c
===
--- /dev/null
+++ clang/test/CodeGen/xcore-unused-inline.c
@@ -0,0 +1,4 @@
+// REQUIRES: xcore-registered-target
+// RUN: %clang_cc1 -triple xcore-unknown-unknown -emit-llvm -o - %s
+
+inline void dead_function(void) {}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -9354,6 +9354,8 @@
 /// XCore uses emitTargetMD to emit TypeString metadata for global symbols.
 void XCoreTargetCodeGenInfo::emitTargetMD(const Decl *D, llvm::GlobalValue *GV,
   CodeGen::CodeGenModule ) const {
+  if (!GV)
+return;
   SmallStringEnc Enc;
   if (getTypeString(Enc, D, CGM, TSC)) {
 llvm::LLVMContext  = CGM.getModule().getContext();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >