[PATCH] D71594: testing clang-tidy

2019-12-17 Thread Mikhail Goncharov via Phabricator via cfe-commits
goncharov added a comment.

In D71594#1788194 , @Eugene.Zelenko 
wrote:

> Where set of Clang-tidy checks come from? .clang-tidy? Could stricter set of 
> custom rules be used?


Yes, clang-tidy uses the .clang-tidy. Do you want to enable additional rules 
for a subproject?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71594



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


[PATCH] D71635: [clang] Rename -frounding-math to -fexperimental-rounding-math and add -frounding-math back as a gcc-compat arg.

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60990 tests passed, 1 failed 
and 727 were skipped.

  failed: lit.lit/shtest-format.py

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71635



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


[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

2019-12-17 Thread Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits
khchen planned changes to this revision.
khchen added a subscriber: eli.friedman.
khchen added a comment.

I asked @kito-cheng about the GCC LTO behavior, 
GCC LTO encodes the ABI info in elf and the behavior in above examples match to 
@efriedma 's responses.
So I think maybe encode ABI in module metadata flag is a good ideal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71387



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


[PATCH] D71648: [WebAssembly] Add avgr_u intrinsics and require nuw in patterns

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60989 tests passed, 0 failed 
and 727 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71648



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


[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

2019-12-17 Thread Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits
khchen added a comment.

In D71387#1788472 , @efriedma wrote:

> > Unfortunately on RISCV, the ABI info is only derived from -mabi option and 
> > the target triple does not encode ABI info (same to gcc).
>
> How does gcc decide the default ABI for a target?  Do you explicitly specify 
> it at configure time?


Yes, default March/ABI are decided in configure time, and there is also default 
March/ABI there.

>> example 1:
>>  What's expected behavior? user specifics -mabi=lp64f so the result object 
>> ABI is lp64f?
> 
> I think the expected behavior is a fatal error.  There isn't any reason to 
> let people shoot themselves in the foot like this.  (module flags have 
> builtin support for producing such an error.)
> 
>> example 2:
>>  What's expected behavior? The result object is -mabi=lp64f because they 
>> have same ABI in module metadata?
> 
> Yes.
> 
>> there is no problem (I think) when overwriting the original ABI. (so the 
>> mixing ABI is not a problem)
> 
> clang emits different IR for hardfloat vs. softfloat targets; if you try to 
> overwrite the ABI of a bitcode file, we'll generate wrong code.

Do you mean different function attributes for different ABI targets?
I see there are `arm_aapcs_vfpcc` annotation and `use-soft-float=false` in 
attributes when giving the `-mfloat-abi=hard` option.
But in RISCV clang emits the same IR for different ABI (-mabi), so maybe it is 
no problem in RISCV?  (-mfloat-abi is unused in riscv clang driver)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71387



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


[PATCH] D71648: [WebAssembly] Add avgr_u intrinsics and require nuw in patterns

2019-12-17 Thread Thomas Lively via Phabricator via cfe-commits
tlively created this revision.
tlively added a reviewer: aheejin.
Herald added subscribers: llvm-commits, cfe-commits, sunfish, hiraditya, 
jgravelle-google, sbc100, dschuff.
Herald added projects: clang, LLVM.

The vector pattern `(a + b + 1) / 2` was previously selected to an
avgr_u instruction regardless of nuw flags, but this is incorrect in
the case where either addition may have an unsigned wrap. This CL
changes the existing pattern to require both adds to have nuw flags
and adds builtin functions and intrinsics for the avgr_u instructions
because the corrected pattern is not representable in C.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71648

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-arith.ll
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll

Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -65,6 +65,16 @@
   ret <16 x i8> %a
 }
 
+; CHECK-LABEL: avgr_u_v16i8:
+; SIMD128-NEXT: .functype avgr_u_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.avgr_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.avgr.unsigned.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @avgr_u_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.avgr.unsigned.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
 ; CHECK-LABEL: any_v16i8:
 ; SIMD128-NEXT: .functype any_v16i8 (v128) -> (i32){{$}}
 ; SIMD128-NEXT: i8x16.any_true $push[[R:[0-9]+]]=, $0{{$}}
@@ -168,6 +178,16 @@
   ret <8 x i16> %a
 }
 
+; CHECK-LABEL: avgr_u_v8i16:
+; SIMD128-NEXT: .functype avgr_u_v8i16 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i16x8.avgr_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <8 x i16> @llvm.wasm.avgr.unsigned.v8i16(<8 x i16>, <8 x i16>)
+define <8 x i16> @avgr_u_v8i16(<8 x i16> %x, <8 x i16> %y) {
+  %a = call <8 x i16> @llvm.wasm.avgr.unsigned.v8i16(<8 x i16> %x, <8 x i16> %y)
+  ret <8 x i16> %a
+}
+
 ; CHECK-LABEL: any_v8i16:
 ; SIMD128-NEXT: .functype any_v8i16 (v128) -> (i32){{$}}
 ; SIMD128-NEXT: i16x8.any_true $push[[R:[0-9]+]]=, $0{{$}}
Index: llvm/test/CodeGen/WebAssembly/simd-arith.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-arith.ll
+++ llvm/test/CodeGen/WebAssembly/simd-arith.ll
@@ -97,6 +97,19 @@
 ; SIMD128-NEXT: i8x16.avgr_u $push[[R:[0-9]+]]=, $0, $1{{$}}
 ; SIMD128-NEXT: return $pop[[R]]{{$}}
 define <16 x i8> @avgr_u_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = add nuw <16 x i8> %x, %y
+  %b = add nuw <16 x i8> %a, 
+  %c = udiv <16 x i8> %b, 
+  ret <16 x i8> %c
+}
+
+; CHECK-LABEL: avgr_u_v16i8_wrap:
+; NO-SIMD128-NOT: i8x16
+; SIMD128-NEXT: .functype avgr_u_v16i8_wrap (v128, v128) -> (v128){{$}}
+; SIMD128-NOT: i8x16.avgr_u
+define <16 x i8> @avgr_u_v16i8_wrap(<16 x i8> %x, <16 x i8> %y) {
   %a = add <16 x i8> %x, %y
   %b = add <16 x i8> %a, 
@@ -401,6 +414,17 @@
 ; SIMD128-NEXT: i16x8.avgr_u $push[[R:[0-9]+]]=, $0, $1{{$}}
 ; SIMD128-NEXT: return $pop[[R]]{{$}}
 define <8 x i16> @avgr_u_v8i16(<8 x i16> %x, <8 x i16> %y) {
+  %a = add nuw <8 x i16> %x, %y
+  %b = add nuw <8 x i16> %a, 
+  %c = udiv <8 x i16> %b, 
+  ret <8 x i16> %c
+}
+
+; CHECK-LABEL: avgr_u_v8i16_wrap:
+; NO-SIMD128-NOT: i16x8
+; SIMD128-NEXT: .functype avgr_u_v8i16_wrap (v128, v128) -> (v128){{$}}
+; SIMD128-NOT: i16x8.avgr_u
+define <8 x i16> @avgr_u_v8i16_wrap(<8 x i16> %x, <8 x i16> %y) {
   %a = add <8 x i16> %x, %y
   %b = add <8 x i16> %a, 
   %c = udiv <8 x i16> %b, 
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
@@ -739,23 +739,24 @@
 } // isCommutable = 1
 
 // Integer unsigned rounding average: avgr_u
-def avgr_u_v16i8 :
-  PatFrag<(ops node:$lhs, node:$rhs),
-  (srl
-(add (add node:$lhs, node:$rhs), (splat16 (i32 1))),
-(v16i8 (splat16 (i32 1)))
-  )>;
-def avgr_u_v8i16 :
-  PatFrag<(ops node:$lhs, node:$rhs),
-  (srl
-(add (add node:$lhs, node:$rhs), (splat8 (i32 1))),
-(v8i16 (splat8 (i32 1)))
-  )>;
-
 let isCommutable = 1, Predicates = [HasUnimplementedSIMD128] in {
-defm AVGR_U : SIMDBinary;
-defm AVGR_U : SIMDBinary;
-}
+defm AVGR_U : SIMDBinary;
+defm AVGR_U : SIMDBinary;
+}
+
+def add_nuw : PatFrag<(ops node:$lhs, node:$rhs),
+  (add node:$lhs, node:$rhs),
+  "return N->getFlags().hasNoUnsignedWrap();">;
+
+fore

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

2019-12-17 Thread Kan Shengchen via Phabricator via cfe-commits
skan added a comment.

In D70157#1788445 , @reames wrote:

> Specifically on the revised patch, I remain confused by the need for multiple 
> subtypes.  The need for fragments *between* the potentially fused 
> instructions doesn't make sense to me.  What I was expecting to see was the 
> following:
>  BoundaryAlign w/target=the branch fragment
>  .. some possibly empty sequence of fragments (i.e. the test/cmp/etc..) ...
>  the branch fragment
>  a new data fragment if the branch fragment was a DF
>
> (i.e. a single BounaryAlign fragment which aligns a payload which is defined 
> as "next fragment to target fragment inclusive".)
>
> To be specific, I'd expect to see the following for an example fused sequence:
>
> 1. BoundaryAlign w/Target = 3
> 2. DataFragment containing TEST RAX, RAX
> 3. RelaxeableFragment containing JNE symbo
>
>   Why do we need anything between the two fragments of the fused pair?
>
>   (As a reminder, I am new to this code.  If I'm missing the obvious, please 
> just point it out.)


JUMP is not always emiteed into `MCRelaxableFragment`, it also can be emitted 
into `MCDataFragment` and  arithmetic ops with constant arguments of unknown 
value (e.g. ADD,AND) can be emitted into 'MCRelaxableFragment' ,  you can find 
related code in `MCObjectStreamer::EmitInstructionImpl`, 
'X86AsmBackend::mayNeedRelaxation'.  Let's say JCC is fused with TEST,  there 
are four possible positions for JCC and CMP

1. JCC and CMP are in same `MCDataFragment`
2. JCC and CMP are in  two different `MCDataFragment`
3. JCC and CMP are in two different `MCRelaxableFragment`
4. JCC in a `MCRelaxableFragment`, CMP is in a `MCDataFragment`

and since `MCCompactEncodedInstFragment` is not applicable yet, i don't what's 
its behaviour.

In order to compute the total size of CMP and JCC in 
`MCAssembler::relaxBoundaryAlign`, I insert a `FusedJccSplit` to force CMP and 
JCC in two fragments. Do you have any better idea?


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

https://reviews.llvm.org/D70157



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


[PATCH] D71644: [clangd] Heuristically resolve dependent call through smart pointer type

2019-12-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge marked an inline comment as done.
nridge added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:116
+  // smart pointer type.
+  ASTContext *Ctx = hackyFindASTContext(T);
+  if (!Ctx)

I don't intend for this function to be in the final patch.

Rather, I'm wondering: am I missing some obvious general way to recover the 
`ASTContext` from a `Type` (or a `Stmt`)?

If not, I think we'll need to modify `targetDecl()`, `allTargetDecls()`, 
`findExplicitReferences()` etc. to have their callers pass in an `ASTContext`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71644



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


[PATCH] D71644: [clangd] Heuristically resolve dependent call through smart pointer type

2019-12-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Fixes https://github.com/clangd/clangd/issues/227


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71644

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -502,7 +502,7 @@
 
   R"cpp(// FIXME: Heuristic resolution of dependent method
 // invoked via smart pointer
-template  struct S { void foo(); };
+template  struct S { void [[foo]]() {} };
 template  struct unique_ptr {
   T* operator->();
 };
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
@@ -86,6 +87,58 @@
   });
 }
 
+ASTContext *hackyFindASTContext(const Type *T) {
+  auto *TST = T->getAs();
+  if (!TST)
+return nullptr;
+  const ClassTemplateDecl *TD = dyn_cast_or_null(
+  TST->getTemplateName().getAsTemplateDecl());
+  if (!TD)
+return nullptr;
+  return &TD->getASTContext();
+}
+
+// Given the type T of a dependent expression that appears of the LHS of a 
"->",
+// heuristically find a corresponding pointee type in whose scope we could look
+// up the name appearing on the RHS.
+const Type *getPointeeType(const Type *T) {
+  if (!T)
+return nullptr;
+
+  if (T->isPointerType()) {
+return T->getAs()->getPointeeType().getTypePtrOrNull();
+  }
+
+  // Try to handle smart pointer types.
+
+  // Look up operator-> in the primary template. If we find one, it's probably 
a
+  // smart pointer type.
+  ASTContext *Ctx = hackyFindASTContext(T);
+  if (!Ctx)
+return nullptr;
+  auto ArrowOps = getMembersReferencedViaDependentName(
+  T, Ctx->DeclarationNames.getCXXOperatorName(OO_Arrow),
+  /*IsNonStaticMember=*/true);
+  if (ArrowOps.empty())
+return nullptr;
+
+  // Getting the return type of the found operator-> method decl isn't useful,
+  // because we discarded template arguments to perform lookup in the primary
+  // template scope, so the return type would just have the form U* where U is 
a
+  // template parameter type.
+  // Instead, just handle the common case where the smart pointer type has the
+  // form of SmartPtr, and assume X is the pointee type.
+  auto *TST = T->getAs();
+  if (!TST)
+return nullptr;
+  if (TST->getNumArgs() == 0)
+return nullptr;
+  const TemplateArgument &FirstArg = TST->getArg(0);
+  if (FirstArg.getKind() != TemplateArgument::Type)
+return nullptr;
+  return FirstArg.getAsType().getTypePtrOrNull();
+}
+
 // TargetFinder locates the entities that an AST node refers to.
 //
 // Typically this is (possibly) one declaration and (possibly) one type, but
@@ -250,14 +303,7 @@
   VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
 const Type *BaseType = E->getBaseType().getTypePtrOrNull();
 if (E->isArrow()) {
-  // FIXME: Handle smart pointer types by looking up operator->
-  // in the primary template.
-  if (!BaseType || !BaseType->isPointerType()) {
-return;
-  }
-  BaseType = BaseType->getAs()
- ->getPointeeType()
- .getTypePtrOrNull();
+  BaseType = getPointeeType(BaseType);
 }
 for (const NamedDecl *D :
  getMembersReferencedViaDependentName(BaseType, E->getMember(),


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -502,7 +502,7 @@
 
   R"cpp(// FIXME: Heuristic resolution of dependent method
 // invoked via smart pointer
-template  struct S { void foo(); };
+template  struct S { void [[foo]]() {} };
 template  struct unique_ptr {
   T* operator->();
 };
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -27,6 +27,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeLocVisitor.h"
 #include "clang/Basic/LangOptions.h

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

2019-12-17 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

Btw, this patch did not apply cleanly on trunk. I instead test the version from 
your repository 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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


[PATCH] D71439: Allow redeclaration of __declspec(uuid)

2019-12-17 Thread Zachary Henkel via Phabricator via cfe-commits
zahen added a comment.

@rnk Can you please submit on my behalf.  I don't have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71439



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


[PATCH] D70615: Add an -fno-temp-file flag for compilation

2019-12-17 Thread Zachary Henkel via Phabricator via cfe-commits
zahen added a comment.

Any additional changes required?  If not could someone please submit on my 
behalf?  @rnk, @hans, @thakis ?


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

https://reviews.llvm.org/D70615



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


[PATCH] D71533: [clangd] Show template arguments in type hierarchy when possible

2019-12-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge marked an inline comment as done.
nridge added a comment.

In D71533#1785376 , @kadircet wrote:

> I think this requires changes in other places too, for example when querying 
> index for the children we rather want to query using the symbolid of template 
> pattern, not the instantiation.


Good catch! I added a couple of additional test cases to exercise this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71533



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


[PATCH] D71533: [clangd] Show template arguments in type hierarchy when possible

2019-12-17 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 234446.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71533

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -409,17 +409,13 @@
   ASSERT_TRUE(!AST.getDiagnostics().empty());
 
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
-  // FIXME(nridge): It would be preferable if the type hierarchy gave us type
-  // names (e.g. "S<0>" for the child and "S<1>" for the parent) rather than
-  // template names (e.g. "S").
+  // Here, we actually don't get any parents, because the unbounded hierarchy
+  // causes instantiation of the base specifier to fail.
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
-  EXPECT_THAT(
-  *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), Parents();
+  EXPECT_THAT(*Result,
+  AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct), Parents()));
 }
 
 TEST(TypeHierarchy, RecursiveHierarchyBounded) {
@@ -449,9 +445,12 @@
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
   *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), Parents();
+  AllOf(WithName("S<2>"), WithKind(SymbolKind::Struct),
+Parents(AllOf(
+WithName("S<1>"), WithKind(SymbolKind::Struct),
+SelectionRangeIs(Source.range("SDef")),
+Parents(AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct),
+  Parents()));
   Result = getTypeHierarchy(AST, Source.point("SRefDependent"), 0,
 TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
@@ -462,6 +461,60 @@
   SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
+TEST(TypeHierarchy, DeriveFromImplicitSpec) {
+  Annotations Source(R"cpp(
+  template 
+  struct Parent {};
+
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(*Result,
+  AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+Children(AllOf(WithName("Child"),
+   WithKind(SymbolKind::Struct), Children();
+}
+
+TEST(TypeHierarchy, DeriveFromTemplate) {
+  Annotations Source(R"cpp(
+  template 
+  struct Parent {};
+
+  template 
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  // FIXME: We'd like this to return the implicit specialization Child,
+  //but currently libIndex does not expose relationships between
+  //implicit specializations.
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(*Result,
+  AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+Children(AllOf(WithName("Child"),
+   WithKind(SymbolKind::Struct), Children();
+}
+
 SymbolID findSymbolIDByName(SymbolIndex *Index, llvm::StringRef Name,
 llvm::StringRef TemplateArgs = "") {
   SymbolID Result;
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -674,13 +674,25 @@
   const SourceManager &SM = AST.getSourceManager();
   SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
   getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
-  DeclRelationSet Relations =
-  DeclRelation::TemplatePattern | DeclRelation::Underlying;
+  // Search for both template instantiations and template patterns.
+  // We prefer the former, if available (generally, one will be
+  // av

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

2019-12-17 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht closed this revision.
rupprecht added a comment.

In D62731#1788902 , @andrew.w.kaylor 
wrote:

> In D62731#1788838 , @rupprecht wrote:
>
> > It seems the discussion of whether or not this is incomplete died out -- 
> > I'd prefer to assume it is incomplete if there is no consensus. Mailed 
> > D71635  to rename `-frounding-math` to 
> > `-fexperimental-rounding-math`.
> >
> > Alternatively we could remove the warning. I still don't see a good 
> > argument for the middle ground of having it called `-frounding-math` but 
> > also generate a warning.
>
>
> It's definitely incomplete but the results will not be any worse than you get 
> when -frounding-math is ignored.
>
> My preference would be to change the text of the warning that is issued but 
> allow -frounding-math to be enabled by this commit without requiring an 
> additional option.


If other reviewers agree, then let's just remove the warning. I can send a 
patch tomorrow unless someone else wants to do that.

> I would also very much like to see this patch re-committed. It's currently in 
> the "approved" state. If anyone objects to this being committed, please use 
> the "request changes" action to indicate this.

It is already re-committed. 7f9b5138470db1dc58f3bc05631284c653c9ed7a 
 reapplied 
it, but IIUC it was not closed in phabricator due to leading whitespace in the 
commit message:

  Reapply af57dbf12e54 "Add support for options -frounding-math, 
ftrapping-math, -ffp-model=, and -ffp-exception-behavior="
  ...
  Differential Revision: https://reviews.llvm.org/D62731

The "Differential" needs to be the first thing, whitespace cannot come before 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[Diffusion] rG825235c140e7: Revert "[Sema] Use the canonical type in function isVector"

2019-12-17 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

I'm still not sure why `__fp16`, which is a storage-only type, is used for the 
element type of `float16x4_t` if we want to avoid promotion to a float vector 
type.

https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point

Was there a discussion on llvm-dev or phabricator?


BRANCHES
  master

Users:
  ahatanak (Author)

https://reviews.llvm.org/rG825235c140e7



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


[PATCH] D70836: [analysis] Fix value tracking for pointers to qualified types

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

Great! I have one question though. Will this also work as intended with sugared 
types?  (e.g. typedefs)
I believe this might be one of the main reason why the original author used 
canonical types in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70836



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


[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 234436.
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.

- Use range based for.


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

https://reviews.llvm.org/D71642

Files:
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/test/Analysis/aggrinit-cfg-output.cpp
  clang/test/Analysis/analyzer-config.c

Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -19,6 +19,7 @@
 // CHECK-NEXT: c++-temp-dtor-inlining = true
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
+// CHECK-NEXT: cfg-expand-default-aggr-inits = false
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
@@ -98,4 +99,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 95
+// CHECK-NEXT: num-entries = 96
Index: clang/test/Analysis/aggrinit-cfg-output.cpp
===
--- /dev/null
+++ clang/test/Analysis/aggrinit-cfg-output.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-expand-default-aggr-inits=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+static char a[] = "foobar";
+
+struct StringRef {
+  const char *member = nullptr;
+  int len = 3;
+};
+
+int main() {
+  StringRef s{a};
+  (void)s;
+}
+
+// CHECK: [B1]
+// CHECK-NEXT:   1: a
+// CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, ArrayToPointerDecay, char *)
+// CHECK-NEXT:   3: [B1.2] (ImplicitCastExpr, NoOp, const char *)
+// CHECK-NEXT:   4: 3
+// CHECK-NEXT:   5: 
+// CHECK-NEXT:   6: {[B1.1]}
+// CHECK-NEXT:   7: StringRef s{a};
+// CHECK-NEXT:   8: s
+// CHECK-NEXT:   9: (void)[B1.8] (CStyleCastExpr, ToVoid, void)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+
Index: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
@@ -44,6 +44,8 @@
   options(Options) {
   AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
   AnaCtxMgr.getCFGBuildOptions().OmitImplicitValueInitializers = true;
+  AnaCtxMgr.getCFGBuildOptions().AddCXXDefaultInitExprInAggregates =
+  Options.ShouldIncludeDefaultInitForAggregates;
 }
 
 AnalysisManager::~AnalysisManager() {
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -542,6 +542,7 @@
 
 private:
   // Visitors to walk an AST and construct the CFG.
+  CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
   CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
   CFGBlock *VisitBinaryOperator(BinaryOperator *B, AddStmtChoice asc);
   CFGBlock *VisitBreakStmt(BreakStmt *B);
@@ -2140,6 +2141,9 @@
 return Block;
   return VisitStmt(S, asc);
 
+case Stmt::InitListExprClass:
+  return VisitInitListExpr(cast(S), asc);
+
 case Stmt::AddrLabelExprClass:
   return VisitAddrLabelExpr(cast(S), asc);
 
@@ -2346,15 +2350,37 @@
   // Visit the children in their reverse order so that they appear in
   // left-to-right (natural) order in the CFG.
   reverse_children RChildren(S);
-  for (reverse_children::iterator I = RChildren.begin(), E = RChildren.end();
-   I != E; ++I) {
-if (Stmt *Child = *I)
+  for (Stmt *Child : RChildren) {
+if (Child)
   if (CFGBlock *R = Visit(Child))
 B = R;
   }
   return B;
 }
 
+CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
+  if (asc.alwaysAdd(*this, ILE)) {
+autoCreateBlock();
+appendStmt(Block, ILE);
+  }
+  CFGBlock *B = Block;
+
+  reverse_children RChildren(ILE);
+  for (Stmt *Child : RChildren) {
+if (!Child)
+  continue;
+if (CFGBlock *R = Visit(Child))
+  B = R;
+if (BuildOpts.AddCXXDefaultInitExprInAggregates) {
+  if (auto *DIE = dyn_cast(Child))
+if (Stmt *Child = DIE->getExpr())
+  if (CFGBlock *R = Visit(Child))
+B = R;
+}
+  }
+  return B;
+}
+
 CFGBlock *CFGBuilder::VisitAddrLabelExpr(AddrLabelExpr *A,
  AddStmtChoice asc) {
   AddressTakenLabels.insert(A->getLabel());
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -112,6 +112,12 

[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGea93d7d64216: [CFG] Add an option to expand 
CXXDefaultInitExpr into aggregate initialization (authored by xazax.hun).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71642

Files:
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/test/Analysis/aggrinit-cfg-output.cpp
  clang/test/Analysis/analyzer-config.c

Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -19,6 +19,7 @@
 // CHECK-NEXT: c++-temp-dtor-inlining = true
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
+// CHECK-NEXT: cfg-expand-default-aggr-inits = false
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
@@ -98,4 +99,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 95
+// CHECK-NEXT: num-entries = 96
Index: clang/test/Analysis/aggrinit-cfg-output.cpp
===
--- /dev/null
+++ clang/test/Analysis/aggrinit-cfg-output.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-expand-default-aggr-inits=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+static char a[] = "foobar";
+
+struct StringRef {
+  const char *member = nullptr;
+  int len = 3;
+};
+
+int main() {
+  StringRef s{a};
+  (void)s;
+}
+
+// CHECK: [B1]
+// CHECK-NEXT:   1: a
+// CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, ArrayToPointerDecay, char *)
+// CHECK-NEXT:   3: [B1.2] (ImplicitCastExpr, NoOp, const char *)
+// CHECK-NEXT:   4: 3
+// CHECK-NEXT:   5: 
+// CHECK-NEXT:   6: {[B1.1]}
+// CHECK-NEXT:   7: StringRef s{a};
+// CHECK-NEXT:   8: s
+// CHECK-NEXT:   9: (void)[B1.8] (CStyleCastExpr, ToVoid, void)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+
Index: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
@@ -44,6 +44,8 @@
   options(Options) {
   AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
   AnaCtxMgr.getCFGBuildOptions().OmitImplicitValueInitializers = true;
+  AnaCtxMgr.getCFGBuildOptions().AddCXXDefaultInitExprInAggregates =
+  Options.ShouldIncludeDefaultInitForAggregates;
 }
 
 AnalysisManager::~AnalysisManager() {
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -542,6 +542,7 @@
 
 private:
   // Visitors to walk an AST and construct the CFG.
+  CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
   CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
   CFGBlock *VisitBinaryOperator(BinaryOperator *B, AddStmtChoice asc);
   CFGBlock *VisitBreakStmt(BreakStmt *B);
@@ -2140,6 +2141,9 @@
 return Block;
   return VisitStmt(S, asc);
 
+case Stmt::InitListExprClass:
+  return VisitInitListExpr(cast(S), asc);
+
 case Stmt::AddrLabelExprClass:
   return VisitAddrLabelExpr(cast(S), asc);
 
@@ -2346,15 +2350,37 @@
   // Visit the children in their reverse order so that they appear in
   // left-to-right (natural) order in the CFG.
   reverse_children RChildren(S);
-  for (reverse_children::iterator I = RChildren.begin(), E = RChildren.end();
-   I != E; ++I) {
-if (Stmt *Child = *I)
+  for (Stmt *Child : RChildren) {
+if (Child)
   if (CFGBlock *R = Visit(Child))
 B = R;
   }
   return B;
 }
 
+CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
+  if (asc.alwaysAdd(*this, ILE)) {
+autoCreateBlock();
+appendStmt(Block, ILE);
+  }
+  CFGBlock *B = Block;
+
+  reverse_children RChildren(ILE);
+  for (Stmt *Child : RChildren) {
+if (!Child)
+  continue;
+if (CFGBlock *R = Visit(Child))
+  B = R;
+if (BuildOpts.AddCXXDefaultInitExprInAggregates) {
+  if (auto *DIE = dyn_cast(Child))
+if (Stmt *Child = DIE->getExpr())
+  if (CFGBlock *R = Visit(Child))
+B = R;
+}
+  }
+  return B;
+}
+
 CFGBlock *CFGBuilder::VisitAddrLabelExpr(AddrLabelExpr *A,
  AddStmtChoice asc) {
   AddressTakenLabels.insert(A->getLabel());
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===
--- clang/include/clang/StaticAnalyzer/

[clang] ea93d7d - [CFG] Add an option to expand CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gabor Horvath via cfe-commits

Author: Gabor Horvath
Date: 2019-12-17T17:56:06-08:00
New Revision: ea93d7d6421612e9ea51b321eaf97fbdd64fe39b

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

LOG: [CFG] Add an option to expand CXXDefaultInitExpr into aggregate 
initialization

This is useful for clients that are relying on linearized CFGs for evaluating
subexpressions and want the default initializer to be evaluated properly.

The upcoming lifetime analysis is using this but it might also be useful
for the static analyzer at some point.

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

Added: 
clang/test/Analysis/aggrinit-cfg-output.cpp

Modified: 
clang/include/clang/Analysis/CFG.h
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
clang/lib/Analysis/CFG.cpp
clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
clang/test/Analysis/analyzer-config.c

Removed: 




diff  --git a/clang/include/clang/Analysis/CFG.h 
b/clang/include/clang/Analysis/CFG.h
index ea3d8b2921a7..93de3178e661 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -1248,6 +1248,7 @@ class CFG {
 bool AddStaticInitBranches = false;
 bool AddCXXNewAllocator = false;
 bool AddCXXDefaultInitExprInCtors = false;
+bool AddCXXDefaultInitExprInAggregates = false;
 bool AddRichCXXConstructors = false;
 bool MarkElidedCXXConstructors = false;
 bool AddVirtualBaseBranches = false;

diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index d853fb74f9c7..00febf688195 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -112,6 +112,12 @@ ANALYZER_OPTION(
 bool, ShouldIncludeScopesInCFG, "cfg-scopes",
 "Whether or not scope information should be included in the CFG.", false)
 
+ANALYZER_OPTION(bool, ShouldIncludeDefaultInitForAggregates,
+"cfg-expand-default-aggr-inits",
+"Whether or not inline CXXDefaultInitializers for aggregate "
+"initialization in the CFG.",
+false)
+
 ANALYZER_OPTION(
 bool, MayInlineTemplateFunctions, "c++-template-inlining",
 "Whether or not templated functions may be considered for inlining.", true)

diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index e10bfd805933..07945a80a311 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -542,6 +542,7 @@ class CFGBuilder {
 
 private:
   // Visitors to walk an AST and construct the CFG.
+  CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
   CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
   CFGBlock *VisitBinaryOperator(BinaryOperator *B, AddStmtChoice asc);
   CFGBlock *VisitBreakStmt(BreakStmt *B);
@@ -2140,6 +2141,9 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
 return Block;
   return VisitStmt(S, asc);
 
+case Stmt::InitListExprClass:
+  return VisitInitListExpr(cast(S), asc);
+
 case Stmt::AddrLabelExprClass:
   return VisitAddrLabelExpr(cast(S), asc);
 
@@ -2346,15 +2350,37 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
   // Visit the children in their reverse order so that they appear in
   // left-to-right (natural) order in the CFG.
   reverse_children RChildren(S);
-  for (reverse_children::iterator I = RChildren.begin(), E = RChildren.end();
-   I != E; ++I) {
-if (Stmt *Child = *I)
+  for (Stmt *Child : RChildren) {
+if (Child)
   if (CFGBlock *R = Visit(Child))
 B = R;
   }
   return B;
 }
 
+CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
+  if (asc.alwaysAdd(*this, ILE)) {
+autoCreateBlock();
+appendStmt(Block, ILE);
+  }
+  CFGBlock *B = Block;
+
+  reverse_children RChildren(ILE);
+  for (Stmt *Child : RChildren) {
+if (!Child)
+  continue;
+if (CFGBlock *R = Visit(Child))
+  B = R;
+if (BuildOpts.AddCXXDefaultInitExprInAggregates) {
+  if (auto *DIE = dyn_cast(Child))
+if (Stmt *Child = DIE->getExpr())
+  if (CFGBlock *R = Visit(Child))
+B = R;
+}
+  }
+  return B;
+}
+
 CFGBlock *CFGBuilder::VisitAddrLabelExpr(AddrLabelExpr *A,
  AddStmtChoice asc) {
   AddressTakenLabels.insert(A->getLabel());

diff  --git a/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp 
b/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
index 94fc09e64aa0..fdd03c75920d 100644
--- a/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
@@ -44,6 +44,8 @@ AnalysisManager::AnalysisManager(ASTContext &ASTCtx,
   optio

[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

LGTM!

Even though this is probably not the right solution for the static analyzer 
use-case (because we may end up having duplicate expressions in the CFG), it 
might actually make the static analyzer perform better than before (because 
it's still better than not having these expressions at all in the CFG). I guess 
we could experiment.




Comment at: clang/lib/Analysis/CFG.cpp:2370-2371
+  reverse_children RChildren(ILE);
+  for (reverse_children::iterator I = RChildren.begin(), E = RChildren.end();
+   I != E; ++I) {
+if (Stmt *Child = *I)

Can this be turned into a foreach loop?



Comment at: clang/test/Analysis/aggrinit-cfg-output.cpp:21
+// CHECK-NEXT:   4: 3
+// CHECK-NEXT:   5: 
+// CHECK-NEXT:   6: {[B1.1]}

We should really fix these :/


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

https://reviews.llvm.org/D71642



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


[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 234435.
xazax.hun added a comment.

- Remove accidentally added files.


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

https://reviews.llvm.org/D71642

Files:
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/test/Analysis/aggrinit-cfg-output.cpp
  clang/test/Analysis/analyzer-config.c

Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -19,6 +19,7 @@
 // CHECK-NEXT: c++-temp-dtor-inlining = true
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
+// CHECK-NEXT: cfg-expand-default-aggr-inits = false
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
@@ -98,4 +99,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 95
+// CHECK-NEXT: num-entries = 96
Index: clang/test/Analysis/aggrinit-cfg-output.cpp
===
--- /dev/null
+++ clang/test/Analysis/aggrinit-cfg-output.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-expand-default-aggr-inits=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+static char a[] = "foobar";
+
+struct StringRef {
+  const char *member = nullptr;
+  int len = 3;
+};
+
+int main() {
+  StringRef s{a};
+  (void)s;
+}
+
+// CHECK: [B1]
+// CHECK-NEXT:   1: a
+// CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, ArrayToPointerDecay, char *)
+// CHECK-NEXT:   3: [B1.2] (ImplicitCastExpr, NoOp, const char *)
+// CHECK-NEXT:   4: 3
+// CHECK-NEXT:   5: 
+// CHECK-NEXT:   6: {[B1.1]}
+// CHECK-NEXT:   7: StringRef s{a};
+// CHECK-NEXT:   8: s
+// CHECK-NEXT:   9: (void)[B1.8] (CStyleCastExpr, ToVoid, void)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+
Index: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
@@ -44,6 +44,8 @@
   options(Options) {
   AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
   AnaCtxMgr.getCFGBuildOptions().OmitImplicitValueInitializers = true;
+  AnaCtxMgr.getCFGBuildOptions().AddCXXDefaultInitExprInAggregates =
+  Options.ShouldIncludeDefaultInitForAggregates;
 }
 
 AnalysisManager::~AnalysisManager() {
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -542,6 +542,7 @@
 
 private:
   // Visitors to walk an AST and construct the CFG.
+  CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
   CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
   CFGBlock *VisitBinaryOperator(BinaryOperator *B, AddStmtChoice asc);
   CFGBlock *VisitBreakStmt(BreakStmt *B);
@@ -2140,6 +2141,9 @@
 return Block;
   return VisitStmt(S, asc);
 
+case Stmt::InitListExprClass:
+  return VisitInitListExpr(cast(S), asc);
+
 case Stmt::AddrLabelExprClass:
   return VisitAddrLabelExpr(cast(S), asc);
 
@@ -2355,6 +2359,29 @@
   return B;
 }
 
+CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
+  if (asc.alwaysAdd(*this, ILE)) {
+autoCreateBlock();
+appendStmt(Block, ILE);
+  }
+  CFGBlock *B = Block;
+
+  reverse_children RChildren(ILE);
+  for (reverse_children::iterator I = RChildren.begin(), E = RChildren.end();
+   I != E; ++I) {
+if (Stmt *Child = *I)
+  if (CFGBlock *R = Visit(Child))
+B = R;
+if (BuildOpts.AddCXXDefaultInitExprInAggregates) {
+  if (auto *DIE = dyn_cast_or_null(*I))
+if (Stmt *Child = DIE->getExpr())
+  if (CFGBlock *R = Visit(Child))
+B = R;
+}
+  }
+  return B;
+}
+
 CFGBlock *CFGBuilder::VisitAddrLabelExpr(AddrLabelExpr *A,
  AddStmtChoice asc) {
   AddressTakenLabels.insert(A->getLabel());
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -112,6 +112,12 @@
 bool, ShouldIncludeScopesInCFG, "cfg-scopes",
 "Whether or not scope information should be included in the CFG.", false)
 
+ANALYZER_OPTION(bool, ShouldIncludeDefaultInitForAggregates,
+"cfg-expand-default-aggr-inits",
+"Whether or not inline CXXDefaultInitializers for aggregate "
+"initializatio

[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 234434.
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.

- Fix review comments.


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

https://reviews.llvm.org/D71642

Files:
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/Analysis/CFG.cpp
  clang/lib/Analysis/LifetimePsetBuilder.cpp
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/test/Analysis/aggrinit-cfg-output.cpp
  clang/test/Analysis/analyzer-config.c

Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -19,6 +19,7 @@
 // CHECK-NEXT: c++-temp-dtor-inlining = true
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
+// CHECK-NEXT: cfg-expand-default-aggr-inits = false
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
@@ -98,4 +99,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 95
+// CHECK-NEXT: num-entries = 96
Index: clang/test/Analysis/aggrinit-cfg-output.cpp
===
--- /dev/null
+++ clang/test/Analysis/aggrinit-cfg-output.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-expand-default-aggr-inits=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+static char a[] = "foobar";
+
+struct StringRef {
+  const char *member = nullptr;
+  int len = 3;
+};
+
+int main() {
+  StringRef s{a};
+  (void)s;
+}
+
+// CHECK: [B1]
+// CHECK-NEXT:   1: a
+// CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, ArrayToPointerDecay, char *)
+// CHECK-NEXT:   3: [B1.2] (ImplicitCastExpr, NoOp, const char *)
+// CHECK-NEXT:   4: 3
+// CHECK-NEXT:   5: 
+// CHECK-NEXT:   6: {[B1.1]}
+// CHECK-NEXT:   7: StringRef s{a};
+// CHECK-NEXT:   8: s
+// CHECK-NEXT:   9: (void)[B1.8] (CStyleCastExpr, ToVoid, void)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+
Index: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
@@ -44,6 +44,8 @@
   options(Options) {
   AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
   AnaCtxMgr.getCFGBuildOptions().OmitImplicitValueInitializers = true;
+  AnaCtxMgr.getCFGBuildOptions().AddCXXDefaultInitExprInAggregates =
+  Options.ShouldIncludeDefaultInitForAggregates;
 }
 
 AnalysisManager::~AnalysisManager() {
Index: clang/lib/Analysis/LifetimePsetBuilder.cpp
===
--- /dev/null
+++ clang/lib/Analysis/LifetimePsetBuilder.cpp
@@ -0,0 +1,1415 @@
+//=- LifetimePsetBuilder.cpp - Diagnose lifetime violations -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Analysis/Analyses/LifetimePsetBuilder.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/Analysis/Analyses/Lifetime.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Lex/Lexer.h"
+
+namespace clang {
+namespace lifetime {
+
+namespace {
+
+#define VERBOSE_DEBUG 0
+#if VERBOSE_DEBUG
+#define DBG(x) llvm::errs() << x
+#else
+#define DBG(x)
+#endif
+
+static bool hasPSet(const Expr *E) {
+  auto TC = classifyTypeCategory(E->getType());
+  return E->isLValue() || TC == TypeCategory::Pointer ||
+ TC == TypeCategory::Owner;
+}
+
+static bool isPointer(const Expr *E) {
+  auto TC = classifyTypeCategory(E->getType());
+  return TC == TypeCategory::Pointer;
+}
+
+/// Collection of methods to update/check PSets from statements/expressions
+/// Conceptually, for each Expr where Expr::isLValue() is true,
+/// we put an entry into the RefersTo map, which contains the set
+/// of Variables that an lvalue might refer to, e.g.
+/// RefersTo(var) = {var}
+/// RefersTo(*p) = pset(p)
+/// RefersTo(a = b) = {a}
+/// RefersTo(a, b) = {b}
+///
+/// For every expression whose Type is a Pointer or an Owner,
+/// we also track the pset (points-to set), e.g.
+///  pset(&v) = {v}
+class PSetsBuilder : public ConstStmtVisitor {
+
+  const FunctionDecl *AnalyzedFD;
+  LifetimeReporterBase &Reporter;
+  ASTContext &ASTCtxt;
+  /// Returns true if the first argument is implicitly convertible
+  /// into the second argument.
+  IsConvertibleTy IsConvertible;
+
+  /// psets of all memory locations, which are identified
+  /// by their non

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

2019-12-17 Thread Philip Reames via Phabricator via cfe-commits
reames added a comment.

I've updated the draft assembler support in https://reviews.llvm.org/D71315 to 
match the proposal here.  Again, this is very much WIP and mostly just to show 
proposed syntax/usage.


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

https://reviews.llvm.org/D70157



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


[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__wasm_argc_argv`

2019-12-17 Thread Dan Gohman via Phabricator via cfe-commits
sunfish updated this revision to Diff 234426.
sunfish added reviewers: sbc100, dschuff, aheejin.
sunfish added a comment.

This updates the `main` vs `__main_argc_argv` patch so that it doesn't apply to 
Emscripten targets, following the discussion in 
https://github.com/WebAssembly/tool-conventions/pull/134.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70700

Files:
  clang/lib/AST/Mangle.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/CodeGen/wasm-call-main.c
  clang/test/CodeGen/wasm-main.c
  clang/test/CodeGen/wasm-main_argc_argv.c
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -730,6 +730,11 @@
 return getArch() == Triple::riscv32 || getArch() == Triple::riscv64;
   }
 
+  /// Tests whether the target is wasm (32- and 64-bit).
+  bool isWasm() const {
+return getArch() == Triple::wasm32 || getArch() == Triple::wasm64;
+  }
+
   /// Tests whether the target supports comdat
   bool supportsCOMDAT() const {
 return !isOSBinFormatMachO();
Index: clang/test/CodeGen/wasm-main_argc_argv.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-main_argc_argv.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Mangle the argc/argv form of main.
+
+int main(int argc, char **argv) {
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @__main_argc_argv(i32 %argc, i8** %argv)
Index: clang/test/CodeGen/wasm-main.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-main.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Don't mangle the no-arg form of main.
+
+int main(void) {
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @main()
Index: clang/test/CodeGen/wasm-call-main.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-call-main.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Mangle argc/argv main even when it's not defined in this TU.
+
+#include 
+
+int main(int argc, char *argv[]);
+
+int foo(void) {
+return main(0, NULL);
+}
+
+// CHECK: call i32 @__main_argc_argv(
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -435,8 +435,7 @@
 break;
 
   case llvm::Triple::UnknownOS:
-if (triple.getArch() == llvm::Triple::wasm32 ||
-triple.getArch() == llvm::Triple::wasm64)
+if (triple.isWasm())
   return;
 break;
   }
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -50,7 +50,8 @@
   CCM_Fast,
   CCM_RegCall,
   CCM_Vector,
-  CCM_Std
+  CCM_Std,
+  CCM_WasmMainArgcArgv
 };
 
 static bool isExternC(const NamedDecl *ND) {
@@ -63,6 +64,16 @@
  const NamedDecl *ND) {
   const TargetInfo &TI = Context.getTargetInfo();
   const llvm::Triple &Triple = TI.getTriple();
+
+  // On wasm, the argc/argv form of "main" is renamed so that the startup code
+  // can call it with the correct function signature.
+  // On Emscripten, users may be exporting "main" and expecting to call it
+  // themselves, so we can't mangle it.
+  if (Triple.isWasm() && !Triple.isOSEmscripten())
+if (const FunctionDecl *FD = dyn_cast(ND))
+  if (FD->isMain() && FD->hasPrototype() && FD->param_size() == 2)
+return CCM_WasmMainArgcArgv;
+
   if (!Triple.isOSWindows() ||
   !(Triple.getArch() == llvm::Triple::x86 ||
 Triple.getArch() == llvm::Triple::x86_64))
@@ -145,6 +156,12 @@
 
   const ASTContext &ASTContext = getASTContext();
   CCMangling CC = getCallingConvMangling(ASTContext, D);
+
+  if (CC == CCM_WasmMainArgcArgv) {
+Out << "__main_argc_argv";
+return;
+  }
+
   bool MCXX = shouldMangleCXXName(D);
   const TargetInfo &TI = Context.getTargetInfo();
   if (CC == CCM_Other || (MCXX && TI.getCXXABI() == TargetCXXABI::Microsoft)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 234427.
xazax.hun added a comment.

- Add missing tests.


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

https://reviews.llvm.org/D71642

Files:
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/test/Analysis/aggrinit-cfg-output.cpp
  clang/test/Analysis/analyzer-config.c

Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -18,6 +18,7 @@
 // CHECK-NEXT: c++-stdlib-inlining = true
 // CHECK-NEXT: c++-temp-dtor-inlining = true
 // CHECK-NEXT: c++-template-inlining = true
+// CHECK-NEXT: cfg-aggr-definit = false
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
@@ -98,4 +99,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 95
+// CHECK-NEXT: num-entries = 96
Index: clang/test/Analysis/aggrinit-cfg-output.cpp
===
--- /dev/null
+++ clang/test/Analysis/aggrinit-cfg-output.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-aggr-definit=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+static char a[] = "foobar";
+
+struct StringRef {
+  const char *member = nullptr;
+  int len = 3;
+};
+
+int main() {
+  StringRef s{a};
+  (void)s;
+}
+
+// CHECK: [B1]
+// CHECK-NEXT:   1: a
+// CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, ArrayToPointerDecay, char *)
+// CHECK-NEXT:   3: [B1.2] (ImplicitCastExpr, NoOp, const char *)
+// CHECK-NEXT:   4: 3
+// CHECK-NEXT:   5: 
+// CHECK-NEXT:   6: {[B1.1]}
+// CHECK-NEXT:   7: StringRef s{a};
+// CHECK-NEXT:   8: s
+// CHECK-NEXT:   9: (void)[B1.8] (CStyleCastExpr, ToVoid, void)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+
Index: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
@@ -44,6 +44,8 @@
   options(Options) {
   AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
   AnaCtxMgr.getCFGBuildOptions().OmitImplicitValueInitializers = true;
+  AnaCtxMgr.getCFGBuildOptions().AddCXXDefaultInitExprInAggregates =
+  Options.ShouldIncludeDefaultInitForAggregates;
 }
 
 AnalysisManager::~AnalysisManager() {
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -542,6 +542,7 @@
 
 private:
   // Visitors to walk an AST and construct the CFG.
+  CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
   CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
   CFGBlock *VisitBinaryOperator(BinaryOperator *B, AddStmtChoice asc);
   CFGBlock *VisitBreakStmt(BreakStmt *B);
@@ -2140,6 +2141,9 @@
 return Block;
   return VisitStmt(S, asc);
 
+case Stmt::InitListExprClass:
+  return VisitInitListExpr(cast(S), asc);
+
 case Stmt::AddrLabelExprClass:
   return VisitAddrLabelExpr(cast(S), asc);
 
@@ -2355,6 +2359,29 @@
   return B;
 }
 
+CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
+  if (asc.alwaysAdd(*this, ILE)) {
+autoCreateBlock();
+appendStmt(Block, ILE);
+  }
+  CFGBlock *B = Block;
+
+  reverse_children RChildren(ILE);
+  for (reverse_children::iterator I = RChildren.begin(), E = RChildren.end();
+   I != E; ++I) {
+if (Stmt *Child = *I)
+  if (CFGBlock *R = Visit(Child))
+B = R;
+if (BuildOpts.AddCXXDefaultInitExprInAggregates) {
+  if (auto *DIE = dyn_cast_or_null(*I))
+if (Stmt *Child = DIE->getExpr())
+  if (CFGBlock *R = Visit(Child))
+B = R;
+}
+  }
+  return B;
+}
+
 CFGBlock *CFGBuilder::VisitAddrLabelExpr(AddrLabelExpr *A,
  AddStmtChoice asc) {
   AddressTakenLabels.insert(A->getLabel());
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -112,6 +112,11 @@
 bool, ShouldIncludeScopesInCFG, "cfg-scopes",
 "Whether or not scope information should be included in the CFG.", false)
 
+ANALYZER_OPTION(bool, ShouldIncludeDefaultInitForAggregates, "cfg-aggr-definit",
+"Whether or not inline CXXDefaultInitializers for aggregat "
+"initialization in the CFG.",
+false)
+
 ANALYZER_OPTION(
 

[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:115
 
+ANALYZER_OPTION(bool, ShouldIncludeDefaultInitForAggregates, 
"cfg-aggr-definit",
+"Whether or not inline CXXDefaultInitializers for aggregat "

Maybe "cfg-expand-default-aggr-inits"?



Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:116
+ANALYZER_OPTION(bool, ShouldIncludeDefaultInitForAggregates, 
"cfg-aggr-definit",
+"Whether or not inline CXXDefaultInitializers for aggregat "
+"initialization in the CFG.",

Typo: "aggregate".


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

https://reviews.llvm.org/D71642



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


[libunwind] 547659a - [unwind] Don't link libpthread and libdl on Fuchsia

2019-12-17 Thread Petr Hosek via cfe-commits

Author: Petr Hosek
Date: 2019-12-17T17:21:43-08:00
New Revision: 547659ae56f5827055f71b495d7b08c10badadb5

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

LOG: [unwind] Don't link libpthread and libdl on Fuchsia

This is a follow up to D71135.

Added: 


Modified: 
libunwind/cmake/config-ix.cmake

Removed: 




diff  --git a/libunwind/cmake/config-ix.cmake b/libunwind/cmake/config-ix.cmake
index 0d833e996ca1..9c8089cfe215 100644
--- a/libunwind/cmake/config-ix.cmake
+++ b/libunwind/cmake/config-ix.cmake
@@ -71,10 +71,6 @@ endif()
 # Check compiler flags
 check_cxx_compiler_flag(-nostdinc++ LIBUNWIND_HAS_NOSTDINCXX_FLAG)
 
-# Check libraries
-check_library_exists(dl dladdr "" LIBUNWIND_HAS_DL_LIB)
-check_library_exists(pthread pthread_once "" LIBUNWIND_HAS_PTHREAD_LIB)
-
 # Check symbols
 check_symbol_exists(__arm__ "" LIBUNWIND_TARGET_ARM)
 check_symbol_exists(__USING_SJLJ_EXCEPTIONS__ "" 
LIBUNWIND_USES_SJLJ_EXCEPTIONS)
@@ -84,3 +80,12 @@ if(LIBUNWIND_TARGET_ARM AND NOT 
LIBUNWIND_USES_SJLJ_EXCEPTIONS AND NOT LIBUNWIND
   # This condition is copied from __libunwind_config.h
   set(LIBUNWIND_USES_ARM_EHABI ON)
 endif()
+
+# Check libraries
+if(FUCHSIA)
+  set(LIBUNWIND_HAS_DL_LIB NO)
+  set(LIBUNWIND_HAS_PTHREAD_LIB NO)
+else()
+  check_library_exists(dl dladdr "" LIBUNWIND_HAS_DL_LIB)
+  check_library_exists(pthread pthread_once "" LIBUNWIND_HAS_PTHREAD_LIB)
+endif()



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


[PATCH] D71642: [CFG] Add on option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision.
xazax.hun added reviewers: NoQ, Szelethus, mgehre.
xazax.hun added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, rnkovacs.

This is useful for clients that are relying on linearized CFGs for evaluating 
subexpressions and want the default initializer to be evaluated properly.

The upcoming lifetime analysis is using this but it might also be useful for 
the static analyzer at some point.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71642

Files:
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/test/Analysis/analyzer-config.c

Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -18,6 +18,7 @@
 // CHECK-NEXT: c++-stdlib-inlining = true
 // CHECK-NEXT: c++-temp-dtor-inlining = true
 // CHECK-NEXT: c++-template-inlining = true
+// CHECK-NEXT: cfg-aggr-definit = false
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
@@ -98,4 +99,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 95
+// CHECK-NEXT: num-entries = 96
Index: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
@@ -44,6 +44,8 @@
   options(Options) {
   AnaCtxMgr.getCFGBuildOptions().setAllAlwaysAdd();
   AnaCtxMgr.getCFGBuildOptions().OmitImplicitValueInitializers = true;
+  AnaCtxMgr.getCFGBuildOptions().AddCXXDefaultInitExprInAggregates =
+  Options.ShouldIncludeDefaultInitForAggregates;
 }
 
 AnalysisManager::~AnalysisManager() {
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -542,6 +542,7 @@
 
 private:
   // Visitors to walk an AST and construct the CFG.
+  CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
   CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
   CFGBlock *VisitBinaryOperator(BinaryOperator *B, AddStmtChoice asc);
   CFGBlock *VisitBreakStmt(BreakStmt *B);
@@ -2140,6 +2141,9 @@
 return Block;
   return VisitStmt(S, asc);
 
+case Stmt::InitListExprClass:
+  return VisitInitListExpr(cast(S), asc);
+
 case Stmt::AddrLabelExprClass:
   return VisitAddrLabelExpr(cast(S), asc);
 
@@ -2355,6 +2359,29 @@
   return B;
 }
 
+CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
+  if (asc.alwaysAdd(*this, ILE)) {
+autoCreateBlock();
+appendStmt(Block, ILE);
+  }
+  CFGBlock *B = Block;
+
+  reverse_children RChildren(ILE);
+  for (reverse_children::iterator I = RChildren.begin(), E = RChildren.end();
+   I != E; ++I) {
+if (Stmt *Child = *I)
+  if (CFGBlock *R = Visit(Child))
+B = R;
+if (BuildOpts.AddCXXDefaultInitExprInAggregates) {
+  if (auto *DIE = dyn_cast_or_null(*I))
+if (Stmt *Child = DIE->getExpr())
+  if (CFGBlock *R = Visit(Child))
+B = R;
+}
+  }
+  return B;
+}
+
 CFGBlock *CFGBuilder::VisitAddrLabelExpr(AddrLabelExpr *A,
  AddStmtChoice asc) {
   AddressTakenLabels.insert(A->getLabel());
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -112,6 +112,11 @@
 bool, ShouldIncludeScopesInCFG, "cfg-scopes",
 "Whether or not scope information should be included in the CFG.", false)
 
+ANALYZER_OPTION(bool, ShouldIncludeDefaultInitForAggregates, "cfg-aggr-definit",
+"Whether or not inline CXXDefaultInitializers for aggregat "
+"initialization in the CFG.",
+false)
+
 ANALYZER_OPTION(
 bool, MayInlineTemplateFunctions, "c++-template-inlining",
 "Whether or not templated functions may be considered for inlining.", true)
Index: clang/include/clang/Analysis/CFG.h
===
--- clang/include/clang/Analysis/CFG.h
+++ clang/include/clang/Analysis/CFG.h
@@ -1248,6 +1248,7 @@
 bool AddStaticInitBranches = false;
 bool AddCXXNewAllocator = false;
 bool AddCXXDefaultInitExprInCtors = false;
+bool AddCXXDefaultInitExprInAggregates = false;
 bool AddRichCXXConstructors = false;
 bool MarkElidedCXXConstructors = false;
 bool AddVirtualBaseBranches = false;
__

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

2019-12-17 Thread Andy Kaylor via Phabricator via cfe-commits
andrew.w.kaylor added a comment.

In D62731#1788838 , @rupprecht wrote:

> It seems the discussion of whether or not this is incomplete died out -- I'd 
> prefer to assume it is incomplete if there is no consensus. Mailed D71635 
>  to rename `-frounding-math` to 
> `-fexperimental-rounding-math`.
>
> Alternatively we could remove the warning. I still don't see a good argument 
> for the middle ground of having it called `-frounding-math` but also generate 
> a warning.


It's definitely incomplete but the results will not be any worse than you get 
when -frounding-math is ignored.

My preference would be to change the text of the warning that is issued but 
allow -frounding-math to be enabled by this commit without requiring an 
additional option.

I would also very much like to see this patch re-committed. It's currently in 
the "approved" state. If anyone objects to this being committed, please use the 
"request changes" action to indicate this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D70258#1788861 , @rjmccall wrote:

> That's what I figured.


Just to say this again:
Current OpenMP code generation keeps a second stack for exactly the same 
purpose as the one introduced here.

> The thing is that that necessarily interacts correctly with everything in 
> Clang's IRGen in ways that making a second stack that Clang's IRGen doesn't 
> directly know about doesn't.
> 
> I think you either need to extract Clang's entire cleanup-stack concept into 
> a generic frontend library, so that both Clang and your generic OpenMP 
> lowering just manipulate that common stack, or you need to call back into the 
> frontend to manage your cleanup.

I like the idea of a generic way to do this but I'm unsure if that is needed 
given the restrictions on OpenMP regions. We basically are not allowed to jump 
out by non-OpenMP means. What we do in the existing, and this rewritten stack 
is remember what OpenMP means will jump out of the current scope and to the end 
of the region. This can be reused in Flang as the restrictions hold there as 
well (as far as I know).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D70258#1788825 , @rjmccall wrote:

> So it's never that OpenMP has things that need to be finalized before exiting 
> (e.g. if something in frontend-emitted code throws an exception), it's just 
> that OpenMP might need to generate its own control flow that breaks through 
> arbitrary scopes in the frontend?


Depending on how you look at it. OpenMP has to finalize some constructs, which 
usually means to place a call before we continue with the code that follows. It 
also introduce a new scope in which things like privatization happen, however 
these are almost completely handled through clang (see below). The one thing 
that comes to mind is `lastprivate` which is handled by OpenMP.

Btw. Exceptions have to be caught inside an OpenMP scope that has a finalizer 
as it would be UB otherwise (as with the `return`).

> I would really hope that the OpenMP implementation in Clang would've used 
> Clang's cleanup stack rather than inventing its own mechanism.

This patch uses (parts of) clangs cleanup logic and the existing CGOpenMP 
cleanup code (which is the existing OpenMP specific stack 
(`CodeGenFunction::OMPCancelStack`) this one will replace).

---

In the code that glues the old CGOpenMP to the new OMPBuilder, where we create 
the finalization callback, the existing `getOMPCancelDestination` is used as 
shown below.

  CodeGenFunction::JumpDest Dest = CGF.getOMPCancelDestination(OMPD_parallel);
  CGF.EmitBranchThroughCleanup(Dest);

In the follow up patch (D70290 ) that will 
lower `#pragma omp parallel` through the OMPIRBuilder, we don't need the 
`clang::CodeGenFunction::OMPCancelDestination`/`clang::CodeGenFunction::OMPCancelStack`
 stuff anymore (for the parallel) and the finalization callback becomes:

  CodeGenFunction::JumpDest Dest = getJumpDestInCurrentScope(DestBB);
  EmitBranchThroughCleanup(Dest);

With `DestBB` defined as the end of the OpenMP region/scope.

---

The additional stack is also needed because depending on the nesting we may or 
may not create control flow that can jump to the end of a scope. So

  #pragma omp parallel
  {
{
  ... ;
  #pragma omp barrier // or an implicit barrier
  ... ;
}
parallel_exit:
  }

will cause a control flow edge from the barrier to `parallel_exit`. This edge 
may not exist if there is another OpenMP region nested in the parallel.

---

In D70258#1788854 , @ABataev wrote:

> In D70258#1788825 , @rjmccall wrote:
>
> > I would really hope that the OpenMP implementation in Clang would've used 
> > Clang's cleanup stack rather than inventing its own mechanism.
>
>
> John, current implementation completely relies on Clang's cleanup stack.


Let me rephrase the above statement less misleading:
The current implementation relies on Clang's cleanup stack *and* the OpenMP 
specific `clang::CodeGenFunction::OMPCancelStack`, which is what this patch 
will eventually replace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

That's what I figured.  The thing is that that necessarily interacts correctly 
with everything in Clang's IRGen in ways that making a second stack that 
Clang's IRGen doesn't directly know about doesn't.

I think you either need to extract Clang's entire cleanup-stack concept into a 
generic frontend library, so that both Clang and your generic OpenMP lowering 
just manipulate that common stack, or you need to call back into the frontend 
to manage your cleanup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


[PATCH] D71635: [clang] Rename -frounding-math to -fexperimental-rounding-math and add -frounding-math back as a gcc-compat arg.

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60990 tests passed, 1 failed 
and 727 were skipped.

  failed: lit.lit/shtest-format.py

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71635



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


[PATCH] D71314: Emit a warning if a variable is uninitialized in indirect ASM goto destination.

2019-12-17 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment.

In D71314#1788797 , @nickdesaulniers 
wrote:

> strange, the below test isn't warning for me with this patch applied:
>
>   $ clang -O2 foo.c -c
>   $ cat foo.c
>   int quux(void) {
> int y;
> asm volatile goto("ja %l1" : "=r"(y) ::: err);
> return y;
>   err:
> return y;
>   }
>


Use `-Wuninitialized`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71314



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


[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.



In D70258#1788825 , @rjmccall wrote:

> So it's never that OpenMP has things that need to be finalized before exiting 
> (e.g. if something in frontend-emitted code throws an exception), it's just 
> that OpenMP might need to generate its own control flow that breaks through 
> arbitrary scopes in the frontend?
>
> I would really hope that the OpenMP implementation in Clang would've used 
> Clang's cleanup stack rather than inventing its own mechanism.


John, current implementation completely relies on Clang's cleanup stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


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

2019-12-17 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment.

It seems the discussion of whether or not this is incomplete died out -- I'd 
prefer to assume it is incomplete if there is no consensus. Mailed D71635 
 to rename `-frounding-math` to 
`-fexperimental-rounding-math`.

Alternatively we could remove the warning. I still don't see a good argument 
for the middle ground of having it called `-frounding-math` but also generate a 
warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

So it's never that OpenMP has things that need to be finalized before exiting 
(e.g. if something in frontend-emitted code throws an exception), it's just 
that OpenMP might need to generate its own control flow that breaks through 
arbitrary scopes in the frontend?

I would really hope that the OpenMP implementation in Clang would've used 
Clang's cleanup stack rather than inventing its own mechanism.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


[PATCH] D71635: [clang] Rename -frounding-math to -fexperimental-rounding-math and add -frounding-math back as a gcc-compat arg.

2019-12-17 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht created this revision.
rupprecht added reviewers: mibintc, chandlerc, echristo, rjmccall, kpn, 
erichkeane, rsmith, andrew.w.kaylor.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D62731  implements an incomplete/experimental 
version of rounding math, but since the flag doesn't have "experimental" in the 
name, the feedback that it is experimental is in the form of 
-Wexperimental-float-control.

Since this flag is already being accepted as a gcc compatability arg, put the 
current implementation being `-f[no-]experimental-rounding-math` instead, and 
allow `-frounding-math` as an ignored arg. Because "experimental" is now in the 
flag name, remove the warning -- it's now clear that the feature is 
incomplete/experimental when invoking it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71635

Files:
  clang/docs/UsersManual.rst
  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/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fp-model.c

Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -43,9 +43,9 @@
 // RUN:   | FileCheck --check-prefix=WARN9 %s
 // WARN9: warning: overriding '-ffp-model=strict' option with '-fno-honor-nans' [-Woverriding-t-option]
 
-// RUN: %clang -### -ffp-model=strict -fno-rounding-math -c %s 2>&1 \
+// RUN: %clang -### -ffp-model=strict -fno-experimental-rounding-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARNa %s
-// WARNa: warning: overriding '-ffp-model=strict' option with '-fno-rounding-math' [-Woverriding-t-option]
+// WARNa: warning: overriding '-ffp-model=strict' option with '-fno-experimental-rounding-math' [-Woverriding-t-option]
 
 // RUN: %clang -### -ffp-model=strict -fno-signed-zeros -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARNb %s
@@ -70,12 +70,12 @@
 // RUN: %clang -### -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NOROUND %s
 // CHECK-NOROUND: "-cc1"
-// CHECK-NOROUND: "-fno-rounding-math"
+// CHECK-NOROUND: "-fno-experimental-rounding-math"
 
-// RUN: %clang -### -frounding-math -c %s 2>&1 \
+// RUN: %clang -### -fexperimental-rounding-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ROUND --implicit-check-not ffp-exception-behavior=strict %s
 // CHECK-ROUND: "-cc1"
-// CHECK-ROUND: "-frounding-math"
+// CHECK-ROUND: "-fexperimental-rounding-math"
 
 // RUN: %clang -### -ftrapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-TRAP %s
@@ -93,7 +93,7 @@
 // CHECK-FPM-FAST: "-mreassociate"
 // CHECK-FPM-FAST: "-freciprocal-math"
 // CHECK-FPM-FAST: "-ffp-contract=fast"
-// CHECK-FPM-FAST: "-fno-rounding-math"
+// CHECK-FPM-FAST: "-fno-experimental-rounding-math"
 // CHECK-FPM-FAST: "-ffast-math"
 // CHECK-FPM-FAST: "-ffinite-math-only"
 
@@ -101,37 +101,37 @@
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-PRECISE %s
 // CHECK-FPM-PRECISE: "-cc1"
 // CHECK-FPM-PRECISE: "-ffp-contract=fast"
-// CHECK-FPM-PRECISE: "-fno-rounding-math"
+// CHECK-FPM-PRECISE: "-fno-experimental-rounding-math"
 
 // RUN: %clang -### -nostdinc -ffp-model=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-STRICT %s
 // CHECK-FPM-STRICT: "-cc1"
 // CHECK-FPM-STRICT: "-ftrapping-math"
-// CHECK-FPM-STRICT: "-frounding-math"
+// CHECK-FPM-STRICT: "-fexperimental-rounding-math"
 // CHECK-FPM-STRICT: "-ffp-exception-behavior=strict"
 
 // RUN: %clang -### -nostdinc -ftrapping-math -ffp-exception-behavior=ignore -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-TRAP-IGNORE %s
 // CHECK-TRAP-IGNORE: "-cc1"
-// CHECK-TRAP-IGNORE: "-fno-rounding-math"
+// CHECK-TRAP-IGNORE: "-fno-experimental-rounding-math"
 // CHECK-TRAP-IGNORE: "-ffp-exception-behavior=ignore"
 
 
 // RUN: %clang -### -nostdinc -ffp-exception-behavior=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FEB-STRICT %s
 // CHECK-FEB-STRICT: "-cc1"
-// CHECK-FEB-STRICT: "-fno-rounding-math"
+// CHECK-FEB-STRICT: "-fno-experimental-rounding-math"
 // CHECK-FEB-STRICT: "-ffp-exception-behavior=strict"
 
 // RUN: %clang -### -nostdinc -ffp-exception-behavior=maytrap -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FEB-MAYTRAP %s
 // CHECK-FEB-MAYTRAP: "-cc1"
-// CHECK-FEB-MAYTRAP: "-fno-rounding-math"
+// CHECK-FEB-MAYTRAP: "-fno-experimental-rounding-math"
 // CHECK-FEB-MAYTRAP: "-ffp-exception-behavior=maytrap"
 
 // RUN: %clang -### -nostdinc -ffp-exception-behavior=ignore -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FEB-IGNORE %s
 // CHECK-FEB-IGNORE: "-cc1"
-// CHECK-FEB-IGNORE: "-fno-rounding-math"
+// CHECK-FEB-IGNORE: "-fno-experimental-rounding-math"
 // CHECK-FEB-IGNORE: "-ffp-exception-behavior=

[PATCH] D71625: [CMake] Added remote test execution support into CrossWinToARMLinux CMake cache file.

2019-12-17 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka marked 2 inline comments as done.
vvereschaka added a comment.

I have updated the diff without using those options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71625



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


[PATCH] D71314: Emit a warning if a variable is uninitialized in indirect ASM goto destination.

2019-12-17 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

strange, the below test isn't warning for me with this patch applied:

  $ clang -O2 foo.c -c
  $ cat foo.c
  int quux(void) {
int y;
asm volatile goto("ja %l1" : "=r"(y) ::: err);
return y;
  err:
return y;
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71314



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


[PATCH] D71625: [CMake] Added remote test execution support into CrossWinToARMLinux CMake cache file.

2019-12-17 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka updated this revision to Diff 234407.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71625

Files:
  clang/cmake/caches/CrossWinToARMLinux.cmake


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,6 +14,8 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
+#   -DREMOTE_TEST_EXECUTE_HOST="" ^
+#   -DREMOTE_TEST_EXECUTE_USER="" ^
 #   
-C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
@@ -82,7 +84,33 @@
 set(BUILTINS_CMAKE_ARGS 
"-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 
-set(LLVM_INSTALL_TOOLCHAIN_ONLYON CACHE BOOL "")
+# Remote test configuration.
+if(DEFINED REMOTE_TEST_EXECUTE_HOST)
+  set(DEFAULT_TEST_EXECUTOR 
"SSHExecutor('${REMOTE_TEST_EXECUTE_HOST}', '${REMOTE_TEST_EXECUTE_USER}')")
+  set(DEFAULT_TEST_TARGET_INFO  
"libcxx.test.target_info.LinuxLocalTI")
+
+  # Allow override with the custom values.
+  if(NOT DEFINED LIBUNWIND_TARGET_INFO)
+set(LIBUNWIND_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBUNWIND_EXECUTOR)
+set(LIBUNWIND_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_TARGET_INFO)
+set(LIBCXXABI_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_EXECUTOR)
+set(LIBCXXABI_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_TARGET_INFO)
+set(LIBCXX_TARGET_INFO  "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_EXECUTOR)
+set(LIBCXX_EXECUTOR "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+endif()
+
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
   llvm-ar
   llvm-cov


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,6 +14,8 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
+#   -DREMOTE_TEST_EXECUTE_HOST="" ^
+#   -DREMOTE_TEST_EXECUTE_USER="" ^
 #   -C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
@@ -82,7 +84,33 @@
 set(BUILTINS_CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 set(RUNTIMES_CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 
-set(LLVM_INSTALL_TOOLCHAIN_ONLY 			ON CACHE BOOL "")
+# Remote test configuration.
+if(DEFINED REMOTE_TEST_EXECUTE_HOST)
+  set(DEFAULT_TEST_EXECUTOR "SSHExecutor('${REMOTE_TEST_EXECUTE_HOST}', '${REMOTE_TEST_EXECUTE_USER}')")
+  set(DEFAULT_TEST_TARGET_INFO  "libcxx.test.target_info.LinuxLocalTI")
+
+  # Allow override with the custom values.
+  if(NOT DEFINED LIBUNWIND_TARGET_INFO)
+set(LIBUNWIND_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBUNWIND_EXECUTOR)
+set(LIBUNWIND_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_TARGET_INFO)
+set(LIBCXXABI_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_EXECUTOR)
+set(LIBCXXABI_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_TARGET_INFO)
+set(LIBCXX_TARGET_INFO  "${DEFAULT_TEST_TARGET_INFO}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_EXECUTOR)
+set(LIBCXX_EXECUTOR "${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+  endif()
+endif()
+
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
   llvm-ar
   llvm-cov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

2019-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

Looks like @rsmith did this here:
https://github.com/llvm/llvm-project/commit/fbe2369f1a514423e4c25417ab3532502fde6f2a

I see that it was replacing a CHECK for a specific comdat group with no comdat 
at all. I *think* that's not correct, we should be checking for the trivial 
comdat group, the one for the wrapper function, not the comdat group of the TLS 
variable. Let's get input from Richard, though.


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

https://reviews.llvm.org/D71572



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


[PATCH] D70701: Fix more VFS tests on Windows

2019-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a subscriber: JDevlieghere.
rnk added a comment.
This revision is now accepted and ready to land.

+@JDevlieghere, due to recent VFS test fixup.

I think this looks good, and in the absence of input from other VFS 
stakeholders, let's land this soon. Thanks!




Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078
+std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl 
&Path) const {
+  if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||
+  llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::windows))
+return {};

rnk wrote:
> amccarth wrote:
> > amccarth wrote:
> > > rnk wrote:
> > > > I think Posix-style paths are considered absolute, even on Windows. The 
> > > > opposite isn't true, a path with a drive letter is considered to be 
> > > > relative if the default path style is Posix. But, I don't think that 
> > > > matters. We only end up in this mixed path style situation on Windows.
> > > > 
> > > > Does this change end up being necessary? I would expect the existing 
> > > > implementation of makeAbsolute to do the right thing on Windows as is. 
> > > > I think the other change seems to be what matters.
> > > Yes, it's necessary.  The Posix-style path `\tests` is not considered 
> > > absolute on Windows.  Thus the original makeAboslute would merge it with 
> > > the current working directory, which gives it a drive letter, like 
> > > `D:\tests\`.  The drive letter component then causes comparisons to fail.
> > To make sure I wasn't misremembering or hallucinating, I double-checked the 
> > behavior here:  Posix-style paths like `\tests` are not considered absolute 
> > paths in Windows-style.
> I see, I agree, the platforms diverge here:
>   bool rootDir = has_root_directory(p, style);
>   bool rootName =
>   (real_style(style) != Style::windows) || has_root_name(p, style);
> 
>   return rootDir && rootName;
> 
> So, on Windows rootDir is true and rootName is false.
> 
> I still wonder if this behavior shouldn't be pushed down into the base class. 
> If I pass the path `\test` to the real `FileSystem::makeAbsolute` on Windows, 
> should that prepend the CWD, or should it leave the path alone?
I think we discussed this verbally, and decided we should prepend the CWD, as 
is done here.


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

https://reviews.llvm.org/D70701



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


[PATCH] D69868: Allow "callbr" to return non-void values

2019-12-17 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 234403.
void added a comment.

Fix bad update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868

Files:
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineVerifier.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/callbr-asm.ll

Index: llvm/test/CodeGen/X86/callbr-asm.ll
===
--- llvm/test/CodeGen/X86/callbr-asm.ll
+++ llvm/test/CodeGen/X86/callbr-asm.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=i686-- -O3 | FileCheck %s
+; RUN: llc < %s -mtriple=i686-- -O3 -verify-machineinstrs | FileCheck %s
 
 ; Tests for using callbr as an asm-goto wrapper
 
@@ -131,3 +131,54 @@
   %1 = load i32, i32* %a.addr, align 4
   ret i32 %1
 }
+
+define i32 @test4(i32 %out1, i32 %out2) {
+; Test 4 - asm-goto with output constraints.
+; CHECK-LABEL: test4:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:  movl $-1, %eax
+; CHECK-NEXT:  movl 4(%esp), %ecx
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %ecx, %ecx
+; CHECK-NEXT:  testl %edx, %ecx
+; CHECK-NEXT:  jne .Ltmp5
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB3_1:
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %ecx, %edx
+; CHECK-NEXT:  testl %ecx, %edx
+; CHECK-NEXT:  jne .Ltmp6
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB3_2:
+; CHECK-NEXT:  addl %edx, %ecx
+; CHECK-NEXT:  movl %ecx, %eax
+; CHECK-NEXT:  retl
+; CHECK-NEXT:  .Ltmp5:
+; CHECK-NEXT:  .LBB3_3:
+; CHECK-NEXT:  movl $-2, %eax
+; CHECK-NEXT:  .Ltmp6:
+; CHECK-NEXT:  .LBB3_4:
+; CHECK-NEXT:  retl
+entry:
+  %0 = callbr { i32, i32 } asm sideeffect "testl $0, $0; testl $1, $2; jne ${3:l}", "=r,=r,r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %out1, i8* blockaddress(@test4, %label_true), i8* blockaddress(@test4, %return))
+  to label %asm.fallthrough [label %label_true, label %return]
+
+asm.fallthrough:  ; preds = %entry
+  %asmresult = extractvalue { i32, i32 } %0, 0
+  %asmresult1 = extractvalue { i32, i32 } %0, 1
+  %1 = callbr { i32, i32 } asm sideeffect "testl $0, $1; testl $2, $3; jne ${5:l}", "=r,=r,r,r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %asmresult, i32 %asmresult1, i8* blockaddress(@test4, %label_true), i8* blockaddress(@test4, %return))
+  to label %asm.fallthrough2 [label %label_true, label %return]
+
+asm.fallthrough2: ; preds = %asm.fallthrough
+  %asmresult3 = extractvalue { i32, i32 } %1, 0
+  %asmresult4 = extractvalue { i32, i32 } %1, 1
+  %add = add nsw i32 %asmresult3, %asmresult4
+  br label %return
+
+label_true:   ; preds = %asm.fallthrough, %entry
+  br label %return
+
+return:   ; preds = %entry, %asm.fallthrough, %label_true, %asm.fallthrough2
+  %retval.0 = phi i32 [ %add, %asm.fallthrough2 ], [ -2, %label_true ], [ -1, %asm.fallthrough ], [ -1, %entry ]
+  ret i32 %retval.0
+}
Index: llvm/test/CodeGen/X86/callbr-asm-outputs.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -1,18 +1,118 @@
-; RUN: not llc -mtriple=i686-- < %s 2> %t
-; RUN: FileCheck %s < %t
+; RUN: llc -mtriple=i686-- -verify-machineinstrs < %s | FileCheck %s
 
-; CHECK: error: asm-goto outputs not supported
+; A test for asm-goto output
 
-; A test for asm-goto output prohibition
-
-define i32 @test(i32 %a) {
+; CHECK-LABEL: test1:
+; CHECK:   movl 4(%esp), %eax
+; CHECK-NEXT:  addl $4, %eax
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  xorl %eax, %eax
+; CHECK-NEXT:  jmp .Ltmp0
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB0_1:
+; CHECK-NEXT:  retl
+; CHECK-NEXT:  .Ltmp0:
+define i32 @test1(i32 %x) {
 entry:
-  %0 = add i32 %a, 4
-  %1 = callbr i32 asm "xorl $1, $1; jmp ${1:l}", "=&r,r,X,~{dirflag},~{fpsr},~{flags}"(i32 %0, i8* blockaddress(@test, %fail)) to label %normal [label %fail]
+  %add = add nsw i32 %x, 4
+  %ret = callbr i32 asm "xorl $1, $0; jmp ${2:l}", "=r,r,X,~{dirflag},~{fpsr},~{flags}"(i32 %add, i8* blockaddress(@test1, %abnormal))
+  to label %normal [label %abnormal]
 
 normal:
-  ret i32 %1
+  ret i32 %ret
 
-fail:
+abnormal:
   ret i32 1
 }
+
+; CHECK-LABEL: test2:
+; CHECK:   # %bb.1: # %if.then
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %esi, %esi
+; CHECK-NEXT:  testl %edi, %esi
+; CHECK-NEXT:  jne .Ltmp1
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB1_2:
+; CHECK-NEXT:  jmp .LBB1_4
+; CHECK-NEXT:  .LBB1_3: # %if.else
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  t

[PATCH] D69868: Allow "callbr" to return non-void values

2019-12-17 Thread Bill Wendling via Phabricator via cfe-commits
void added inline comments.



Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:1116
+if (auto *cbr = dyn_cast(getBasicBlock()->getTerminator()))
+  if (cbr->getDefaultDest() != bb)
+for (unsigned i = 0, e = cbr->getNumIndirectDests(); i != e; ++i)

nickdesaulniers wrote:
> void wrote:
> > rnk wrote:
> > > Is it possible for a BB to be both an indirect successor and a 
> > > fallthrough successor? I suppose that could be the case with the Linux 
> > > macro that gets the current PC.
> > > 
> > > In any case, it's probably safe to remove this condition, and then we 
> > > don't have to worry.
> > It is possible. I have a testcase for it in this patch. :-)
> But you don't have a test case for what @rnk is asking about. (Unsure if it 
> would be necessary).
> 
> I think @rnk is getting at this case from C:
> 
> ```
> void foo(void) {
>   asm goto ("#NICK":: "r"(&&hello) :: hello);
> hello:
>   return;
> }
> ```
> Clang will emit this as:
> ```
> define dso_local void @foo() #0 {
> entry:
>   callbr void asm sideeffect "#NICK", "r,X,~{dirflag},~{fpsr},~{flags}"(i8* 
> blockaddress(@foo, %hello), i8* blockaddress(@foo, %hello)) #1
>   to label %asm.fallthrough [label %hello], !srcloc !2
> 
> asm.fallthrough:  ; preds = %entry
>   br label %hello
> 
> hello:; preds = %asm.fallthrough, 
> %entry
>   ret void
> }
> ```
> ie. the `blockaddress` is passed twice, once as the address of a label (GNU C 
> extension), once as the indirect destination of the `asm goto`.
> 
> So to @rnk 's question:
> > Is it possible for a BB to be both an indirect successor and a fallthrough 
> > successor?
> It is valid LLVM IR to have a BB be both; Clang today (or with 
> https://reviews.llvm.org/D69876) will not emit such formation (but could).
> 
> ie. imagine the above:
> 
> >  callbr void asm sideeffect "#NICK", "r,X,~{dirflag},~{fpsr},~{flags}"(i8* 
> > blockaddress(@foo, %hello), i8* blockaddress(@foo, %hello)) #1
> >   to label %asm.fallthrough [label %hello], !srcloc !2
> 
> to instead be:
> 
> >  callbr void asm sideeffect "#NICK", "r,X,~{dirflag},~{fpsr},~{flags}"(i8* 
> > blockaddress(@foo, %hello), i8* blockaddress(@foo, %asm.fallthrough)) #1
> >   to label %asm.fallthrough [label %asm.fallthrough], !srcloc !2
> 
> I still don't understand @rnk 's point about
> > In any case, it's probably safe to remove this condition, and then we don't 
> > have to worry.
> though.
I meant that I had a testcase that required the conditional he suggested I 
remove. Sorry for the confusion.

You're right that we can generate the clang code where the fallthrough is the 
same as the indirect. I didn't mean to imply that it wasn't the case. I can add 
a testcase. I asked a question on the "cfe-dev" mailing list to determine how I 
could identify the "fallthrough" block from the CFG. So far no one has 
responded. Until I can determine that, I won't be able to handle that properly 
with this conditional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868



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


[PATCH] D69868: Allow "callbr" to return non-void values

2019-12-17 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 234402.
void marked 3 inline comments as done.
void added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/Stmt.cpp
  clang/lib/Analysis/UninitializedValues.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/Analysis/uninit-asm-goto.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/Parser/asm-goto.c
  clang/test/Parser/asm-goto.cpp
  clang/test/Sema/asm-goto.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp

Index: llvm/lib/CodeGen/MachineBasicBlock.cpp
===
--- llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -1109,6 +1109,17 @@
   if (Succ->isEHPad())
 return false;
 
+  // Splitting the critical edge to a callbr's indirect block isn't advised.
+  // Don't do it in this generic function.
+  if (Succ->hasAddressTaken())
+if (auto *cbr = dyn_cast(getBasicBlock()->getTerminator()))
+  if (auto *bb = Succ->getBasicBlock())
+if (cbr->getDefaultDest() != bb)
+  if (llvm::any_of(cbr->getIndirectDests(), [&](const BasicBlock *succ){
+  return succ == bb;
+  }))
+return false;
+
   const MachineFunction *MF = getParent();
 
   // Performance might be harmed on HW that implements branching using exec mask
Index: clang/test/Sema/asm-goto.cpp
===
--- clang/test/Sema/asm-goto.cpp
+++ clang/test/Sema/asm-goto.cpp
@@ -1,53 +1,51 @@
 // RUN: %clang_cc1 %s -triple i386-pc-linux-gnu -verify -fsyntax-only
 // RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -verify -fsyntax-only
 
-struct NonTrivial {
-  ~NonTrivial();
+struct S {
+  ~S();
   int f(int);
 private:
   int k;
 };
-void JumpDiagnostics(int n) {
-// expected-error@+1 {{cannot jump from this goto statement to its label}}
+void test1(int n) {
+  // expected-error@+1 {{cannot jump from this goto statement to its label}}
   goto DirectJump;
-// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
-  NonTrivial tnp1;
+  // expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  S s1;
 
 DirectJump:
-// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
   asm goto("jmp %l0;" Later);
-// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
-  NonTrivial tnp2;
-// expected-note@+1 {{possible target of asm goto statement}}
+  // expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  S s2;
+  // expected-note@+1 {{possible target of asm goto statement}}
 Later:
   return;
 }
 
-struct S { ~S(); };
-void foo(int a) {
+struct T { ~T(); };
+void test2(int a) {
   if (a) {
 FOO:
-// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
-// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
-S s;
-void *p = &&BAR;
-// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
-  asm goto("jmp %l0;" BAR);
-// expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
+// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
+// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
+T t;
+void *p = &&bar;
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" bar);
+// expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
 goto *p;
 p = &&FOO;
 goto *p;
 return;
   }
-// expected-note@+2 {{possible target of asm goto statement}}
-// expected-note@+1 {{possible target of indirect goto statement}}
-BAR:
+  // expected-note@+2 {{possible target of asm goto statement}}
+  // expected-note@+1 {{possible target of indirect goto statement}}
+bar:
   return;
 }
 
-
-//Asm goto:
-int test16(int n)
+int test3(int n)
 {
   // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
   // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
@@ -57,7 +55,7 @@
   return ({int a[n];label_true: 2;});
   // expected-note@+1 {{jump bypasses initialization of variable length array}}
   int b[n];
-// expected-note@+1 {{possible target of asm goto statement}}
+  // expected-note@+1 {{possible target of asm goto statement}}
 loop:
   return 0;
 }
Index: clang/test/Parser/asm-g

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D70258#1788427 , @rjmccall wrote:

> In D70258#1788396 , @jdoerfert wrote:
>
> > In D70258#1788305 , @rjmccall 
> > wrote:
> >
> > > Introducing an IRBuilder-level finalization stack sounds like it's going 
> > > to be a huge mess if your goal is to plug this into other frontends.
> >
> >
> > While I get that you don't want to review this, I would really like to 
> > understand why you think this would become a mess.
>
>
> I guess it depends on what you're expecting to be able to achieve with this 
> stack.  Frontends have their own notion of what needs to be finalized and 
> what can trigger control flow.  If your finalization stack is purely for the 
> convenience of your internal IR-generation, it's fine.  If the need for a 
> finalizer can cross the emission of arbitrary frontend code, or if your code 
> needs to emit branches that might cross arbitrary frontend "scope" 
> boundaries, you're going to be in trouble.


Let me explain what it does and why, maybe that helps:

The "finalizer" is a frontend provided callback as only the frontend "knows" 
what values to finalize and "how" to finalize them if we leave the scope. It is 
created only for OpenMP scopes as we only manage leaving those in the 
OMPBuilder. If we generate code for an OpenMP directive that will cause control 
flow to leave the region the finalizer is invoked. Other language finalization, 
e.g., because of nested control flow, will happen undisturbed. It works because 
OpenMP restricts what can happen in a region, e.g. you cannot return from an 
OpenMP parallel region, so all exits have to be "natural" or OpenMP related.

FWIW: We already have (basically) the same scheme in Clang, managed by the 
CGOpenMP and doing basically the same thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


[PATCH] D71625: [CMake] Added remote test execution support into CrossWinToARMLinux CMake cache file.

2019-12-17 Thread Sergej Jaskiewicz via Phabricator via cfe-commits
broadwaylamb added a comment.

LGTM




Comment at: clang/cmake/caches/CrossWinToARMLinux.cmake:88-91
+set(LIBCXXABI_LINK_TESTS_WITH_SHARED_LIBCXXABI  OFF CACHE BOOL "")
+set(LIBCXXABI_LINK_TESTS_WITH_SHARED_LIBCXX OFF CACHE BOOL "")
+set(LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXXOFF CACHE BOOL "")
+set(LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXXABI OFF CACHE BOOL "")

These options haven't yet made it into the upstream, maybe it's better remove 
them for now and add them later when they're stabilized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71625



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


[clang] 6d3f43e - [analysis] Discard type qualifiers when casting values retrieved from the Store.

2019-12-17 Thread Artem Dergachev via cfe-commits

Author: Artem Dergachev
Date: 2019-12-17T15:00:41-08:00
New Revision: 6d3f43ec61a60c37963ee5f54289cf0759fb5d61

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

LOG: [analysis] Discard type qualifiers when casting values retrieved from the 
Store.

This canonicalizes the representation of unknown pointer symbols,
which reduces the overall confusion in pointer cast representation.

Patch by Vince Bridgers!

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

Added: 
clang/test/Analysis/uninit-val-const-likeness.c

Modified: 
clang/lib/StaticAnalyzer/Core/Store.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/Store.cpp 
b/clang/lib/StaticAnalyzer/Core/Store.cpp
index b4ab6877726c..20660d1c2d67 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -393,6 +393,11 @@ SVal StoreManager::attemptDownCast(SVal Base, QualType 
TargetType,
   return UnknownVal();
 }
 
+static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
+  return ty1->getPointeeType().getTypePtr() == 
+ty2->getPointeeType().getTypePtr();
+}
+
 /// CastRetrievedVal - Used by subclasses of StoreManager to implement
 ///  implicit casts that arise from loads from regions that are reinterpreted
 ///  as another region.
@@ -421,10 +426,11 @@ SVal StoreManager::CastRetrievedVal(SVal V, const 
TypedValueRegion *R,
   // FIXME: We really need a single good function to perform casts for us
   // correctly every time we need it.
   if (castTy->isPointerType() && !castTy->isVoidPointerType())
-if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
-  if (SR->getSymbol()->getType().getCanonicalType() !=
-  castTy.getCanonicalType())
-return loc::MemRegionVal(castRegion(SR, castTy));
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) {
+  QualType sr = SR->getSymbol()->getType(); 
+  if (!hasSameUnqualifiedPointeeType(sr, castTy))
+  return loc::MemRegionVal(castRegion(SR, castTy));
+}
 
   return svalBuilder.dispatchCast(V, castTy);
 }

diff  --git a/clang/test/Analysis/uninit-val-const-likeness.c 
b/clang/test/Analysis/uninit-val-const-likeness.c
new file mode 100644
index ..1ee1aefe8dba
--- /dev/null
+++ b/clang/test/Analysis/uninit-val-const-likeness.c
@@ -0,0 +1,56 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core %s -verify 
+// expected-no-diagnostics
+
+#define SIZE 2
+
+typedef struct {
+  int noOfSymbols;
+} Params;
+
+static void create(const Params * const params, int fooList[]) {
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work(Params * const params) {
+  int fooList[SIZE];
+  create(params, fooList);
+  int sum = 0;
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i]; // no-warning
+  return sum;
+}
+
+static void create2(const Params * const * pparams, int fooList[]) {
+  const Params * params = *pparams;
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work2(const Params * const params) {
+  int fooList[SIZE];
+  create2(¶ms, fooList);
+  int sum = 0;
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i]; // no-warning
+  return sum;
+}
+
+static void create3(Params * const * pparams, int fooList[]) {
+  const Params * params = *pparams;
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work3(const Params * const params) {
+  int fooList[SIZE];
+  Params *const *ptr = (Params *const*)¶ms;
+  create3(ptr, fooList);
+  int sum = 0;
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i]; // no-warning
+  return sum;
+}



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


[PATCH] D70836: [analysis] Fix value tracking for pointers to qualified types

2019-12-17 Thread Artem Dergachev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6d3f43ec61a6: [analysis] Discard type qualifiers when 
casting values retrieved from the Store. (authored by dergachev.a).

Changed prior to commit:
  https://reviews.llvm.org/D70836?vs=231473&id=234397#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70836

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/uninit-val-const-likeness.c


Index: clang/test/Analysis/uninit-val-const-likeness.c
===
--- /dev/null
+++ clang/test/Analysis/uninit-val-const-likeness.c
@@ -0,0 +1,56 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core %s -verify 
+// expected-no-diagnostics
+
+#define SIZE 2
+
+typedef struct {
+  int noOfSymbols;
+} Params;
+
+static void create(const Params * const params, int fooList[]) {
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work(Params * const params) {
+  int fooList[SIZE];
+  create(params, fooList);
+  int sum = 0;
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i]; // no-warning
+  return sum;
+}
+
+static void create2(const Params * const * pparams, int fooList[]) {
+  const Params * params = *pparams;
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work2(const Params * const params) {
+  int fooList[SIZE];
+  create2(¶ms, fooList);
+  int sum = 0;
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i]; // no-warning
+  return sum;
+}
+
+static void create3(Params * const * pparams, int fooList[]) {
+  const Params * params = *pparams;
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work3(const Params * const params) {
+  int fooList[SIZE];
+  Params *const *ptr = (Params *const*)¶ms;
+  create3(ptr, fooList);
+  int sum = 0;
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i]; // no-warning
+  return sum;
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -393,6 +393,11 @@
   return UnknownVal();
 }
 
+static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
+  return ty1->getPointeeType().getTypePtr() == 
+ty2->getPointeeType().getTypePtr();
+}
+
 /// CastRetrievedVal - Used by subclasses of StoreManager to implement
 ///  implicit casts that arise from loads from regions that are reinterpreted
 ///  as another region.
@@ -421,10 +426,11 @@
   // FIXME: We really need a single good function to perform casts for us
   // correctly every time we need it.
   if (castTy->isPointerType() && !castTy->isVoidPointerType())
-if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
-  if (SR->getSymbol()->getType().getCanonicalType() !=
-  castTy.getCanonicalType())
-return loc::MemRegionVal(castRegion(SR, castTy));
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) {
+  QualType sr = SR->getSymbol()->getType(); 
+  if (!hasSameUnqualifiedPointeeType(sr, castTy))
+  return loc::MemRegionVal(castRegion(SR, castTy));
+}
 
   return svalBuilder.dispatchCast(V, castTy);
 }


Index: clang/test/Analysis/uninit-val-const-likeness.c
===
--- /dev/null
+++ clang/test/Analysis/uninit-val-const-likeness.c
@@ -0,0 +1,56 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core %s -verify 
+// expected-no-diagnostics
+
+#define SIZE 2
+
+typedef struct {
+  int noOfSymbols;
+} Params;
+
+static void create(const Params * const params, int fooList[]) {
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work(Params * const params) {
+  int fooList[SIZE];
+  create(params, fooList);
+  int sum = 0;
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i]; // no-warning
+  return sum;
+}
+
+static void create2(const Params * const * pparams, int fooList[]) {
+  const Params * params = *pparams;
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work2(const Params * const params) {
+  int fooList[SIZE];
+  create2(¶ms, fooList);
+  int sum = 0;
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i]; // no-warning
+  return sum;
+}
+
+static void create3(Params * const * pparams, int fooList[]) {
+  const Params * params = *pparams;
+  int tmpList[SIZE] = {0};
+  for (int i = 0; i < params->noOfSymbols; i++)
+fooList[i] = tmpList[i];
+}
+
+int work3(const Params * const params) {
+  int fooList[SIZE];
+  Params *const *ptr = (Params *con

[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

2019-12-17 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D71451#1787729 , @Jac1494 wrote:

> In D71451#1786517 , @dblaikie wrote:
>
> > Do you have any particular users/use case for this?
>
>
> A case when shared library built without debug  info 
>  and executable with debug info. And while debugging we want to know the 
> types of extern.


OK - I was/am trying to clarify you have a particular user/need for this, since 
no one's had one for many years, so I'm just slightly surprised.

@aprantl - you folks might want to check what the growth of this is like since 
apple platforms defaults to standalone-debug.
@Jac1494 - have you made any measurements of the size increase of this change? 
Perhaps a self-host build of clang?


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

https://reviews.llvm.org/D71451



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


[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

2019-12-17 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D70524#1787566 , @SouraVX wrote:

> > It looks like this implementation is a bit buggy in one way and incomplete 
> > in another:
> > 
> > 1. even if the auto-returning function is defined, that function definition 
> > doesn't describe the concrete return type. Compare GCC and Clang's output 
> > for your example and note that... oh.
>
> I think that's correct behavior, consider this for a moment --
>
>   struct foo {
>   auto foo_func();
>   };
>   int foo::foo_func(){return 0;}
>   clang error: ->
>   error: return type of out-of-line definition of 'foo::foo_func' differs 
> from that in the declaration
>
>
> So this seems fair to me, regardless of the concrete return type{assuming 
> this is what this patch is doing}. We should be emitting `auto` in 
> declaration. AKA in unspecified_type. GCC(trunk) also seems fair here.


Sorry, the example I had in mind was this:

  struct type {
auto func();
  };
  auto type::func() {
return 3;
  }

Which GCC produces:

  DW_TAG_structure_type
DW_AT_name ("type")
DW_TAG_subprogram
  DW_AT_name ("func")
  DW_AT_type (DW_TAG_unspecified_type "auto")
  ...
  DW_TAG_subprogram
DW_AT_specification (... ^)
DW_AT_type (DW_TAG_base_type "int")
...

(this should be the same debug info even if the function is defined inline in 
the class, rather than defined out of line (assuming the function's called - 
which produces a definition))

>> Hmm, maybe this feature/suggestion is broken or at least not exactly awesome 
>> when it comes to auto-returning functions that are eventually void-returning 
>> functions? Now the function definition has no DW_AT_type to override the 
>> unspecified_type in the declaration... :/ that's unfortunate (@probinson - 
>> thoughts?)
> 
> I'm a bit confused here, regardless of the concrete return 
> type{void/int/float} GCC(trunk) is emitting 
>  `DW_TAG_unspecified_type`
>  `DW_AT_name "auto"` 
>  Are you trying to say we should be emitting `DW_AT_type void/int/float` ?? 
> That's what functionality of clang is right now, and if we entertain that, 
> then their is no point of emitting `DW_TAG_unspecifed_type auto` at first 
> place.

As @probinson said - for the definition DIE/DISubprogram the type should be the 
concrete, deduced type. For the declaration DIE/DISubprogram, the return type 
should be "auto".


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

https://reviews.llvm.org/D70524



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


[PATCH] D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols

2019-12-17 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D70537#1788252 , @probinson wrote:

> @dblaikie let me reflect this back to make sure I get it:
>  Template members (methods or variables) would never appear in the *metadata* 
> description of the struct; but metadata descriptions of the instances would 
> refer back to that struct (as the scope for the instance).  Then DwarfDebug 
> would paste them all together when emitting the DWARF description(s).  The 
> in-struct child DIE could then have the deduced type because by the time the 
> definition metadata is produced, we actually know what that type is.  This is 
> okay because template data members are necessarily static; they don't affect 
> size or layout of the struct in any way.
>
> So, CGDebugInfo would skip basically any templated member when constructing 
> the struct, but when the template is (finally, fully) instantiated, its 
> definition could know everything it needs to, and point back to the struct.
>
> Sounds reasonable.


Right - and that's what's already done for member function templates & what 
should be (but isn't currently) done for member variable templates & I think 
that would fix this bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70537



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


[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

2019-12-17 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo marked an inline comment as done.
mstorsjo added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2551
+  // actually handle multiple TUs defining the same wrapper.
+  if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() &&
+  Wrapper->isWeakForLinker())

mstorsjo wrote:
> rnk wrote:
> > IMO we should be doing the same thing on ELF, you can see the code pattern 
> > used elsewhere:
> > http://llvm-cs.pcc.me.uk/?q=setComdat
> > 
> > Before comdats were made explicit in the IR and we stopped implicitly 
> > making comdat groups for odr things on ELF, these wrappers would've been in 
> > a comdat group.
> Ok, so remove the `isOSWindows()` and maybe remove the comment altogether as 
> it isn't windows specific any longer?
In the current tests, for linux there's explicit checks that these aren't 
marked as comdat, see e.g. 
https://github.com/llvm/llvm-project/blob/master/clang/test/CodeGenCXX/cxx11-thread-local.cpp#L208.
 Is there a specific reason for that, or should it be changed?


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

https://reviews.llvm.org/D71572



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


[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

2019-12-17 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

@phosek I fixed the tests in be88a20c900463c4919433109e4c17cd001da580 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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


[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

2019-12-17 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo marked an inline comment as done.
mstorsjo added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2551
+  // actually handle multiple TUs defining the same wrapper.
+  if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() &&
+  Wrapper->isWeakForLinker())

rnk wrote:
> IMO we should be doing the same thing on ELF, you can see the code pattern 
> used elsewhere:
> http://llvm-cs.pcc.me.uk/?q=setComdat
> 
> Before comdats were made explicit in the IR and we stopped implicitly making 
> comdat groups for odr things on ELF, these wrappers would've been in a comdat 
> group.
Ok, so remove the `isOSWindows()` and maybe remove the comment altogether as it 
isn't windows specific any longer?


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

https://reviews.llvm.org/D71572



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


[clang] be88a20 - [driver][darwin] Use explicit -mlinker-version in the -platform_version tests

2019-12-17 Thread Alex Lorenz via cfe-commits

Author: Alex Lorenz
Date: 2019-12-17T14:25:22-08:00
New Revision: be88a20c900463c4919433109e4c17cd001da580

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

LOG: [driver][darwin] Use explicit -mlinker-version in the -platform_version 
tests

The driver actually adds a default -mlinker-version, based on HOST_LINK_VERSION
cmake variable. The tests should be explicit about which version they're using 
to
trigger the right behavior.

Added: 


Modified: 
clang/test/Driver/darwin-ld-platform-version-ios.c
clang/test/Driver/darwin-ld-platform-version-macos.c
clang/test/Driver/darwin-ld-platform-version-tvos.c
clang/test/Driver/darwin-ld-platform-version-watchos.c

Removed: 




diff  --git a/clang/test/Driver/darwin-ld-platform-version-ios.c 
b/clang/test/Driver/darwin-ld-platform-version-ios.c
index 157aa18bc32f..2255d3f990d7 100644
--- a/clang/test/Driver/darwin-ld-platform-version-ios.c
+++ b/clang/test/Driver/darwin-ld-platform-version-ios.c
@@ -1,8 +1,8 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target arm64-apple-ios12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-ios12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck %s
-// RUN: %clang -target x86_64-apple-ios13-simulator -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -### %t.o 2>&1 \
+// RUN: %clang -target x86_64-apple-ios13-simulator -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
 // CHECK: "-platform_version" "ios" "12.3.0" "13.0"

diff  --git a/clang/test/Driver/darwin-ld-platform-version-macos.c 
b/clang/test/Driver/darwin-ld-platform-version-macos.c
index 1e86a28b05b6..1b849a6d2dfa 100644
--- a/clang/test/Driver/darwin-ld-platform-version-macos.c
+++ b/clang/test/Driver/darwin-ld-platform-version-macos.c
@@ -1,12 +1,12 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target x86_64-apple-macos10.13 -isysroot 
%S/Inputs/MacOSX10.14.sdk -### %t.o 2>&1 \
+// RUN: %clang -target x86_64-apple-macos10.13 -isysroot 
%S/Inputs/MacOSX10.14.sdk -mlinker-version=0 -### %t.o 2>&1 \
 // RUN:   | FileCheck %s
 // RUN: env SDKROOT=%S/Inputs/MacOSX10.14.sdk %clang -target 
x86_64-apple-macos10.13.0.1 -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck %s
 
 // CHECK: "-platform_version" "macos" "10.13.0" "10.14"
 
-// RUN: %clang -target x86_64-apple-macos10.13 -### %t.o 2>&1 \
+// RUN: %clang -target x86_64-apple-macos10.13  -mlinker-version=520 -### %t.o 
2>&1 \
 // RUN:   | FileCheck --check-prefix=NOSDK %s
 // NOSDK: "-platform_version" "macos" "10.13.0" "0.0.0"

diff  --git a/clang/test/Driver/darwin-ld-platform-version-tvos.c 
b/clang/test/Driver/darwin-ld-platform-version-tvos.c
index cb32f7096f5c..1ef6a0b60004 100644
--- a/clang/test/Driver/darwin-ld-platform-version-tvos.c
+++ b/clang/test/Driver/darwin-ld-platform-version-tvos.c
@@ -1,8 +1,8 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target arm64-apple-tvos12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck %s
-// RUN: %clang -target x86_64-apple-tvos13-simulator -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -### %t.o 2>&1 \
+// RUN: %clang -target x86_64-apple-tvos13-simulator -isysroot 
%S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
 // CHECK: "-platform_version" "tvos" "12.3.0" "13.0"

diff  --git a/clang/test/Driver/darwin-ld-platform-version-watchos.c 
b/clang/test/Driver/darwin-ld-platform-version-watchos.c
index f2666b014f0e..9663fc613816 100644
--- a/clang/test/Driver/darwin-ld-platform-version-watchos.c
+++ b/clang/test/Driver/darwin-ld-platform-version-watchos.c
@@ -1,8 +1,8 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot 
%S/Inputs/WatchOS6.0.sdk -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot 
%S/Inputs/WatchOS6.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck %s
-// RUN: %clang -target x86_64-apple-watchos6-simulator -isysroot 
%S/Inputs/WatchOS6.0.sdk -### %t.o 2>&1 \
+// RUN: %clang -target x86_64-apple-watchos6-simulator -isysroot 
%S/Inputs/WatchOS6.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
 // CHECK: "-platform_version" "watchos" "5.2.0" "6.0.0"



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


[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

2019-12-17 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

Looks like `HOST_LINK_VERSION` is influencing the decision. I should specify 
the version manually in the tests then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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


[PATCH] D71408: [lit] Remove lit's REQUIRES-ANY directive

2019-12-17 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre added a subscriber: nemanjai.
thopre added a comment.
Herald added a subscriber: wuzish.

In D71408#1787959 , @jdenny wrote:

> This breaks the lit test suite.  `llvm/utils/lit/tests/shtest-format.py` 
> needs to be updated for the removed tests.


This is embarrassing. Thanks @nemanjai for fixing this. I did run the tests and 
remember having one FAIL which I could reproduce without the patch applied. I 
presume ninja check-all includes those 2 tests so I'm not sure how I missed it. 
On a related note, I used to receive mail notification when one of my commit 
would make a buildbot fail but don't anymore. Is there a way to register for 
those mail notification?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71408



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


[PATCH] D71408: [lit] Remove lit's REQUIRES-ANY directive

2019-12-17 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment.

In D71408#1788582 , @thopre wrote:

> This is embarrassing.


It happens.

> I presume ninja check-all includes those 2 tests so I'm not sure how I missed 
> it.

It should.

> On a related note, I used to receive mail notification when one of my commit 
> would make a buildbot fail but don't anymore.

Sometimes I don't see emails if a bot was already broken.  I noticed this fail 
only after pulling and running check-lit locally.

> Is there a way to register for those mail notification?

Not sure.  I thought it was based on the commit author field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71408



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


[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

2019-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2551
+  // actually handle multiple TUs defining the same wrapper.
+  if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() &&
+  Wrapper->isWeakForLinker())

IMO we should be doing the same thing on ELF, you can see the code pattern used 
elsewhere:
http://llvm-cs.pcc.me.uk/?q=setComdat

Before comdats were made explicit in the IR and we stopped implicitly making 
comdat groups for odr things on ELF, these wrappers would've been in a comdat 
group.


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

https://reviews.llvm.org/D71572



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


[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

2019-12-17 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

@phosek Thanks, I think it reproduces on our macOS bots. I'll start fixing it 
right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71579



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


[PATCH] D71579: [driver][darwin] Pass -platform_version flag to the linker instead of the -_version_min flag

2019-12-17 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

I'm not sure if the detection is working correctly, this is failing on our 
macOS toolchain bots:

  FAIL: Clang :: Driver/darwin-ld-platform-version-ios.c (5284 of 16450)
   TEST 'Clang :: Driver/darwin-ld-platform-version-ios.c' 
FAILED 
  Script:
  --
  : 'RUN: at line 1';   touch 
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-ios.c.tmp.o
  : 'RUN: at line 3';   
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/clang -target 
arm64-apple-ios12.3 -isysroot 
/b/s/w/ir/k/llvm-project/clang/test/Driver/Inputs/iPhoneOS13.0.sdk -### 
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-ios.c.tmp.o
 2>&1| /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/FileCheck 
/b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-ios.c
  : 'RUN: at line 5';   
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/clang -target 
x86_64-apple-ios13-simulator -isysroot 
/b/s/w/ir/k/llvm-project/clang/test/Driver/Inputs/iPhoneOS13.0.sdk -### 
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-ios.c.tmp.o
 2>&1| /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/FileCheck 
--check-prefix=SIMUL 
/b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-ios.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
/b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-ios.c:8:11:
 error: CHECK: expected string not found in input
  // CHECK: "-platform_version" "ios" "12.3.0" "13.0"
^
  :1:1: note: scanning from here
  Fuchsia clang version 10.0.0 
(https://fuchsia.googlesource.com/a/third_party/llvm-project 
1a8ff89653d2a80a013701fe927d2d32491bff59)
  ^
  :5:141: note: possible intended match here
   "/usr/bin/ld" "-demangle" "-lto_library" 
"/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/lib/libLTO.dylib" 
"-dynamic" "-arch" "arm64" "-iphoneos_version_min" "12.3.0" "-syslibroot" 
"/b/s/w/ir/k/llvm-project/clang/test/Driver/Inputs/iPhoneOS13.0.sdk" "-o" 
"a.out" 
"/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-ios.c.tmp.o"
 "-lSystem" 
"/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/lib/clang/10.0.0/lib/darwin/libclang_rt.ios.a"

  ^
  
  --
  
  
  Testing:  0.. 10.. 20.. 30
  FAIL: Clang :: Driver/darwin-ld-platform-version-macos.c (5286 of 16450)
   TEST 'Clang :: 
Driver/darwin-ld-platform-version-macos.c' FAILED 
  Script:
  --
  : 'RUN: at line 1';   touch 
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-macos.c.tmp.o
  : 'RUN: at line 3';   
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/clang -target 
x86_64-apple-macos10.13 -isysroot 
/b/s/w/ir/k/llvm-project/clang/test/Driver/Inputs/MacOSX10.14.sdk -### 
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-macos.c.tmp.o
 2>&1| /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/FileCheck 
/b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c
  : 'RUN: at line 5';   env 
SDKROOT=/b/s/w/ir/k/llvm-project/clang/test/Driver/Inputs/MacOSX10.14.sdk 
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/clang -target 
x86_64-apple-macos10.13.0.1 -mlinker-version=520 -### 
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-macos.c.tmp.o
 2>&1| /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/FileCheck 
/b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c
  : 'RUN: at line 10';   
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/clang -target 
x86_64-apple-macos10.13 -### 
/b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/tools/clang/test/Driver/Output/darwin-ld-platform-version-macos.c.tmp.o
 2>&1| /b/s/w/ir/k/recipe_cleanup/clangybBdXS/llvm_build_dir/bin/FileCheck 
--check-prefix=NOSDK 
/b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
/b/s/w/ir/k/llvm-project/clang/test/Driver/darwin-ld-platform-version-macos.c:8:11:
 error: CHECK: expected string not found in input
  // CHECK: "-platform_version" "macos" "10.13.0" "10.14"
^
  :1:1: note: scanning from here
  Fuchsia clang version 10.0.0 
(https://fuchsia.googlesource.com/a/third_party/llvm-project 
1a8ff89653d2a80a013701fe927d2d32491bff59)
  ^
  :5:364: note: possible intended match here
   "/usr/bin/ld" "-demangle" "-lto_library" 
"/

[PATCH] D71612: [analyzer] Add PlacementNewChecker

2019-12-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:40
+if (const auto *ERegion = dyn_cast(TVRegion)) {
+  const auto *SRegion = cast(ERegion->getSuperRegion());
+  DefinedOrUnknownSVal Extent = SRegion->getExtent(svalBuilder);

xazax.hun wrote:
> Hmm, I think this logic might need some more testing. Could you add some 
> tests with multi dimensional arrays?
Yeah, this code is scary because at this point literally nobody knows when 
exactly do we an have element region wrapping the pointer 
(https://bugs.llvm.org/show_bug.cgi?id=43364).

`MemRegion` represents a segment of memory, whereas `loc::MemRegionVal` 
represents the point that is the left-hand side of that segment. I recommend 
using `C.getSVal(Place).getAsRegion()` only as a reference to that point, not 
the whole segment. Then you could decompose the region into a base region and 
an offset (i.e., `MemRegion::getAsOffset()`), and subtract the offset from the 
base region's extent to see how much space is there in the region.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:65
+  if (!State)
+return SVal();
+  SValBuilder &svalBuilder = C.getSValBuilder();

That produces an `UndefinedVal`. I think you'd much rather have `UnknownVal`.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:115-117
+  llvm::formatv("Argument of default placement new provides storage "
+"capacity of {0} bytes, but the allocated type "
+"requires storage capacity of {1} bytes",

whisperity wrote:
> This message might be repeating phrases too much, and seems long. Also, I 
> would expect things like //default placement new// or //argument of placement 
> new// to be confusing. Not every person running Clang SA knows the 
> nitty-gritty of the standard by heart...
> 
> More nitpicking: even the "default" (what does this even mean, again?) 
> placement new takes **two** arguments, albeit written in a weird grammar, so 
> there is no "argument of" by the looks of it. I really think this is 
> confusing.
> 
> Something more concise, simpler, still getting the meaning across:
> 
> > Storage provided to placement new is only `N` bytes, whereas allocating a 
> > `T` requires `M` bytes
> 
Having long messages is usually not a problem for us (instead, we'd much rather 
have full sentences properly explaining what's going on), but i agree that your 
text is much neater and on point.



Comment at: clang/test/Analysis/placement-new.cpp:6
+// RUN:   -analyzer-output=text -verify \
+// RUN:   -triple x86_64-unknown-linux-gnu
+

Wow, somebody actually remembers to add a target triple for tests that depend 
on the target triple! My respect, sir.



Comment at: clang/test/Analysis/placement-new.cpp:11
+void f() {
+  short s; // expected-note-re {{'s' declared{{.*
+  long *lp = ::new (&s) long; // expected-warning{{Argument of default 
placement new provides storage capacity of 2 bytes, but the allocated type 
requires storage capacity of 8 bytes}} expected-note 3 {{}}

I'm legit curious what's hidden behind the regex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71612



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


[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

2019-12-17 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment.

In D68913#1787784 , @thakis wrote:

> Mordante, do you need someone to land this for you?


Thanks for the offer, but it's not required. I want to commit it once I fixed 
all issues that would be introduced by D68912 


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

https://reviews.llvm.org/D68913



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


[PATCH] D71556: [AArch64][SVE] Implement intrinsic for non-faulting loads

2019-12-17 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> Perhaps we can clarify the intent that the non-faulting mode may have 
> side-effects by renaming the flag to something like 
> NonFaultingWithSideEffects?

We could specify something like that... but that probably requires some changes 
to target-independent DAGCombine, and it's sort of a weird edge case given that 
no other loads have that kind of side-effect. If we are going to make a change 
like that to MachineMemOperand, we'd probably want to propose it on llvmdev.  
I'd prefer a little extra code here to avoid that rabbit hole.  We're busy 
enough already with other substantial changes to target-independent code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71556



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


[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436



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


[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

2019-12-17 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

In D70157#1788418 , @reames wrote:

> In D70157#1788025 , @jyknight wrote:
>
> > > .push_align_branch_boundary [N,] [instruction,]*
> >
> > I'd like to raise again the possibility of using a more general region 
> > directive to denote "It is allowable to add prefixes/nops before 
> > instructions in this region if the assembler wants to", as I'd started 
> > discussing in https://reviews.llvm.org/D71238#1786885 (but let's move the 
> > discussion here).
>
>
> James, I think this proposal is increasing the scope of this proposal too 
> much.  It also ignores some of the use cases identified and described in the 
> writeup (i.e. the scoped semantics).  I'm open to discussing such a feature 
> more generally, but I'd prefer to see a more narrowly focused feature 
> immediately.


I do not intend that we expand the scope of the project to include any of the 
other features.

All I want is to slightly consider surrounding features when adding the new 
assembly syntax. The situations where we want to avoid modifying a certain 
block of code are extremely likely to apply to //any// 
nop-or-prefix-introducing code modifications -- not just modifications 
resulting from branch alignment. So if we can make the directives annotating 
where such changes are allowable (and conversely, where they are not) 
generally-applicable, with a very minimal amount of work, that would be nice.

I also don't understand what you mean by "it ignores [...] scoped semantics"?


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

https://reviews.llvm.org/D70157



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


[PATCH] D71625: [CMake] Added remote test execution support into CrossWinToARMLinux CMake cache file.

2019-12-17 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka created this revision.
vvereschaka added reviewers: aorlov, andreil99, broadwaylamb.
Herald added subscribers: cfe-commits, kristof.beyls, mgorny.
Herald added a project: clang.

Added two confguration argument to provide a host name and SSH user name to run 
the tests on the remote target host.

  

- REMOTE_TEST_EXECUTE_HOST  - remote host name or address.
- REMOTE_TEST_EXECUTE_USER  - passwordless SSH account name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71625

Files:
  clang/cmake/caches/CrossWinToARMLinux.cmake


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,6 +14,8 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
+#   -DREMOTE_TEST_EXECUTE_HOST="" ^
+#   -DREMOTE_TEST_EXECUTE_USER="" ^
 #   
-C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
@@ -82,7 +84,39 @@
 set(BUILTINS_CMAKE_ARGS 
"-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 
-set(LLVM_INSTALL_TOOLCHAIN_ONLYON CACHE BOOL "")
+# Test configuration.
+set(LIBCXXABI_LINK_TESTS_WITH_SHARED_LIBCXXABI  OFF CACHE BOOL "")
+set(LIBCXXABI_LINK_TESTS_WITH_SHARED_LIBCXX OFF CACHE BOOL "")
+set(LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXXOFF CACHE BOOL "")
+set(LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXXABI OFF CACHE BOOL "")
+
+# Remote tests.
+if(DEFINED REMOTE_TEST_EXECUTE_HOST)
+  set(DEFAULT_TEST_EXECUTOR 
"SSHExecutor('${REMOTE_TEST_EXECUTE_HOST}', '${REMOTE_TEST_EXECUTE_USER}')")
+  set(DEFAULT_TEST_TARGET_INFO  
"libcxx.test.target_info.LinuxLocalTI")
+
+  # Allow override with the custom values.
+  if(NOT DEFINED LIBUNWIND_TARGET_INFO)
+set(LIBUNWIND_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBUNWIND_EXECUTOR)
+set(LIBUNWIND_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_TARGET_INFO)
+set(LIBCXXABI_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_EXECUTOR)
+set(LIBCXXABI_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_TARGET_INFO)
+set(LIBCXX_TARGET_INFO  "${DEFAULT_TEST_TARGET_INFO}" 
CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXX_EXECUTOR)
+set(LIBCXX_EXECUTOR "${DEFAULT_TEST_EXECUTOR}" CACHE 
STRING "")
+  endif()
+endif()
+
+set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
   llvm-ar
   llvm-cov


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,6 +14,8 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
+#   -DREMOTE_TEST_EXECUTE_HOST="" ^
+#   -DREMOTE_TEST_EXECUTE_USER="" ^
 #   -C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
@@ -82,7 +84,39 @@
 set(BUILTINS_CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 set(RUNTIMES_CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux;-DCMAKE_AR=${CMAKE_AR}" CACHE STRING "")
 
-set(LLVM_INSTALL_TOOLCHAIN_ONLY 			ON CACHE BOOL "")
+# Test configuration.
+set(LIBCXXABI_LINK_TESTS_WITH_SHARED_LIBCXXABI  OFF CACHE BOOL "")
+set(LIBCXXABI_LINK_TESTS_WITH_SHARED_LIBCXX OFF CACHE BOOL "")
+set(LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXXOFF CACHE BOOL "")
+set(LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXXABI OFF CACHE BOOL "")
+
+# Remote tests.
+if(DEFINED REMOTE_TEST_EXECUTE_HOST)
+  set(DEFAULT_TEST_EXECUTOR "SSHExecutor('${REMOTE_TEST_EXECUTE_HOST}', '${REMOTE_TEST_EXECUTE_USER}')")
+  set(DEFAULT_TEST_TARGET_INFO  "libcxx.test.target_info.LinuxLocalTI")
+
+  # Allow override with the custom values.
+  if(NOT DEFINED LIBUNWIND_TARGET_INFO)
+set(LIBUNWIND_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBUNWIND_EXECUTOR)
+set(LIBUNWIND_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_TARGET_INFO)
+set(LIBCXXABI_TARGET_INFO   "${DEFAULT_TEST_TARGET_INFO}" CACHE STRING "")
+  endif()
+  if(NOT DEFINED LIBCXXABI_EXECUTOR)
+set(LIBCXXABI_EXECUTOR  "${DEFAULT_TEST_EXECUTOR}" CACHE ST

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-17 Thread Eric Astor via Phabricator via cfe-commits
epastor added a comment.

Thanks for feedback!




Comment at: clang/test/CodeGen/ms-inline-asm-64.c:18
 // CHECK: call void asm sideeffect inteldialect
-// CHECK-SAME: mov [eax], $0
+// CHECK-SAME: mov qword ptr [eax], $0
 // CHECK-SAME: "r,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}})

rnk wrote:
> I guess this has the same semantics as it did before.
It does; this previously resolved to a "qword ptr" mov automatically, since $0 
unpacked as a (forced) 64-bit register name. Now we need to specify the size of 
the move, since the operand is an immediate rather than a register.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821
+getParser().parsePrimaryExpr(Val, End))
+  return Error(Start, "unexpected token!");
+  } else if (ParseIntelInlineAsmIdentifier(Val, ID, Info, false, End, true)) {

rnk wrote:
> Please test this corner case, I imagine it looks like:
>   mov eax, offset 3
Interesting. This corner case didn't trigger in that scenario; we get an 
"expected identifier" error message with good source location, followed by 
another error "use of undeclared label '3'" in debug builds... and in release 
builds, we instead get a crash. On tracing the crash, it's a AsmStrRewrite 
applying to a SMLoc not coming from the same string...

As near as I can tell, the issue is that we end up trying to parse "3" as a 
not-yet-declared label, as such expand it to `__MSASMLABEL_.${:uid}__3`, and 
then end up in a bad state because the operand rewrite is applying to the 
expanded symbol... which isn't in the same AsmString. If you use an actual 
undeclared label, you hit the same crash in release builds.

This is going to take some work; I'll get back to it in a day or two.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1825
+  } else if (Info.isKind(InlineAsmIdentifierInfo::IK_EnumVal)) {
+return Error(Start, "offset operator cannot yet handle constants");
+  }

rnk wrote:
> Please add a test for this corner case, I'm curious to see if the error 
> reporting really works.
This error reporting doesn't work, in fact. We instead get "cannot take the 
address of an rvalue of type 'int'", with bad source location. Will investigate 
why we end up there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436



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


[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-17 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 234371.
epastor marked 6 inline comments as done.
epastor added a comment.

- Address refactoring comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/TableGen/TGParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,13 +279,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isMem() const override { return Kind == Memory; }
   bool isMemUnsized() const {
@@ -613,9 +610,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ 

[PATCH] D71529: [Sema] Fixes -Wrange-loop-analysis warnings

2019-12-17 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ec6128daeea: [Sema] Fixes -Wrange-loop-analysis warnings 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71529

Files:
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -1309,7 +1309,7 @@
 /// Motivating case is const_iterator begin() const vs iterator 
begin().
 auto &OverloadSet = OverloadMap[std::make_pair(
 CurContext, Method->getDeclName().getAsOpaqueInteger())];
-for (const DeclIndexPair& Entry : OverloadSet) {
+for (const DeclIndexPair Entry : OverloadSet) {
   Result &Incumbent = Results[Entry.second];
   switch (compareOverloads(*Method,
*cast(Incumbent.Declaration),
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1174,7 +1174,7 @@
 // We analyze lambda bodies separately. Skip them here.
 bool TraverseLambdaExpr(LambdaExpr *LE) {
   // Traverse the captures, but not the body.
-  for (const auto &C : zip(LE->captures(), LE->capture_inits()))
+  for (const auto C : zip(LE->captures(), LE->capture_inits()))
 TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C));
   return true;
 }


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -1309,7 +1309,7 @@
 /// Motivating case is const_iterator begin() const vs iterator begin().
 auto &OverloadSet = OverloadMap[std::make_pair(
 CurContext, Method->getDeclName().getAsOpaqueInteger())];
-for (const DeclIndexPair& Entry : OverloadSet) {
+for (const DeclIndexPair Entry : OverloadSet) {
   Result &Incumbent = Results[Entry.second];
   switch (compareOverloads(*Method,
*cast(Incumbent.Declaration),
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1174,7 +1174,7 @@
 // We analyze lambda bodies separately. Skip them here.
 bool TraverseLambdaExpr(LambdaExpr *LE) {
   // Traverse the captures, but not the body.
-  for (const auto &C : zip(LE->captures(), LE->capture_inits()))
+  for (const auto C : zip(LE->captures(), LE->capture_inits()))
 TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C));
   return true;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71527: [Driver] Avoid copies in range-based for loops

2019-12-17 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb750486c5d96: [Driver] Avoid copies in range-based for loops 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71527

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp


Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -850,7 +850,7 @@
 /*static*/ void ToolChain::addSystemIncludes(const ArgList &DriverArgs,
  ArgStringList &CC1Args,
  ArrayRef Paths) {
-  for (const auto Path : Paths) {
+  for (const auto &Path : Paths) {
 CC1Args.push_back("-internal-isystem");
 CC1Args.push_back(DriverArgs.MakeArgString(Path));
   }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -744,7 +744,7 @@
   ArrayRef Dirs,
   StringRef FileName) {
   SmallString<128> WPath;
-  for (const StringRef &Dir : Dirs) {
+  for (const std::string &Dir : Dirs) {
 if (Dir.empty())
   continue;
 WPath.clear();


Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -850,7 +850,7 @@
 /*static*/ void ToolChain::addSystemIncludes(const ArgList &DriverArgs,
  ArgStringList &CC1Args,
  ArrayRef Paths) {
-  for (const auto Path : Paths) {
+  for (const auto &Path : Paths) {
 CC1Args.push_back("-internal-isystem");
 CC1Args.push_back(DriverArgs.MakeArgString(Path));
   }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -744,7 +744,7 @@
   ArrayRef Dirs,
   StringRef FileName) {
   SmallString<128> WPath;
-  for (const StringRef &Dir : Dirs) {
+  for (const std::string &Dir : Dirs) {
 if (Dir.empty())
   continue;
 WPath.clear();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] b750486 - [Driver] Avoid copies in range-based for loops

2019-12-17 Thread Mark de Wever via cfe-commits

Author: Mark de Wever
Date: 2019-12-17T21:56:04+01:00
New Revision: b750486c5d96320daf77a9760166f78b4a0c942e

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

LOG: [Driver] Avoid copies in range-based for loops

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChain.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 9139915761d1..f2b234315512 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -744,7 +744,7 @@ static bool searchForFile(SmallVectorImpl &FilePath,
   ArrayRef Dirs,
   StringRef FileName) {
   SmallString<128> WPath;
-  for (const StringRef &Dir : Dirs) {
+  for (const std::string &Dir : Dirs) {
 if (Dir.empty())
   continue;
 WPath.clear();

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index ba4128e06226..cab97b1a601a 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -850,7 +850,7 @@ void ToolChain::addExternCSystemIncludeIfExists(const 
ArgList &DriverArgs,
 /*static*/ void ToolChain::addSystemIncludes(const ArgList &DriverArgs,
  ArgStringList &CC1Args,
  ArrayRef Paths) {
-  for (const auto Path : Paths) {
+  for (const auto &Path : Paths) {
 CC1Args.push_back("-internal-isystem");
 CC1Args.push_back(DriverArgs.MakeArgString(Path));
   }



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


[clang] 3ec6128 - [Sema] Fixes -Wrange-loop-analysis warnings

2019-12-17 Thread Mark de Wever via cfe-commits

Author: Mark de Wever
Date: 2019-12-17T21:54:32+01:00
New Revision: 3ec6128daeeaadde7dd2eca8b343a56f60a8364d

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

LOG: [Sema] Fixes -Wrange-loop-analysis warnings

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/SemaCodeComplete.cpp

Removed: 




diff  --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp 
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 2c70c0599ecf..04611dadde66 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1174,7 +1174,7 @@ namespace {
 // We analyze lambda bodies separately. Skip them here.
 bool TraverseLambdaExpr(LambdaExpr *LE) {
   // Traverse the captures, but not the body.
-  for (const auto &C : zip(LE->captures(), LE->capture_inits()))
+  for (const auto C : zip(LE->captures(), LE->capture_inits()))
 TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C));
   return true;
 }

diff  --git a/clang/lib/Sema/SemaCodeComplete.cpp 
b/clang/lib/Sema/SemaCodeComplete.cpp
index e75aca4d81e5..b4299b615a61 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -1309,7 +1309,7 @@ void ResultBuilder::AddResult(Result R, DeclContext 
*CurContext,
 /// Motivating case is const_iterator begin() const vs iterator 
begin().
 auto &OverloadSet = OverloadMap[std::make_pair(
 CurContext, Method->getDeclName().getAsOpaqueInteger())];
-for (const DeclIndexPair& Entry : OverloadSet) {
+for (const DeclIndexPair Entry : OverloadSet) {
   Result &Incumbent = Results[Entry.second];
   switch (compareOverloads(*Method,
*cast(Incumbent.Declaration),



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


[PATCH] D71623: [compiler-rt] [test] Add missing %run to fread_fwrite MSAN test

2019-12-17 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision.
mgorny added reviewers: krytarowski, m.ostapenko, eugenis.
Herald added subscribers: llvm-commits, Sanitizers, dberris.
Herald added projects: LLVM, Sanitizers.

Add a missing %run substitution to fread_fwrite test.  This fixes
the test on NetBSD where %run disables ASLR as necessary for MSAN
to function.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D71623

Files:
  compiler-rt/test/msan/fread_fwrite.cpp


Index: compiler-rt/test/msan/fread_fwrite.cpp
===
--- compiler-rt/test/msan/fread_fwrite.cpp
+++ compiler-rt/test/msan/fread_fwrite.cpp
@@ -1,5 +1,5 @@
 // RUN: %clangxx_msan -g %s -o %t
-// RUN: not %t 2>&1 | FileCheck %s
+// RUN: not %run %t 2>&1 | FileCheck %s
 // RUN: %t 1
 
 #include 


Index: compiler-rt/test/msan/fread_fwrite.cpp
===
--- compiler-rt/test/msan/fread_fwrite.cpp
+++ compiler-rt/test/msan/fread_fwrite.cpp
@@ -1,5 +1,5 @@
 // RUN: %clangxx_msan -g %s -o %t
-// RUN: not %t 2>&1 | FileCheck %s
+// RUN: not %run %t 2>&1 | FileCheck %s
 // RUN: %t 1
 
 #include 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

2019-12-17 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> Unfortunately on RISCV, the ABI info is only derived from -mabi option and 
> the target triple does not encode ABI info (same to gcc).

How does gcc decide the default ABI for a target?  Do you explicitly specify it 
at configure time?

> example 1:
>  What's expected behavior? user specifics -mabi=lp64f so the result object 
> ABI is lp64f?

I think the expected behavior is a fatal error.  There isn't any reason to let 
people shoot themselves in the foot like this.  (module flags have builtin 
support for producing such an error.)

> example 2:
>  What's expected behavior? The result object is -mabi=lp64f because they have 
> same ABI in module metadata?

Yes.

> there is no problem (I think) when overwriting the original ABI. (so the 
> mixing ABI is not a problem)

clang emits different IR for hardfloat vs. softfloat targets; if you try to 
overwrite the ABI of a bitcode file, we'll generate wrong code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71387



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


[PATCH] D71530: [Frontend] Fixes -Wrange-loop-analysis warnings

2019-12-17 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG385339034f08: [Frontend] Fixes -Wrange-loop-analysis 
warnings (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71530

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -334,7 +334,7 @@
 StringRef CheckerAndPackageList = A->getValue();
 SmallVector CheckersAndPackages;
 CheckerAndPackageList.split(CheckersAndPackages, ",");
-for (const StringRef CheckerOrPackage : CheckersAndPackages)
+for (const StringRef &CheckerOrPackage : CheckersAndPackages)
   Opts.CheckersAndPackages.emplace_back(CheckerOrPackage, IsEnabled);
   }
 
@@ -476,7 +476,7 @@
 SmallVector CheckersAndPackages;
 AnOpts.RawSilencedCheckersAndPackages.split(CheckersAndPackages, ";");
 
-for (const StringRef CheckerOrPackage : CheckersAndPackages) {
+for (const StringRef &CheckerOrPackage : CheckersAndPackages) {
   if (Diags) {
 bool IsChecker = CheckerOrPackage.contains('.');
 bool IsValidName =
@@ -607,7 +607,7 @@
XRayInstrSet &S) {
   llvm::SmallVector BundleParts;
   llvm::SplitString(Bundle, BundleParts, ",");
-  for (const auto B : BundleParts) {
+  for (const auto &B : BundleParts) {
 auto Mask = parseXRayInstrValue(B);
 if (Mask == XRayInstrKind::None)
   if (B != "none")


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -334,7 +334,7 @@
 StringRef CheckerAndPackageList = A->getValue();
 SmallVector CheckersAndPackages;
 CheckerAndPackageList.split(CheckersAndPackages, ",");
-for (const StringRef CheckerOrPackage : CheckersAndPackages)
+for (const StringRef &CheckerOrPackage : CheckersAndPackages)
   Opts.CheckersAndPackages.emplace_back(CheckerOrPackage, IsEnabled);
   }
 
@@ -476,7 +476,7 @@
 SmallVector CheckersAndPackages;
 AnOpts.RawSilencedCheckersAndPackages.split(CheckersAndPackages, ";");
 
-for (const StringRef CheckerOrPackage : CheckersAndPackages) {
+for (const StringRef &CheckerOrPackage : CheckersAndPackages) {
   if (Diags) {
 bool IsChecker = CheckerOrPackage.contains('.');
 bool IsValidName =
@@ -607,7 +607,7 @@
XRayInstrSet &S) {
   llvm::SmallVector BundleParts;
   llvm::SplitString(Bundle, BundleParts, ",");
-  for (const auto B : BundleParts) {
+  for (const auto &B : BundleParts) {
 auto Mask = parseXRayInstrValue(B);
 if (Mask == XRayInstrKind::None)
   if (B != "none")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 3853390 - [Frontend] Fixes -Wrange-loop-analysis warnings

2019-12-17 Thread Mark de Wever via cfe-commits

Author: Mark de Wever
Date: 2019-12-17T21:52:47+01:00
New Revision: 385339034f08212d95ade89fdbacb014d86be1e2

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

LOG: [Frontend] Fixes -Wrange-loop-analysis warnings

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 23de18babda1..8631536e8f60 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -334,7 +334,7 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, 
ArgList &Args,
 StringRef CheckerAndPackageList = A->getValue();
 SmallVector CheckersAndPackages;
 CheckerAndPackageList.split(CheckersAndPackages, ",");
-for (const StringRef CheckerOrPackage : CheckersAndPackages)
+for (const StringRef &CheckerOrPackage : CheckersAndPackages)
   Opts.CheckersAndPackages.emplace_back(CheckerOrPackage, IsEnabled);
   }
 
@@ -476,7 +476,7 @@ static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,
 SmallVector CheckersAndPackages;
 AnOpts.RawSilencedCheckersAndPackages.split(CheckersAndPackages, ";");
 
-for (const StringRef CheckerOrPackage : CheckersAndPackages) {
+for (const StringRef &CheckerOrPackage : CheckersAndPackages) {
   if (Diags) {
 bool IsChecker = CheckerOrPackage.contains('.');
 bool IsValidName =
@@ -607,7 +607,7 @@ static void parseXRayInstrumentationBundle(StringRef 
FlagName, StringRef Bundle,
XRayInstrSet &S) {
   llvm::SmallVector BundleParts;
   llvm::SplitString(Bundle, BundleParts, ",");
-  for (const auto B : BundleParts) {
+  for (const auto &B : BundleParts) {
 auto Mask = parseXRayInstrValue(B);
 if (Mask == XRayInstrKind::None)
   if (B != "none")



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


[PATCH] D71585: [perf-training] Change profile file pattern string to use %4m instead of %p

2019-12-17 Thread Alex Langford via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3f789e037cb: [perf-training] Change profile file pattern 
string to use %4m instead of %p (authored by xinxinw1, committed by xiaobai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71585

Files:
  clang/utils/perf-training/lit.cfg


Index: clang/utils/perf-training/lit.cfg
===
--- clang/utils/perf-training/lit.cfg
+++ clang/utils/perf-training/lit.cfg
@@ -37,5 +37,5 @@
 config.substitutions.append( ('%clang', ' %s %s ' % (config.clang, 
sysroot_flags) ) )
 config.substitutions.append( ('%test_root', config.test_exec_root ) )
 
-config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%p.profraw'
+config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%4m.profraw'
 


Index: clang/utils/perf-training/lit.cfg
===
--- clang/utils/perf-training/lit.cfg
+++ clang/utils/perf-training/lit.cfg
@@ -37,5 +37,5 @@
 config.substitutions.append( ('%clang', ' %s %s ' % (config.clang, sysroot_flags) ) )
 config.substitutions.append( ('%test_root', config.test_exec_root ) )
 
-config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%p.profraw'
+config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%4m.profraw'
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

2019-12-17 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

I tested the configurations mentioned at 
https://groups.google.com/d/topic/polly-dev/vxumPMhrSEs/discussion again and 
did not find any issues that were not there before. Therefore IMHO this is good 
to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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


[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

2019-12-17 Thread Philip Reames via Phabricator via cfe-commits
reames added a comment.

Specifically on the revised patch, I remain confused by the need for multiple 
subtypes.  The need for fragments *between* the potentially fused instructions 
doesn't make sense to me.  What I was expecting to see was the following:
BoundaryAlign w/target=the branch fragment
.. some possibly empty sequence of fragments (i.e. the test/cmp/etc..) ...
the branch fragment
a new data fragment if the branch fragment was a DF

(i.e. a single BounaryAlign fragment which aligns a payload which is defined as 
"next fragment to target fragment inclusive".)

To be specific, I'd expect to see the following for an example fused sequence:

1. BoundaryAlign w/Target = 3
2. DataFragment containing TEST RAX, RAX
3. RelaxeableFragment containing JNE symbo

Why do we need anything between the two fragments of the fused pair?

(As a reminder, I am new to this code.  If I'm missing the obvious, please just 
point it out.)


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

https://reviews.llvm.org/D70157



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


[clang] b3f789e - [perf-training] Change profile file pattern string to use %4m instead of %p

2019-12-17 Thread Alex Langford via cfe-commits

Author: Xin-Xin Wang
Date: 2019-12-17T12:12:21-08:00
New Revision: b3f789e037cbfdb1439c01a4eefc9ab9bb0d2c64

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

LOG: [perf-training] Change profile file pattern string to use %4m instead of %p

Summary: With %p, each test file that we're using to generate profile data will 
make its own profraw file which is around 60 MB in size. If we have a lot of 
test files, that quickly uses a lot of space. Use %4m instead to share the 
profraw files used to store the profile data. We use 4 here based on the 
default value in 
https://reviews.llvm.org/source/llvm-github/browse/master/llvm/CMakeLists.txt$604

Reviewers: beanz, phosek, xiaobai, smeenai, vsk

Reviewed By: vsk

Subscribers: vsk, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/utils/perf-training/lit.cfg

Removed: 




diff  --git a/clang/utils/perf-training/lit.cfg 
b/clang/utils/perf-training/lit.cfg
index 67a42345da46..e5b7162e5904 100644
--- a/clang/utils/perf-training/lit.cfg
+++ b/clang/utils/perf-training/lit.cfg
@@ -37,5 +37,5 @@ config.substitutions.append( ('%clang_skip_driver', ' %s %s 
%s ' % (cc1_wrapper,
 config.substitutions.append( ('%clang', ' %s %s ' % (config.clang, 
sysroot_flags) ) )
 config.substitutions.append( ('%test_root', config.test_exec_root ) )
 
-config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%p.profraw'
+config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%4m.profraw'
 



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


[PATCH] D64573: [Syntax] Allow to mutate syntax trees

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60958 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64573



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


[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D70258#1788396 , @jdoerfert wrote:

> In D70258#1788305 , @rjmccall wrote:
>
> > Introducing an IRBuilder-level finalization stack sounds like it's going to 
> > be a huge mess if your goal is to plug this into other frontends.
>
>
> While I get that you don't want to review this, I would really like to 
> understand why you think this would become a mess.


I guess it depends on what you're expecting to be able to achieve with this 
stack.  Frontends have their own notion of what needs to be finalized and what 
can trigger control flow.  If your finalization stack is purely for the 
convenience of your internal IR-generation, it's fine.  If the need for a 
finalizer can cross the emission of arbitrary frontend code, or if your code 
needs to emit branches that might cross arbitrary frontend "scope" boundaries, 
you're going to be in trouble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


[PATCH] D64573: [Syntax] Allow to mutate syntax trees

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:25
+/// Allows to create syntax trees from subtrees not backed by the source code.
+class Factory {
+public:

gribozavr2 wrote:
> Why a class to contain static functions? Is it like a namespace, or is there 
> some future design intent?..
> 
> Or is it for the friendship?
Good point, this pattern is terrible.
It is for friendship, but this is supposed to be an implementation detail, 
therefore shouldn't affect user interface.

I turned these into free-standing functions. Not sure if we should group those 
into a namespace for simpler discoverability via code completion (although one 
can just do `syntax::create^` to get the same effect now) 



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:123
+  unsigned Original : 1;
+  unsigned CanModify : 1;
 };

gribozavr2 wrote:
> Is it worth eagerly computing the "can modify" bit? Mapping expanded tokens 
> to spelled is log(n) after all, and doing it for every syntax node can add up 
> to nontrivial costs...
I would expect this bit to be requested quite often, so I think this would 
actually pay off in the long run. Requesting this bit should be very cheap.

Note that we do quite a bit of `O(log N)` searches in the same code that sets 
`CanModify` bit, so it does not actually make the complexity worse, but 
definitely makes the constant larger.

If this starts being the bottleneck (I bet this will happen eventually), I 
believe we can compute it eagerly with `O(1)` amortized cost. That would 
involve traversing the AST in the **true** source code order, which will 
require tweaking the `RecursiveASTVisitor` and is much easier done when we have 
good test coverage with existing implementation.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:483
+
+syntax::Leaf *syntax::Factory::createPunctuation(syntax::Arena &A,
+ tok::TokenKind K) {

gribozavr2 wrote:
> I can imagine that both building the syntax tree from the AST, and 
> synthesizing syntax trees from thin air will eventually grow pretty long. So 
> I'd like to suggest to split synthesis of syntax trees into a separate file, 
> even though it will be pretty short today.
Good point.
I've moved only the implementation for now, declaration of synthesis functions 
are still in `BuildTree.h` as it seems small enough for now.

I can totally imagine us moving those to a separate file later, though.



Comment at: clang/lib/Tooling/Syntax/ComputeReplacements.cpp:79
+syntax::computeReplacements(const syntax::Arena &A,
+const syntax::TranslationUnit *TU) {
+  auto &Buffer = A.tokenBuffer();

gribozavr2 wrote:
> Why not a reference for the TU?
All accessors on syntax tree return pointers (almost everything can be null), 
so using pointers everywhere just makes the code more uniform.

This one does look a bit different, though, agree that using a reference in 
this public API is probably more natural.



Comment at: clang/lib/Tooling/Syntax/ComputeReplacements.cpp:92
+assert(Buffer.expandedTokens().begin() <= RemovedRange.begin());
+assert(RemovedRange.begin() < Buffer.expandedTokens().end());
+

gribozavr2 wrote:
> This assertion duplicates the one in rangeOfExpanded; do we need to duplicate?
> 
> The code in this function does not depend on the assumption that tokens are 
> expanded tokens; it is rangeOfExpanded that does.
> 
> If you decide to remove the assertion here, please move the comment from here 
> into that function.
Good point, thanks. Moved the comment to `rangeOfExpanded`, we don't need to 
assert in both places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64573



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


[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

2019-12-17 Thread Philip Reames via Phabricator via cfe-commits
reames added a comment.

In D70157#1788025 , @jyknight wrote:

> > .push_align_branch_boundary [N,] [instruction,]*
>
> I'd like to raise again the possibility of using a more general region 
> directive to denote "It is allowable to add prefixes/nops before instructions 
> in this region if the assembler wants to", as I'd started discussing in 
> https://reviews.llvm.org/D71238#1786885 (but let's move the discussion here).


James, I think this proposal is increasing the scope of this proposal too much. 
 It also ignores some of the use cases identified and described in the writeup 
(i.e. the scoped semantics).  I'm open to discussing such a feature more 
generally, but I'd prefer to see a more narrowly focused feature immediately.


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

https://reviews.llvm.org/D70157



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


[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

2019-12-17 Thread Philip Reames via Phabricator via cfe-commits
reames added a comment.

In D70157#1787403 , @annita.zhang 
wrote:

> In D70157#1787160 , @MaskRay wrote:
>
> >
>
>
>
>
> > There is a precedant: .pushsection/.popsection (MCStreamer::SectionStack). 
> > With .push_align_branch/.pop_align_branch, we probably don't need the 
> > 'switch-to-default' action.
> > 
> > I don't know how likely we may ever need nested states (e.g. an `.include` 
> > directive inside an .align_branch region where the included file has own 
> > idea about branch alignment), but .push/.pop does not seem to be more 
> > complex than disable/enable/default.
>
> I rethink about the directives and prefer the .push/.pop pair as @MaskRay 
> suggested. To be specified, I'd suggest to use .push_align_branch_boundary 
> and .pop_align_branch_boundary to align with MC command line options. They 
> will cowork with the command line options and overwrite the options if both 
> are existing.


I agree that we need the push/pop semantics.

> To be clarified, I described the behavior of the directives from my 
> understanding. Feel free to speak if you have difference opinion.
> 
> .push_align_branch_boundary [N,] [instruction,]*
> 
>   This directive specifies the beginning of a region which will overwrite the 
> value set by the command line or by the previous directive. It can represent 
> either an enabling or disabling directive controlled by parameter N. 
>   N indicates to align the branches within N byte boundary. The default value 
> is 32. If N is 0, it means the branch alignment is off within this region. 
>   Instruction specifies types of branches to align. The value is one or 
> multiple values from fused, jcc, jmp, call, ret and indirect. The default 
> value is fused, jcc and jmp. (may change later)

I'd remove the defaults.  Let's just be explicit about what is being 
enabled/disabled.


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

https://reviews.llvm.org/D70157



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


[PATCH] D64573: [Syntax] Allow to mutate syntax trees

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 234359.
ilya-biryukov marked 24 inline comments as done.
ilya-biryukov added a comment.

- Addressed most comments
- Rebased onto HEAD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64573

Files:
  clang/include/clang/Tooling/Syntax/BuildTree.h
  clang/include/clang/Tooling/Syntax/Mutations.h
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/ComputeReplacements.cpp
  clang/lib/Tooling/Syntax/Mutations.cpp
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -9,14 +9,24 @@
 #include "clang/Tooling/Syntax/Tree.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Syntax/BuildTree.h"
+#include "clang/Tooling/Syntax/Mutations.h"
 #include "clang/Tooling/Syntax/Nodes.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -24,6 +34,38 @@
 using namespace clang;
 
 namespace {
+static syntax::Leaf *firstLeaf(syntax::Tree *T) {
+  while (auto *C = T->firstChild()) {
+if (auto *L = dyn_cast(C))
+  return L;
+T = cast(C);
+  }
+  return nullptr;
+}
+
+static syntax::Leaf *lastLeaf(syntax::Tree *T) {
+  while (auto *C = T->firstChild()) {
+// Find the last child.
+while (auto *Next = C->nextSibling())
+  C = Next;
+
+if (auto *L = dyn_cast(C))
+  return L;
+T = cast(C);
+  }
+  return nullptr;
+}
+
+static llvm::ArrayRef tokens(syntax::Node *N) {
+  if (auto *L = dyn_cast(N))
+return llvm::makeArrayRef(L->token(), 1);
+
+  auto *T = dyn_cast(N);
+  syntax::Leaf *First = firstLeaf(T);
+  syntax::Leaf *Last = lastLeaf(T);
+  return llvm::makeArrayRef(First->token(), Last->token() + 1);
+}
+
 class SyntaxTreeTest : public ::testing::Test {
 protected:
   // Build a syntax tree for the code.
@@ -80,13 +122,13 @@
 // Prepare to run a compiler.
 std::vector Args = {"syntax-test", "-std=c++11",
   "-fsyntax-only", FileName};
-auto CI = createInvocationFromCommandLine(Args, Diags, FS);
-assert(CI);
-CI->getFrontendOpts().DisableFree = false;
-CI->getPreprocessorOpts().addRemappedFile(
+Invocation = createInvocationFromCommandLine(Args, Diags, FS);
+assert(Invocation);
+Invocation->getFrontendOpts().DisableFree = false;
+Invocation->getPreprocessorOpts().addRemappedFile(
 FileName, llvm::MemoryBuffer::getMemBufferCopy(Code).release());
 CompilerInstance Compiler;
-Compiler.setInvocation(std::move(CI));
+Compiler.setInvocation(Invocation);
 Compiler.setDiagnostics(Diags.get());
 Compiler.setFileManager(FileMgr.get());
 Compiler.setSourceManager(SourceMgr.get());
@@ -108,6 +150,27 @@
 }
   }
 
+  /// Finds the deepest node in the tree that covers exactly \p R.
+  /// FIXME: implement this efficiently and move to public syntax tree API.
+  syntax::Node *nodeByRange(llvm::Annotations::Range R, syntax::Node *Root) {
+llvm::ArrayRef Toks = tokens(Root);
+
+if (Toks.front().location().isFileID() &&
+Toks.back().location().isFileID() &&
+syntax::Token::range(*SourceMgr, Toks.front(), Toks.back()) ==
+syntax::FileRange(SourceMgr->getMainFileID(), R.Begin, R.End))
+  return Root;
+
+auto *T = dyn_cast(Root);
+if (!T)
+  return nullptr;
+for (auto *C = T->firstChild(); C != nullptr; C = C->nextSibling()) {
+  if (auto *Result = nodeByRange(R, C))
+return Result;
+}
+return nullptr;
+  }
+
   // Data fields.
   llvm::IntrusiveRefCntPtr Diags =
   new DiagnosticsEngine(new DiagnosticIDs, new DiagnosticOptions);
@@ -117,6 +180,7 @@
   new FileManager(FileSystemOptions(), FS);
   llvm::IntrusiveRefCntPtr SourceMgr =
   new SourceManager(*Diags, *FileMgr);
+  std::shared_ptr Invocation;
   // Set after calling buildTree().
   std::unique_ptr Are

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a subscriber: rjmccall.
jdoerfert added a comment.

In D70258#1788305 , @rjmccall wrote:

> I opposed the creation of this library on these terms in the first place, so 
> I'm pretty sure I'm not on the hook to review it.


That's fine with me.

> Introducing an IRBuilder-level finalization stack sounds like it's going to 
> be a huge mess if your goal is to plug this into other frontends.

While I get that you don't want to review this, I would really like to 
understand why you think this would become a mess.

FWIW. The only code that is currently in Clang to make this work is going to be 
removed once the code generation is moved as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


[PATCH] D71612: [analyzer] Add PlacementNewChecker

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:22
+ProgramStateRef State) const;
+  mutable std::unique_ptr BT_Placement;
+};

I think now it is safe to have the bugtype by value and use member 
initialization.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:35
+  //   long *lp = ::new (buf) long;
+  if (const auto *TVRegion = dyn_cast(MRegion))
+// FIXME Handle offset other than 0. E.g.:

Add braces here.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:40
+if (const auto *ERegion = dyn_cast(TVRegion)) {
+  const auto *SRegion = cast(ERegion->getSuperRegion());
+  DefinedOrUnknownSVal Extent = SRegion->getExtent(svalBuilder);

Hmm, I think this logic might need some more testing. Could you add some tests 
with multi dimensional arrays?



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:44
+  // This is modelled by the MallocChecker.
+  const llvm::APSInt *ExtentInt = svalBuilder.getKnownValue(State, Extent);
+  if (!ExtentInt)

This query will only check if you know the exact value of the target of 
placement you.

What you actually care about if the size is at least as big as the placed 
object.

So instead of getting a known value I think it might be a better idea to 
evaluate a less than or equal operator with `evalBinOp`.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:64
+CheckerContext &C, const CXXNewExpr *NE, ProgramStateRef State) const {
+  if (!State)
+return SVal();

When do you see a null state?



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:72
+  } else {
+ElementCount = svalBuilder.makeIntVal(1, true);
+  }

Probably you could just return the size without building a more complex 
symbolic expression in this case? I.e. just put the right value in the 
makeIntVal.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:79
+
+  if (ElementCount.getAs()) {
+// size in Bytes = ElementCount*TypeSize

You could:
```
Optional NL = ElementCount.getAs();
```

And later you could replace the `castAs` with a deref of the optional.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:81
+// size in Bytes = ElementCount*TypeSize
+SVal SizeInBytes = svalBuilder.evalBinOpNN(
+State, BO_Mul, ElementCount.castAs(),

Prefer to use `evalBinOp` over `evalBinOpNN`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71612



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


[PATCH] D71585: [perf-training] Change profile file pattern string to use %4m instead of %p

2019-12-17 Thread Xin-Xin Wang via Phabricator via cfe-commits
xinxinw1 added a comment.

@vsk Thanks! The %c mode looks pretty cool but yeah, I don't really have the 
bandwidth to test it right now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71585



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


[PATCH] D71513: [compiler-rt] [test] Disable MPROTECT on two builtin tests

2019-12-17 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 234351.
mgorny edited the summary of this revision.
mgorny added a comment.

Switch to using `%run_nomprotect` instead of extra `%paxctl`. This should be 
more consistent and should avoid potential issues with `:` on Windows.


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

https://reviews.llvm.org/D71513

Files:
  compiler-rt/test/builtins/Unit/clear_cache_test.c
  compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
  compiler-rt/test/lit.common.cfg.py
  compiler-rt/test/sanitizer_common/netbsd_commands/run_nomprotect.sh


Index: compiler-rt/test/sanitizer_common/netbsd_commands/run_nomprotect.sh
===
--- /dev/null
+++ compiler-rt/test/sanitizer_common/netbsd_commands/run_nomprotect.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+/usr/sbin/paxctl +m "${1}"
+exec "${@}"
Index: compiler-rt/test/lit.common.cfg.py
===
--- compiler-rt/test/lit.common.cfg.py
+++ compiler-rt/test/lit.common.cfg.py
@@ -112,6 +112,19 @@
 (' clang', """\n\n*** Do not use 'clangXXX' in tests,
  instead define '%clangXXX' substitution in lit config. ***\n\n""") )
 
+if config.host_os == 'NetBSD':
+  nb_commands_dir = os.path.join(config.compiler_rt_src_root,
+ "test", "sanitizer_common", "netbsd_commands")
+  config.netbsd_noaslr_prefix = ('sh ' +
+ os.path.join(nb_commands_dir, 
'run_noaslr.sh'))
+  config.netbsd_nomprotect_prefix = ('sh ' +
+ os.path.join(nb_commands_dir,
+  'run_nomprotect.sh'))
+  config.substitutions.append( ('%run_nomprotect',
+config.netbsd_nomprotect_prefix) )
+else:
+  config.substitutions.append( ('%run_nomprotect', '%run') )
+
 # Allow tests to be executed on a simulator or remotely.
 if config.emulator:
   config.substitutions.append( ('%run', config.emulator) )
@@ -498,9 +511,3 @@
 
 config.clang = " " + " ".join(run_wrapper + [config.compile_wrapper, 
config.clang]) + " "
 config.target_cflags = " " + " ".join(target_cflags + extra_cflags) + " "
-
-if config.host_os == 'NetBSD':
-  nb_commands_dir = os.path.join(config.compiler_rt_src_root,
- "test", "sanitizer_common", "netbsd_commands")
-  config.netbsd_noaslr_prefix = ('sh ' +
- os.path.join(nb_commands_dir, 
'run_noaslr.sh'))
Index: compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
===
--- compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
+++ compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
@@ -1,5 +1,5 @@
 // REQUIRES: native-run
-// RUN: %clang_builtins %s %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %run_nomprotect %t
 // REQUIRES: librt_has_enable_execute_stack
 //===-- enable_execute_stack_test.c - Test __enable_execute_stack 
--===//
 //
Index: compiler-rt/test/builtins/Unit/clear_cache_test.c
===
--- compiler-rt/test/builtins/Unit/clear_cache_test.c
+++ compiler-rt/test/builtins/Unit/clear_cache_test.c
@@ -1,6 +1,6 @@
 // REQUIRES: native-run
 // UNSUPPORTED: arm, aarch64
-// RUN: %clang_builtins %s %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %run_nomprotect %t
 // REQUIRES: librt_has_clear_cache
 //===-- clear_cache_test.c - Test clear_cache 
-===//
 //


Index: compiler-rt/test/sanitizer_common/netbsd_commands/run_nomprotect.sh
===
--- /dev/null
+++ compiler-rt/test/sanitizer_common/netbsd_commands/run_nomprotect.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+/usr/sbin/paxctl +m "${1}"
+exec "${@}"
Index: compiler-rt/test/lit.common.cfg.py
===
--- compiler-rt/test/lit.common.cfg.py
+++ compiler-rt/test/lit.common.cfg.py
@@ -112,6 +112,19 @@
 (' clang', """\n\n*** Do not use 'clangXXX' in tests,
  instead define '%clangXXX' substitution in lit config. ***\n\n""") )
 
+if config.host_os == 'NetBSD':
+  nb_commands_dir = os.path.join(config.compiler_rt_src_root,
+ "test", "sanitizer_common", "netbsd_commands")
+  config.netbsd_noaslr_prefix = ('sh ' +
+ os.path.join(nb_commands_dir, 'run_noaslr.sh'))
+  config.netbsd_nomprotect_prefix = ('sh ' +
+ os.path.join(nb_commands_dir,
+  'run_nomprotect.sh'))
+  config.substitutions.append( ('%run_nomprotect',
+config.netbsd_nomprotect_prefix) )
+else:
+  config.substitutions.append( ('%run_nomprotect', '%run') )
+
 # Allow tests to be executed on a simula

[clang] 599d1cc - [Clang FE, SystemZ] Recognize -mpacked-stack CL option

2019-12-17 Thread Jonas Paulsson via cfe-commits

Author: Jonas Paulsson
Date: 2019-12-17T11:26:17-08:00
New Revision: 599d1cc07a51e9a556afa2a995930f7ffe0e42cd

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

LOG: [Clang FE, SystemZ]  Recognize -mpacked-stack CL option

Recognize -mpacked-stack from the command line and add a function attribute
"mpacked-stack" when passed. This is needed for building the Linux kernel.

If this option is passed for any other target than SystemZ, an error is
generated.

Review: Ulrich Weigand
https://reviews.llvm.org/D71441

Added: 
clang/test/CodeGen/packed-stack.c

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index c8d03f2bb2aa..c6700333c13a 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -113,6 +113,7 @@ VALUE_CODEGENOPT(XRayInstructionThreshold , 32, 200)
 CODEGENOPT(InstrumentForProfiling , 1, 0) ///< Set when -pg is enabled.
 CODEGENOPT(CallFEntry , 1, 0) ///< Set when -mfentry is enabled.
 CODEGENOPT(MNopMCount , 1, 0) ///< Set when -mnop-mcount is enabled.
+CODEGENOPT(PackedStack , 1, 0) ///< Set when -mpacked-stack is enabled.
 CODEGENOPT(LessPreciseFPMAD  , 1, 0) ///< Enable less precise MAD instructions 
to
  ///< be generated.
 CODEGENOPT(PrepareForLTO , 1, 0) ///< Set when -flto is enabled on the

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b4ff681c70f8..38504d6330da 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2474,6 +2474,8 @@ def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert 
calls to fentry at functi
   Flags<[CC1Option]>, Group;
 def mnop_mcount : Flag<["-"], "mnop-mcount">, HelpText<"Generate 
mcount/__fentry__ calls as nops. To activate they need to be patched in.">,
   Flags<[CC1Option]>, Group;
+def mpacked_stack : Flag<["-"], "mpacked-stack">, HelpText<"Use packed stack 
layout (SystemZ only).">,
+  Flags<[CC1Option]>, Group;
 def mips16 : Flag<["-"], "mips16">, Group;
 def mno_mips16 : Flag<["-"], "mno-mips16">, Group;
 def mmicromips : Flag<["-"], "mmicromips">, Group;

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index a2d34d41db2e..0db17431814f 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -971,6 +971,14 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
QualType RetTy,
 }
   }
 
+  if (CGM.getCodeGenOpts().PackedStack) {
+if (getContext().getTargetInfo().getTriple().getArch() !=
+llvm::Triple::systemz)
+  CGM.getDiags().Report(diag::err_opt_not_valid_on_target)
+<< "-mpacked-stack";
+Fn->addFnAttr("packed-stack");
+  }
+
   if (RetTy->isVoidType()) {
 // Void type; nothing to return.
 ReturnValue = Address::invalid();

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 5c28a3ab1735..5bf0efcf0503 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5014,6 +5014,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
   if (TC.SupportsProfiling())
 Args.AddLastArg(CmdArgs, options::OPT_mnop_mcount);
 
+  Args.AddLastArg(CmdArgs, options::OPT_mpacked_stack);
+
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
 CmdArgs.push_back("-fapple-kext");

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 852adbbdea58..23de18babda1 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1104,6 +1104,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, 
ArgList &Args, InputKind IK,
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.MNopMCount = Args.hasArg(OPT_mnop_mcount);
+  Opts.PackedStack = Args.hasArg(OPT_mpacked_stack);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
 
   if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {

diff  --git a/clang/test/CodeGen/packed-stack.c 
b/clang/test/CodeGen/packed-stack.c
new file mode 100644
index ..eaf00a7f8b0e
--- /dev/null
+++ b/clang/test/CodeGen/packed-stack.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -mpacked-stack -triple s390x-ibm-linux -emit-llvm \
+// RUN:   -o - %s 2>&1 | FileCheck  %s
+/

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

2019-12-17 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added inline comments.



Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29
+const char *__ubsan::getObjCClassName(ValueHandle Pointer) {
+#if defined(__APPLE__)
+  // We need to query the ObjC runtime for some information, but do not want

delcypher wrote:
> vsk wrote:
> > delcypher wrote:
> > > The compiler-rt codebase tends to use `SANITIZER_MAC` macro (defined to 
> > > be 1 if Apple otherwise it's 0) rather than `__APPLE__`.
> > I see. That seems problematic, as it makes it tricky to add macOS-specific 
> > (or iOS-specific) functionality down the road. I don't think SANITIZER_MAC 
> > should be defined unless TARGET_OS_MACOS is true.
> Sorry I should clarify. `SANITIZER_MAC` is poorly named but it is defined to 
> be `1` for Apple platforms and `0`. I'm just pointing out the convention that 
> exists today. You're absolutely right that we might want to do different 
> things for different Apple platforms but I don't think we want to start doing 
> a mass re-name until arm64e and arm64_32 support are completed landed in 
> llvm's master.
I think 'SANITIZER_MAC' is confusing, and my preference would be to not use it. 
`__APPLE__` seems clearer to me, and (IIUC) the plan is to replace usage of 
'SANITIZER_MAC' with it down the line anyway?


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

https://reviews.llvm.org/D71491



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


[PATCH] D71588: [objc_direct] fix uniquing when re-declaring a readwrite-direct property

2019-12-17 Thread Alex Lorenz via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa4e1819c1683: [objc_direct] fix uniquing when re-declaring a 
readwrite-direct property (authored by MadCoder, committed by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D71588?vs=234215&id=234346#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71588

Files:
  clang/lib/AST/DeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -191,6 +191,14 @@
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
 
+int useFoo(Foo *f) {
+  // CHECK-LABEL: define i32 @useFoo
+  // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo 
getDirect_setDynamic]"
+  [f setGetDynamic_setDirect:1];
+  return [f getDirect_setDynamic];
+}
+
 __attribute__((objc_root_class))
 @interface RootDeclOnly
 @property(direct, readonly) int intProperty;
Index: clang/lib/AST/DeclObjC.cpp
===
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -958,10 +958,18 @@
   auto *CtxD = cast(getDeclContext());
 
   if (auto *ImplD = dyn_cast(CtxD)) {
-if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface())
+if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
   if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
   isInstanceMethod()))
 return MD;
+  // readwrite properties may have been re-declared in an extension.
+  // look harder.
+  if (isPropertyAccessor())
+for (auto *Ext : IFD->known_extensions())
+  if (ObjCMethodDecl *MD =
+  Ext->getMethod(getSelector(), isInstanceMethod()))
+return MD;
+}
   } else if (auto *CImplD = dyn_cast(CtxD)) {
 if (ObjCCategoryDecl *CatD = CImplD->getCategoryDecl())
   if (ObjCMethodDecl *MD = CatD->getMethod(getSelector(),


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -191,6 +191,14 @@
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
 
+int useFoo(Foo *f) {
+  // CHECK-LABEL: define i32 @useFoo
+  // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+  [f setGetDynamic_setDirect:1];
+  return [f getDirect_setDynamic];
+}
+
 __attribute__((objc_root_class))
 @interface RootDeclOnly
 @property(direct, readonly) int intProperty;
Index: clang/lib/AST/DeclObjC.cpp
===
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -958,10 +958,18 @@
   auto *CtxD = cast(getDeclContext());
 
   if (auto *ImplD = dyn_cast(CtxD)) {
-if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface())
+if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
   if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
   isInstanceMethod()))
 return MD;
+  // readwrite properties may have been re-declared in an extension.
+  // look harder.
+  if (isPropertyAccessor())
+for (auto *Ext : IFD->known_extensions())
+  if (ObjCMethodDecl *MD =
+  Ext->getMethod(getSelector(), isInstanceMethod()))
+return MD;
+}
   } else if (auto *CImplD = dyn_cast(CtxD)) {
 if (ObjCCategoryDecl *CatD = CImplD->getCategoryDecl())
   if (ObjCMethodDecl *MD = CatD->getMethod(getSelector(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] a4e1819 - [objc_direct] fix uniquing when re-declaring a readwrite-direct property

2019-12-17 Thread Alex Lorenz via cfe-commits

Author: Pierre Habouzit
Date: 2019-12-17T11:07:48-08:00
New Revision: a4e1819c16836dba928e646024a2406bb2eb3f94

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

LOG: [objc_direct] fix uniquing when re-declaring a readwrite-direct property

ObjCMethodDecl::getCanonicalDecl() for re-declared readwrite properties,
only looks in the ObjCInterface for the declaration of the setter
method, which it won't find.

When the method is a property accessor, we must look in extensions for a
possible redeclaration.

Radar-Id: rdar://problem/57991337

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

Added: 


Modified: 
clang/lib/AST/DeclObjC.cpp
clang/test/CodeGenObjC/direct-method.m

Removed: 




diff  --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp
index fed76aecdaa2..ca70afd9db25 100644
--- a/clang/lib/AST/DeclObjC.cpp
+++ b/clang/lib/AST/DeclObjC.cpp
@@ -958,10 +958,18 @@ ObjCMethodDecl *ObjCMethodDecl::getCanonicalDecl() {
   auto *CtxD = cast(getDeclContext());
 
   if (auto *ImplD = dyn_cast(CtxD)) {
-if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface())
+if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
   if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
   isInstanceMethod()))
 return MD;
+  // readwrite properties may have been re-declared in an extension.
+  // look harder.
+  if (isPropertyAccessor())
+for (auto *Ext : IFD->known_extensions())
+  if (ObjCMethodDecl *MD =
+  Ext->getMethod(getSelector(), isInstanceMethod()))
+return MD;
+}
   } else if (auto *CImplD = dyn_cast(CtxD)) {
 if (ObjCCategoryDecl *CatD = CImplD->getCategoryDecl())
   if (ObjCMethodDecl *MD = CatD->getMethod(getSelector(),

diff  --git a/clang/test/CodeGenObjC/direct-method.m 
b/clang/test/CodeGenObjC/direct-method.m
index 373bd22a84cd..c42910c1676a 100644
--- a/clang/test/CodeGenObjC/direct-method.m
+++ b/clang/test/CodeGenObjC/direct-method.m
@@ -191,6 +191,14 @@ int useRoot(Root *r) {
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
 
+int useFoo(Foo *f) {
+  // CHECK-LABEL: define i32 @useFoo
+  // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo 
getDirect_setDynamic]"
+  [f setGetDynamic_setDirect:1];
+  return [f getDirect_setDynamic];
+}
+
 __attribute__((objc_root_class))
 @interface RootDeclOnly
 @property(direct, readonly) int intProperty;



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


[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

2019-12-17 Thread John McCall via Phabricator via cfe-commits
rjmccall resigned from this revision.
rjmccall added a comment.

I opposed the creation of this library on these terms in the first place, so 
I'm pretty sure I'm not on the hook to review it.  Introducing an 
IRBuilder-level finalization stack sounds like it's going to be a huge mess if 
your goal is to plug this into other frontends.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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


[PATCH] D71588: [objc_direct] fix uniquing when re-declaring a readwrite-direct property

2019-12-17 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

I couldn't come up with a reasonable Sema-based test, so I'm going to commit 
this as it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71588



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


  1   2   3   >