[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-06 Thread Bjorn Pettersson via Phabricator via cfe-commits
bjope added inline comments.



Comment at: clang/test/Sema/warn-bad-function-cast.c:49
+#ifdef FIXED_POINT
+  (void)(_Fract) if1(); // no warning
+#endif

bjope wrote:
> vabridgers wrote:
> > bjope wrote:
> > > bjope wrote:
> > > > bjope wrote:
> > > > > This should be added before the line saying `/* All following casts 
> > > > > issue warning */`.
> > > > Is the `(void)` needed/relevant here?
> > > As questioned earlier, shouldn't we expect a warning for this scenario?
> > > 
> > > There is however a problem that we get the warning for _Fract to _Fract 
> > > conversion. And it would be nice with a more complete set of tests 
> > > involving both FixedPoint->FixedPoint, FixedPoint->Integer and 
> > > Integer->FixedPoint casts.
> > If you have any *specific* suggestions for test cases, I'm open to that.
> Add:
> 
> ```
> _Fract ff1(void);
> ```
> 
> And inside foo add these three tests (you'll need to add appropriate expects):
> ```
> (_Fract)ff1();  // No warning expected.
> (_Fract)if1();  // Warning expected.
> (int)ff1();  // Warning expected.
> ```
I still think it would be nice not to break the structure of this test. Tests 
seem to be divided into three categories:

/* Casts to void types are always OK.  */

  /* Casts to the same type or similar types are OK.  */

/* All following casts issue warning */

And you have currently inserted all new tests in the last section.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85157

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


[PATCH] D85473: [Clang] Add option to allow marking pass-by-value args as noalias.

2020-08-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:371-374
+LANGOPT(PassByValueNoAlias, 1, 0, "Allows assuming no references to passed by "
+  "value escape before transferring execution "
+  "to the called function. Note that this does 
"
+  "not hold for C++")

Per the comment at the top of the file, the description should be a single noun 
phrase; we use this in diagnostics such as "AST file was built with %0 enabled 
but it is currently disabled". Something like "assumption that by-value 
parameters do not alias any other values" maybe?



Comment at: clang/include/clang/Driver/Options.td:4287-4290
+def fpass_by_value_noalias: Flag<["-"], "fpass-by-value-noalias">,
+  HelpText<"Allows assuming no references to passed by value escape before "
+   "transferring execution to the called function. Note that this "
+   "does not hold for C++">;

This should be in `Group`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85473

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


[clang] cce1b0e - [PowerPC] Implement Vector Extract Low/High Order Builtins in LLVM/Clang

2020-08-06 Thread via cfe-commits

Author: biplmish
Date: 2020-08-07T01:02:29-05:00
New Revision: cce1b0e8919e9b0036f5debe60865575520df1c7

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

LOG: [PowerPC] Implement Vector Extract Low/High Order Builtins in LLVM/Clang

This patch implements the function prototypes vec_extractl and vec_extracth in 
altivec.h to utilize the vector extract double element instructions introduced 
in Power10.

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsPPC.def
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-p10vector.c
llvm/include/llvm/IR/IntrinsicsPowerPC.td
llvm/lib/Target/PowerPC/PPCInstrPrefix.td
llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsPPC.def 
b/clang/include/clang/Basic/BuiltinsPPC.def
index 5aed03f74f65..b79ed41284ac 100644
--- a/clang/include/clang/Basic/BuiltinsPPC.def
+++ b/clang/include/clang/Basic/BuiltinsPPC.def
@@ -336,6 +336,16 @@ BUILTIN(__builtin_altivec_vinshvrx, "V8UsV8UsUiV8Us", "")
 BUILTIN(__builtin_altivec_vinswvlx, "V4UiV4UiUiV4Ui", "")
 BUILTIN(__builtin_altivec_vinswvrx, "V4UiV4UiUiV4Ui", "")
 
+// P10 Vector Extract built-ins.
+BUILTIN(__builtin_altivec_vextdubvlx, "V2ULLiV16UcV16UcUi", "")
+BUILTIN(__builtin_altivec_vextdubvrx, "V2ULLiV16UcV16UcUi", "")
+BUILTIN(__builtin_altivec_vextduhvlx, "V2ULLiV8UsV8UsUi", "")
+BUILTIN(__builtin_altivec_vextduhvrx, "V2ULLiV8UsV8UsUi", "")
+BUILTIN(__builtin_altivec_vextduwvlx, "V2ULLiV4UiV4UiUi", "")
+BUILTIN(__builtin_altivec_vextduwvrx, "V2ULLiV4UiV4UiUi", "")
+BUILTIN(__builtin_altivec_vextddvlx, "V2ULLiV2ULLiV2ULLiUi", "")
+BUILTIN(__builtin_altivec_vextddvrx, "V2ULLiV2ULLiV2ULLiUi", "")
+
 // VSX built-ins.
 
 BUILTIN(__builtin_vsx_lxvd2x, "V2divC*", "")

diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index f42200f5bd4e..d0a3b198351c 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -17101,6 +17101,92 @@ vec_inserth(vector unsigned int __a, vector unsigned 
int __b,
 #endif
 }
 
+/* vec_extractl */
+
+static __inline__ vector unsigned long long __ATTRS_o_ai vec_extractl(
+vector unsigned char __a, vector unsigned char __b, unsigned int __c) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vextdubvrx(__a, __b, __c);
+#else
+  vector unsigned long long __ret = __builtin_altivec_vextdubvlx(__a, __b, 
__c);
+  return vec_sld(__ret, __ret, 8);
+#endif
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai vec_extractl(
+vector unsigned short __a, vector unsigned short __b, unsigned int __c) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vextduhvrx(__a, __b, __c);
+#else
+  vector unsigned long long __ret = __builtin_altivec_vextduhvlx(__a, __b, 
__c);
+  return vec_sld(__ret, __ret, 8);
+#endif
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai vec_extractl(
+vector unsigned int __a, vector unsigned int __b, unsigned int __c) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vextduwvrx(__a, __b, __c);
+#else
+  vector unsigned long long __ret = __builtin_altivec_vextduwvlx(__a, __b, 
__c);
+  return vec_sld(__ret, __ret, 8);
+#endif
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai
+vec_extractl(vector unsigned long long __a, vector unsigned long long __b,
+ unsigned int __c) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vextddvrx(__a, __b, __c);
+#else
+  vector unsigned long long __ret = __builtin_altivec_vextddvlx(__a, __b, __c);
+  return vec_sld(__ret, __ret, 8);
+#endif
+}
+
+/* vec_extracth */
+
+static __inline__ vector unsigned long long __ATTRS_o_ai vec_extracth(
+vector unsigned char __a, vector unsigned char __b, unsigned int __c) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vextdubvlx(__a, __b, __c);
+#else
+  vector unsigned long long __ret = __builtin_altivec_vextdubvrx(__a, __b, 
__c);
+  return vec_sld(__ret, __ret, 8);
+#endif
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai vec_extracth(
+vector unsigned short __a, vector unsigned short __b, unsigned int __c) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vextduhvlx(__a, __b, __c);
+#else
+  vector unsigned long long __ret = __builtin_altivec_vextduhvrx(__a, __b, 
__c);
+  return vec_sld(__ret, __ret, 8);
+#endif
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai vec_extracth(
+vector unsigned int __a, vector unsigned int __b, unsigned int __c) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vextduwvlx(__a, __b, __c);
+#else
+  vector unsigned long long __ret = __builtin_altivec_vextduwvrx(__a, __b, 
__c);
+  return vec_sld(__ret, __ret, 8);
+#endif
+}
+
+static __inline__ 

[PATCH] D84622: [PowerPC] Implement Vector Extract Low/High Order Builtins in LLVM/Clang

2020-08-06 Thread Biplob Mishra via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcce1b0e8919e: [PowerPC] Implement Vector Extract Low/High 
Order Builtins in LLVM/Clang (authored by biplmish).

Changed prior to commit:
  https://reviews.llvm.org/D84622?vs=280803&id=283814#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84622

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll
@@ -253,3 +253,91 @@
   ret <2 x i64> %0
 }
 declare <2 x i64> @llvm.ppc.altivec.vinsd(<2 x i64>, i64, i32 immarg)
+
+define <2 x i64> @testVEXTDUBVLX(<16 x i8> %a, <16 x i8> %b, i32 %c) {
+; CHECK-LABEL: testVEXTDUBVLX:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vextdubvlx v2, v2, v3, r7
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call <2 x i64> @llvm.ppc.altivec.vextdubvlx(<16 x i8> %a, <16 x i8> %b, i32 %c)
+  ret <2 x i64> %0
+}
+declare <2 x i64> @llvm.ppc.altivec.vextdubvlx(<16 x i8>, <16 x i8>, i32)
+
+define <2 x i64> @testVEXTDUBVRX(<16 x i8> %a, <16 x i8> %b, i32 %c) {
+; CHECK-LABEL: testVEXTDUBVRX:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vextdubvrx v2, v2, v3, r7
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call <2 x i64> @llvm.ppc.altivec.vextdubvrx(<16 x i8> %a, <16 x i8> %b, i32 %c)
+  ret <2 x i64> %0
+}
+declare <2 x i64> @llvm.ppc.altivec.vextdubvrx(<16 x i8>, <16 x i8>, i32)
+
+define <2 x i64> @testVEXTDUHVLX(<8 x i16> %a, <8 x i16> %b, i32 %c) {
+; CHECK-LABEL: testVEXTDUHVLX:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vextduhvlx v2, v2, v3, r7
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call <2 x i64> @llvm.ppc.altivec.vextduhvlx(<8 x i16> %a, <8 x i16> %b, i32 %c)
+  ret <2 x i64> %0
+}
+declare <2 x i64> @llvm.ppc.altivec.vextduhvlx(<8 x i16>, <8 x i16>, i32)
+
+define <2 x i64> @testVEXTDUHVRX(<8 x i16> %a, <8 x i16> %b, i32 %c) {
+; CHECK-LABEL: testVEXTDUHVRX:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vextduhvrx v2, v2, v3, r7
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call <2 x i64> @llvm.ppc.altivec.vextduhvrx(<8 x i16> %a, <8 x i16> %b, i32 %c)
+  ret <2 x i64> %0
+}
+declare <2 x i64> @llvm.ppc.altivec.vextduhvrx(<8 x i16>, <8 x i16>, i32)
+
+define <2 x i64> @testVEXTDUWVLX(<4 x i32> %a, <4 x i32> %b, i32 %c) {
+; CHECK-LABEL: testVEXTDUWVLX:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vextduwvlx v2, v2, v3, r7
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call <2 x i64> @llvm.ppc.altivec.vextduwvlx(<4 x i32> %a, <4 x i32> %b, i32 %c)
+  ret <2 x i64> %0
+}
+declare <2 x i64> @llvm.ppc.altivec.vextduwvlx(<4 x i32>, <4 x i32>, i32)
+
+define <2 x i64> @testVEXTDUWVRX(<4 x i32> %a, <4 x i32> %b, i32 %c) {
+; CHECK-LABEL: testVEXTDUWVRX:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vextduwvrx v2, v2, v3, r7
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call <2 x i64> @llvm.ppc.altivec.vextduwvrx(<4 x i32> %a, <4 x i32> %b, i32 %c)
+  ret <2 x i64> %0
+}
+declare <2 x i64> @llvm.ppc.altivec.vextduwvrx(<4 x i32>, <4 x i32>, i32)
+
+define <2 x i64> @testVEXTDDVLX(<2 x i64> %a, <2 x i64> %b, i32 %c) {
+; CHECK-LABEL: testVEXTDDVLX:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vextddvlx v2, v2, v3, r7
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call <2 x i64> @llvm.ppc.altivec.vextddvlx(<2 x i64> %a, <2 x i64> %b, i32 %c)
+  ret <2 x i64> %0
+}
+declare <2 x i64> @llvm.ppc.altivec.vextddvlx(<2 x i64>, <2 x i64>, i32)
+
+define <2 x i64> @testVEXTDDVRX(<2 x i64> %a, <2 x i64> %b, i32 %c) {
+; CHECK-LABEL: testVEXTDDVRX:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vextddvrx v2, v2, v3, r7
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call <2 x i64> @llvm.ppc.altivec.vextddvrx(<2 x i64> %a, <2 x i64> %b, i32 %c)
+  ret <2 x i64> %0
+}
+declare <2 x i64> @llvm.ppc.altivec.vextddvrx(<2 x i64>, <2 x i64>, i32)
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -948,37 +948,69 @@
 (int_ppc_altivec_vinsdrx v2i64:$vDi, i64:$rA, i64:$rB))]>,
   RegConstraint<"$vDi = $vD">, NoEncode<"$vDi">;
   def VEXTDUBVLX : VAForm_1a<24, (outs vrrc:$vD),
- (ins vrrc:$vA, vrrc:$vB, g8rc:$rC),
+ (ins vrrc:$vA, vrrc:$vB, gprc:$rC),
  "vextdubvlx $vD, $vA, $vB, $rC",
- IIC_VecG

[PATCH] D85453: [PowerPC] Implement __int128 vector divide operations

2020-08-06 Thread Amy Kwan via Phabricator via cfe-commits
amyk accepted this revision as: amyk.
amyk added a comment.
This revision is now accepted and ready to land.

Overall LGTM.




Comment at: llvm/test/CodeGen/PowerPC/p10-vector-divide.ll:53
+
+define <1 x i128> @test_vdivsq(<1 x i128> %x, <1 x i128> %y) nounwind readnone 
{
+; CHECK-LABEL: test_vdivsq:

nit: I think `nounwind readnone` can probably be omitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85453

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


[PATCH] D83338: [PowerPC][Power10] Implemented Vector Shift Builtins

2020-08-06 Thread Amy Kwan via Phabricator via cfe-commits
amyk added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1144
+(v1i128 (VSLQ v1i128:$VRA, v1i128:$VRB))>;
+  def : Pat<(v1i128 (PPCshl v1i128:$VRA, v1i128:$VRB)),
+(v1i128 (VSLQ v1i128:$VRA, v1i128:$VRB))>;

amyk wrote:
> I noticed that we have patterns for the PPCISD nodes, but I think no tests to 
> show these patterns?
My apologies, upon closer inspection I noticed that your tests are in fact the 
ones with the PPCISD nodes but you are missing the regular shl/srl/sra. Please 
add the tests for these, as well. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83338

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


[PATCH] D85500: [clangd] Highlight structured bindings at local scope as LocalVariable

2020-08-06 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85500

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -515,13 +515,14 @@
 $Class[[S]] *$Field[[Next]];
   };
   $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
   void $Function[[f]]($Class[[S]] $Parameter[[P]]) {
 int $LocalVariable[[A]][2] = {1,2};
-auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
-auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
-$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+auto [$LocalVariable[[B1]], $LocalVariable[[B2]]] = 
$LocalVariable[[A]];
+auto [$LocalVariable[[G1]], $LocalVariable[[G2]]] = 
$Variable[[Global]];
+$Class[[auto]] [$LocalVariable[[P1]], $LocalVariable[[P2]]] = 
$Parameter[[P]];
 // Highlights references to BindingDecls.
-$Variable[[B1]]++;
+$LocalVariable[[B1]]++;
   }
 )cpp",
   R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -90,8 +90,10 @@
? HighlightingKind::StaticField
: VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
   : HighlightingKind::Variable;
-  if (isa(D))
-return HighlightingKind::Variable;
+  if (auto *BD = dyn_cast(D))
+return BD->getDeclContext()->isFunctionOrMethod()
+   ? HighlightingKind::LocalVariable
+   : HighlightingKind::Variable;
   if (isa(D))
 return HighlightingKind::Function;
   if (isa(D) || isa(D) ||


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -515,13 +515,14 @@
 $Class[[S]] *$Field[[Next]];
   };
   $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
   void $Function[[f]]($Class[[S]] $Parameter[[P]]) {
 int $LocalVariable[[A]][2] = {1,2};
-auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
-auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
-$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+auto [$LocalVariable[[B1]], $LocalVariable[[B2]]] = $LocalVariable[[A]];
+auto [$LocalVariable[[G1]], $LocalVariable[[G2]]] = $Variable[[Global]];
+$Class[[auto]] [$LocalVariable[[P1]], $LocalVariable[[P2]]] = $Parameter[[P]];
 // Highlights references to BindingDecls.
-$Variable[[B1]]++;
+$LocalVariable[[B1]]++;
   }
 )cpp",
   R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -90,8 +90,10 @@
? HighlightingKind::StaticField
: VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
   : HighlightingKind::Variable;
-  if (isa(D))
-return HighlightingKind::Variable;
+  if (auto *BD = dyn_cast(D))
+return BD->getDeclContext()->isFunctionOrMethod()
+   ? HighlightingKind::LocalVariable
+   : HighlightingKind::Variable;
   if (isa(D))
 return HighlightingKind::Function;
   if (isa(D) || isa(D) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

2020-08-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

The file is auto-generated by:
`path/to/clang-tblgen -gen-opt-docs -I clang/include/clang/Driver -I 
llvm/include/ clang/include/clang/Driver/ClangOptionDocs.td > 
clang/docs/ClangCommandLineReference.rst`

The description is retrieved from `DocBrief`. It `DocBrief` does not exist, 
`HelpText` provides a fallback. I think there needs to be a better way to 
organize detailed help messages. Inlined help is just too cumbersome. For 
attributes, there is a TableGen file `clang/include/clang/Basic/AttrDocs.td`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85239

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


[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

2020-08-06 Thread Richard Trieu via Phabricator via cfe-commits
rtrieu added a comment.

In D85287#2199490 , @sberg wrote:

> In D85287#2199463 , @rtrieu wrote:
>
>> Also, have you tried running warning over a codebase?
>
> As I wrote: "At least building LibreOffice with this change caused no false 
> positives."  (Or maybe I misunderstand your question.)

My bad.  I read too fast and thought you said you ran the scan over LibreOffice 
instead of your warning.


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

https://reviews.llvm.org/D85287

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


[clang-tools-extra] b284767 - Reinstate check that we don't crash.

2020-08-06 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-08-06T19:07:50-07:00
New Revision: b2847671b83f6acb71a78d4e37bd57967c858f4e

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

LOG: Reinstate check that we don't crash.

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
index 46e84aa7614f..075c3aca9d03 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
@@ -794,16 +794,16 @@ struct Bar {
   }
 };
 
-// FIXME: It's not clear that we should be diagnosing this. The `&&` operator
-// here is unresolved and could resolve to an overloaded operator that might
-// have side-effects on its operands. For other constructs with the same
-// property (eg, the `S2` cases above) we suppress this diagnostic. This
-// started failing when Clang started properly modeling the fold-expression as
-// containing an unresolved operator name.
-//template 
-//struct Bar2 {
-//  static_assert((... && (sizeof(Values) > 0)) == (... && (sizeof(Values) > 
0)));
-//  // -MESSAGES: :[[@LINE-1]]:47: warning: both sides of operator are 
equivalent [misc-redundant-expression]
-//};
+template 
+struct Bar2 {
+  static_assert((... && (sizeof(Values) > 0)) == (... && (sizeof(Values) > 
0)));
+  // FIXME: It's not clear that we should be diagnosing this. The `&&` operator
+  // here is unresolved and could resolve to an overloaded operator that might
+  // have side-effects on its operands. For other constructs with the same
+  // property (eg, the `S2` cases above) we suppress this diagnostic. This
+  // started failing when Clang started properly modeling the fold-expression 
as
+  // containing an unresolved operator name.
+  // FIXME-MESSAGES: :[[@LINE-1]]:47: warning: both sides of operator are 
equivalent [misc-redundant-expression]
+};
 
 } // namespace no_crash



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


[clang-tools-extra] 2f1fffa - Disable clang-tidy test that started failing after clang commit ed5a18f.

2020-08-06 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-08-06T19:06:06-07:00
New Revision: 2f1fffab73f83e6a729cb4d68a99f930e44bc7a8

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

LOG: Disable clang-tidy test that started failing after clang commit ed5a18f.

This checker appears to be intentionally not diagnosing cases where an
operator appearing in a duplicated expression might have side-effects;
Clang is now modeling fold-expressions as having an unresolved operator
name within them, so they now trip up this check.

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
index c47ef7d36ff5..46e84aa7614f 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
@@ -794,9 +794,16 @@ struct Bar {
   }
 };
 
-template 
-struct Bar2 {
-  static_assert((... && (sizeof(Values) > 0)) == (... && (sizeof(Values) > 
0)));
-  // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: both sides of operator are 
equivalent [misc-redundant-expression]
-};
+// FIXME: It's not clear that we should be diagnosing this. The `&&` operator
+// here is unresolved and could resolve to an overloaded operator that might
+// have side-effects on its operands. For other constructs with the same
+// property (eg, the `S2` cases above) we suppress this diagnostic. This
+// started failing when Clang started properly modeling the fold-expression as
+// containing an unresolved operator name.
+//template 
+//struct Bar2 {
+//  static_assert((... && (sizeof(Values) > 0)) == (... && (sizeof(Values) > 
0)));
+//  // -MESSAGES: :[[@LINE-1]]:47: warning: both sides of operator are 
equivalent [misc-redundant-expression]
+//};
+
 } // namespace no_crash



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


[PATCH] D85272: [clangd] Semantic highlighting for dependent template name in template argument

2020-08-06 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf4ba7a100a56: [clangd] Semantic highlighting for dependent 
template name in template argument (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85272

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -686,6 +686,14 @@
   void $Function[[bar]]($TemplateParameter[[T]] $Parameter[[F]]) {
 $Parameter[[F]].$DependentName[[foo]]();
   }
+)cpp",
+  // Dependent template name
+  R"cpp(
+  template  class> struct $Class[[A]] {};
+  template 
+  using $Typedef[[W]] = $Class[[A]]<
+$TemplateParameter[[T]]::template $DependentType[[Waldo]]
+  >;
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -296,6 +296,18 @@
 return true;
   }
 
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) {
+switch (L.getArgument().getKind()) {
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+  H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+  break;
+default:
+  break;
+}
+return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
+  }
+
   // findExplicitReferences will walk nested-name-specifiers and
   // find anything that can be resolved to a Decl. However, non-leaf
   // components of nested-name-specifiers which are dependent names


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -686,6 +686,14 @@
   void $Function[[bar]]($TemplateParameter[[T]] $Parameter[[F]]) {
 $Parameter[[F]].$DependentName[[foo]]();
   }
+)cpp",
+  // Dependent template name
+  R"cpp(
+  template  class> struct $Class[[A]] {};
+  template 
+  using $Typedef[[W]] = $Class[[A]]<
+$TemplateParameter[[T]]::template $DependentType[[Waldo]]
+  >;
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -296,6 +296,18 @@
 return true;
   }
 
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) {
+switch (L.getArgument().getKind()) {
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+  H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+  break;
+default:
+  break;
+}
+return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
+  }
+
   // findExplicitReferences will walk nested-name-specifiers and
   // find anything that can be resolved to a Decl. However, non-leaf
   // components of nested-name-specifiers which are dependent names
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] f4ba7a1 - [clangd] Semantic highlighting for dependent template name in template argument

2020-08-06 Thread Nathan Ridge via cfe-commits

Author: Nathan Ridge
Date: 2020-08-06T21:23:49-04:00
New Revision: f4ba7a100a56b63f9b3eac709ecea9f514d90c00

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

LOG: [clangd] Semantic highlighting for dependent template name in template 
argument

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

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

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index ed75ce80999c..4bedea457e5c 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -296,6 +296,18 @@ class CollectExtraHighlightings
 return true;
   }
 
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) {
+switch (L.getArgument().getKind()) {
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+  H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+  break;
+default:
+  break;
+}
+return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
+  }
+
   // findExplicitReferences will walk nested-name-specifiers and
   // find anything that can be resolved to a Decl. However, non-leaf
   // components of nested-name-specifiers which are dependent names

diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp 
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 453986552ac5..1b90b21b2c58 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -686,6 +686,14 @@ sizeof...($TemplateParameter[[Elements]]);
   void $Function[[bar]]($TemplateParameter[[T]] $Parameter[[F]]) {
 $Parameter[[F]].$DependentName[[foo]]();
   }
+)cpp",
+  // Dependent template name
+  R"cpp(
+  template  class> struct $Class[[A]] {};
+  template 
+  using $Typedef[[W]] = $Class[[A]]<
+$TemplateParameter[[T]]::template $DependentType[[Waldo]]
+  >;
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);



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


[PATCH] D85272: [clangd] Semantic highlighting for dependent template name in template argument

2020-08-06 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/SemanticHighlighting.cpp:305
+  break;
+default:;
+}

nridge wrote:
> hokein wrote:
> > nit: move the trailing `;` to a new line.
> I have done this, but clang-format moves it back... do we really want to 
> fight wit it?
I wrote it as:

```
  default:
break;
```

instead, I think that may be more idiomatic anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85272

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


[PATCH] D85272: [clangd] Semantic highlighting for dependent template name in template argument

2020-08-06 Thread Nathan Ridge via Phabricator via cfe-commits
nridge updated this revision to Diff 283784.
nridge added a comment.

Address nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85272

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -686,6 +686,14 @@
   void $Function[[bar]]($TemplateParameter[[T]] $Parameter[[F]]) {
 $Parameter[[F]].$DependentName[[foo]]();
   }
+)cpp",
+  // Dependent template name
+  R"cpp(
+  template  class> struct $Class[[A]] {};
+  template 
+  using $Typedef[[W]] = $Class[[A]]<
+$TemplateParameter[[T]]::template $DependentType[[Waldo]]
+  >;
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -296,6 +296,18 @@
 return true;
   }
 
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) {
+switch (L.getArgument().getKind()) {
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+  H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+  break;
+default:
+  break;
+}
+return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
+  }
+
   // findExplicitReferences will walk nested-name-specifiers and
   // find anything that can be resolved to a Decl. However, non-leaf
   // components of nested-name-specifiers which are dependent names


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -686,6 +686,14 @@
   void $Function[[bar]]($TemplateParameter[[T]] $Parameter[[F]]) {
 $Parameter[[F]].$DependentName[[foo]]();
   }
+)cpp",
+  // Dependent template name
+  R"cpp(
+  template  class> struct $Class[[A]] {};
+  template 
+  using $Typedef[[W]] = $Class[[A]]<
+$TemplateParameter[[T]]::template $DependentType[[Waldo]]
+  >;
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -296,6 +296,18 @@
 return true;
   }
 
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) {
+switch (L.getArgument().getKind()) {
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+  H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+  break;
+default:
+  break;
+}
+return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
+  }
+
   // findExplicitReferences will walk nested-name-specifiers and
   // find anything that can be resolved to a Decl. However, non-leaf
   // components of nested-name-specifiers which are dependent names
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang] ed5a18f - PR30738: Implement two-phase name lookup for fold-expressions.

2020-08-06 Thread Nico Weber via cfe-commits
Hi,

it looks like this broke check-clang-tools, see e.g.
http://45.33.8.238/linux/24975/step_8.txt

Nico

On Thu, Aug 6, 2020 at 7:57 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Richard Smith
> Date: 2020-08-06T16:56:39-07:00
> New Revision: ed5a18fc0399dce994aa354a33e6f981f9828647
>
> URL:
> https://github.com/llvm/llvm-project/commit/ed5a18fc0399dce994aa354a33e6f981f9828647
> DIFF:
> https://github.com/llvm/llvm-project/commit/ed5a18fc0399dce994aa354a33e6f981f9828647.diff
>
> LOG: PR30738: Implement two-phase name lookup for fold-expressions.
>
> Added:
>
>
> Modified:
> clang/include/clang/AST/ExprCXX.h
> clang/include/clang/Sema/Sema.h
> clang/lib/AST/ASTContext.cpp
> clang/lib/Parse/ParseExpr.cpp
> clang/lib/Sema/SemaDeclCXX.cpp
> clang/lib/Sema/SemaExpr.cpp
> clang/lib/Sema/SemaLookup.cpp
> clang/lib/Sema/SemaOverload.cpp
> clang/lib/Sema/SemaTemplate.cpp
> clang/lib/Sema/SemaTemplateVariadic.cpp
> clang/lib/Sema/TreeTransform.h
> clang/lib/Serialization/ASTReaderStmt.cpp
> clang/lib/Serialization/ASTWriterStmt.cpp
> clang/test/AST/ast-dump-expr-json.cpp
> clang/test/AST/ast-dump-expr.cpp
> clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/include/clang/AST/ExprCXX.h
> b/clang/include/clang/AST/ExprCXX.h
> index 3f272c96a2d8..b53bb20f7ebc 100644
> --- a/clang/include/clang/AST/ExprCXX.h
> +++ b/clang/include/clang/AST/ExprCXX.h
> @@ -4519,31 +4519,38 @@ class CXXFoldExpr : public Expr {
>friend class ASTStmtReader;
>friend class ASTStmtWriter;
>
> +  enum SubExpr { Callee, LHS, RHS, Count };
> +
>SourceLocation LParenLoc;
>SourceLocation EllipsisLoc;
>SourceLocation RParenLoc;
>// When 0, the number of expansions is not known. Otherwise, this is
> one more
>// than the number of expansions.
>unsigned NumExpansions;
> -  Stmt *SubExprs[2];
> +  Stmt *SubExprs[SubExpr::Count];
>BinaryOperatorKind Opcode;
>
>  public:
> -  CXXFoldExpr(QualType T, SourceLocation LParenLoc, Expr *LHS,
> -  BinaryOperatorKind Opcode, SourceLocation EllipsisLoc, Expr
> *RHS,
> -  SourceLocation RParenLoc, Optional NumExpansions)
> +  CXXFoldExpr(QualType T, UnresolvedLookupExpr *Callee,
> +  SourceLocation LParenLoc, Expr *LHS, BinaryOperatorKind
> Opcode,
> +  SourceLocation EllipsisLoc, Expr *RHS, SourceLocation
> RParenLoc,
> +  Optional NumExpansions)
>: Expr(CXXFoldExprClass, T, VK_RValue, OK_Ordinary),
> LParenLoc(LParenLoc),
>  EllipsisLoc(EllipsisLoc), RParenLoc(RParenLoc),
>  NumExpansions(NumExpansions ? *NumExpansions + 1 : 0),
> Opcode(Opcode) {
> -SubExprs[0] = LHS;
> -SubExprs[1] = RHS;
> +SubExprs[SubExpr::Callee] = Callee;
> +SubExprs[SubExpr::LHS] = LHS;
> +SubExprs[SubExpr::RHS] = RHS;
>  setDependence(computeDependence(this));
>}
>
>CXXFoldExpr(EmptyShell Empty) : Expr(CXXFoldExprClass, Empty) {}
>
> -  Expr *getLHS() const { return static_cast(SubExprs[0]); }
> -  Expr *getRHS() const { return static_cast(SubExprs[1]); }
> +  UnresolvedLookupExpr *getCallee() const {
> +return static_cast(SubExprs[SubExpr::Callee]);
> +  }
> +  Expr *getLHS() const { return
> static_cast(SubExprs[SubExpr::LHS]); }
> +  Expr *getRHS() const { return
> static_cast(SubExprs[SubExpr::RHS]); }
>
>/// Does this produce a right-associated sequence of operators?
>bool isRightFold() const {
> @@ -4577,10 +4584,12 @@ class CXXFoldExpr : public Expr {
>}
>
>// Iterators
> -  child_range children() { return child_range(SubExprs, SubExprs + 2); }
> +  child_range children() {
> +return child_range(SubExprs, SubExprs + SubExpr::Count);
> +  }
>
>const_child_range children() const {
> -return const_child_range(SubExprs, SubExprs + 2);
> +return const_child_range(SubExprs, SubExprs + SubExpr::Count);
>}
>  };
>
>
> diff  --git a/clang/include/clang/Sema/Sema.h
> b/clang/include/clang/Sema/Sema.h
> index 7a83e0316618..b88e5578c114 100644
> --- a/clang/include/clang/Sema/Sema.h
> +++ b/clang/include/clang/Sema/Sema.h
> @@ -3556,6 +3556,12 @@ class Sema final {
>OverloadCandidateSet *CandidateSet,
>ExprResult *Result);
>
> +  ExprResult CreateUnresolvedLookupExpr(CXXRecordDecl *NamingClass,
> +NestedNameSpecifierLoc NNSLoc,
> +DeclarationNameInfo DNI,
> +const UnresolvedSetImpl &Fns,
> +bool PerformADL = true);
> +
>ExprResult CreateOverloadedUnaryOp(SourceLocation OpLoc,
>   UnaryOperatorKind Opc,
>   const UnresolvedSetImpl &Fns

[PATCH] D85157: [Sema] Add casting check for fixed to fixed point conversions

2020-08-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment.

Ping! Ok to land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85157

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


[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment.

Thanks for the recent comments. I just pushed a few improvements over the last 
patch that didn't comprehend latest comments from @rsmith and @Quuxplusone. 
I'll read through those carefully and address those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85097

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


[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 283780.
vabridgers added a comment.

use source from expression in fixit
s/seperate/separate/
address some chained comparison ambiguities outside of original scope of changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85097

Files:
  clang/docs/DiagnosticsReference.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/warn-compare-op-parentheses.c

Index: clang/test/Sema/warn-compare-op-parentheses.c
===
--- /dev/null
+++ clang/test/Sema/warn-compare-op-parentheses.c
@@ -0,0 +1,263 @@
+// RUN: %clang_cc1 -fsyntax-only -Wcompare-op-parentheses -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wparentheses -verify %s
+
+int tests(int p1, int p2, int p3, int p4, int p5, int p6) {
+  int b = 0;
+  b = p1 < p2 < p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+  b = p1 < p2 <= p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+  b = p1 <= p2 <= p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '<=' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+  b = p1 <= p2 < p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '<=' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+  b = p1 > p2 > p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '>' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '>' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+  b = p1 > p2 >= p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '>' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '>=' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+  b = p1 >= p2 >= p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '>=' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '>=' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+  b = p1 >= p2 > p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '>=' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '>' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+  b = p1 > p2 < p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses around the '>' comparison to silence this warning}}
+  // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+  // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+  b = p1 p3;
+  // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+  // expected-note@-2 {{place parentheses aroun

[PATCH] D81479: [BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic

2020-08-06 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song abandoned this revision.
yonghong-song added a comment.

inline asm can do the work, so abandon this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81479

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


[clang] ed5a18f - PR30738: Implement two-phase name lookup for fold-expressions.

2020-08-06 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-08-06T16:56:39-07:00
New Revision: ed5a18fc0399dce994aa354a33e6f981f9828647

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

LOG: PR30738: Implement two-phase name lookup for fold-expressions.

Added: 


Modified: 
clang/include/clang/AST/ExprCXX.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/Parse/ParseExpr.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/AST/ast-dump-expr-json.cpp
clang/test/AST/ast-dump-expr.cpp
clang/test/SemaTemplate/cxx1z-fold-expressions.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 3f272c96a2d8..b53bb20f7ebc 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4519,31 +4519,38 @@ class CXXFoldExpr : public Expr {
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 
+  enum SubExpr { Callee, LHS, RHS, Count };
+
   SourceLocation LParenLoc;
   SourceLocation EllipsisLoc;
   SourceLocation RParenLoc;
   // When 0, the number of expansions is not known. Otherwise, this is one more
   // than the number of expansions.
   unsigned NumExpansions;
-  Stmt *SubExprs[2];
+  Stmt *SubExprs[SubExpr::Count];
   BinaryOperatorKind Opcode;
 
 public:
-  CXXFoldExpr(QualType T, SourceLocation LParenLoc, Expr *LHS,
-  BinaryOperatorKind Opcode, SourceLocation EllipsisLoc, Expr *RHS,
-  SourceLocation RParenLoc, Optional NumExpansions)
+  CXXFoldExpr(QualType T, UnresolvedLookupExpr *Callee,
+  SourceLocation LParenLoc, Expr *LHS, BinaryOperatorKind Opcode,
+  SourceLocation EllipsisLoc, Expr *RHS, SourceLocation RParenLoc,
+  Optional NumExpansions)
   : Expr(CXXFoldExprClass, T, VK_RValue, OK_Ordinary), 
LParenLoc(LParenLoc),
 EllipsisLoc(EllipsisLoc), RParenLoc(RParenLoc),
 NumExpansions(NumExpansions ? *NumExpansions + 1 : 0), Opcode(Opcode) {
-SubExprs[0] = LHS;
-SubExprs[1] = RHS;
+SubExprs[SubExpr::Callee] = Callee;
+SubExprs[SubExpr::LHS] = LHS;
+SubExprs[SubExpr::RHS] = RHS;
 setDependence(computeDependence(this));
   }
 
   CXXFoldExpr(EmptyShell Empty) : Expr(CXXFoldExprClass, Empty) {}
 
-  Expr *getLHS() const { return static_cast(SubExprs[0]); }
-  Expr *getRHS() const { return static_cast(SubExprs[1]); }
+  UnresolvedLookupExpr *getCallee() const {
+return static_cast(SubExprs[SubExpr::Callee]);
+  }
+  Expr *getLHS() const { return static_cast(SubExprs[SubExpr::LHS]); }
+  Expr *getRHS() const { return static_cast(SubExprs[SubExpr::RHS]); }
 
   /// Does this produce a right-associated sequence of operators?
   bool isRightFold() const {
@@ -4577,10 +4584,12 @@ class CXXFoldExpr : public Expr {
   }
 
   // Iterators
-  child_range children() { return child_range(SubExprs, SubExprs + 2); }
+  child_range children() {
+return child_range(SubExprs, SubExprs + SubExpr::Count);
+  }
 
   const_child_range children() const {
-return const_child_range(SubExprs, SubExprs + 2);
+return const_child_range(SubExprs, SubExprs + SubExpr::Count);
   }
 };
 

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 7a83e0316618..b88e5578c114 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3556,6 +3556,12 @@ class Sema final {
   OverloadCandidateSet *CandidateSet,
   ExprResult *Result);
 
+  ExprResult CreateUnresolvedLookupExpr(CXXRecordDecl *NamingClass,
+NestedNameSpecifierLoc NNSLoc,
+DeclarationNameInfo DNI,
+const UnresolvedSetImpl &Fns,
+bool PerformADL = true);
+
   ExprResult CreateOverloadedUnaryOp(SourceLocation OpLoc,
  UnaryOperatorKind Opc,
  const UnresolvedSetImpl &Fns,
@@ -3822,7 +3828,6 @@ class Sema final {
   bool LookupInSuper(LookupResult &R, CXXRecordDecl *Class);
 
   void LookupOverloadedOperatorName(OverloadedOperatorKind Op, Scope *S,
-QualType T1, QualType T2,
 UnresolvedSetImpl &Functions);
 
   LabelDecl *LookupOrCreateLabel(IdentifierInfo *II, SourceLocation IdentLoc,
@@ -5166,6 +5171,8 @@ class Sema fi

[PATCH] D79279: Add overloaded versions of builtin mem* functions

2020-08-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D79279#2201136 , @rjmccall wrote:

> In D79279#2200916 , @rsmith wrote:
>
>> In D79279#2197176 , @rjmccall wrote:
>>
>>> I thought part of the point of `__builtin_memcpy` was so that C library 
>>> headers could do `#define memcpy(x, y, z) __builtin_memcpy(x, y, z)`.  If 
>>> so, the conformance issue touches `__builtin_memcpy` as well, not just 
>>> calls to the library builtin.

For what it's worth, giving `__builtin_memcpy` broader semantics than `memcpy` 
wouldn't be unprecedented: it's already the case that `__builtin_memcpy` is 
usable in constant expressions where plain `memcpy` is not (but there's no 
macro risk in that case at least since C++ `memcpy` can't be a macro, and a 
call to memcpy is never an ICE in C).

>> They would have to declare it as well (because C code can `#undef memcpy` 
>> and expect to then be able to call a real function), so the `#define` would 
>> be pointless.
>
> It wouldn't be pointless; it would enable optimization of direct calls to 
> memcpy (the 99% case) without the compiler having to special-case a function 
> by name.

I mean, yes, in an environment where the compiler didn't special-case the 
function by name anyway it wouldn't be pointless. I'm not aware of any such 
environment in popular usage, but that could just be ignorance on my part.

> If we just want memcpy to do the right thing when called directly, that's not 
> ridiculous.  I don't think it would actually have any conformance problems: a 
> volatile memcpy is just a less optimizable memcpy, and to the extent that an 
> address-space-aware memcpy is different from the standard definition, it's 
> probably UB to use the standard definition to copy memory in non-generic 
> address spaces.

I think this is a constraint violation in C; C11 6.5.2.2/2 (a "Constraints" 
paragraph) requires that "Each argument shall have a type such that its value 
may be assigned to an object with the unqualified version of the type of its 
corresponding parameter." So this would be an extension, not just defining UB, 
and seems like the kind of extension we'd normally diagnose under 
`-pedantic-errors`, but we could make an exception. I also think it'd be mildly 
surprising for an explicitly-declared function to allow different argument 
conversions depending on whether its name is `memcpy`.

It seems to me that you're not entirely comfortable with making `memcpy` and 
`__builtin_memcpy` differ in this way, and I'm not entirely comfortable with 
making `memcpy`'s behavior differ from its declared type. Meanwhile, 
@tstellar's patch wants `__builtin_memcpy` to be overloaded on address space. I 
don't see a way to address all three concerns at once.

I think it would be reasonable in general to guarantee that our `__builtin_` 
functions have contracts at least as wide as the underlying C function, but 
allow them to have extensions, and to keep the plain C functions unextended. I 
had actually thought we already did that in more cases than we do (but perhaps 
I was thinking about the LLVM math intrinsics that guarantee to not set 
`errno`). That would mean that a C standard library implementation is still 
free to `#define foo(x,y,z) __builtin_foo(x,y,z)`, but if they do, they may 
pick up extensions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

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


[PATCH] D85485: Fix quiet mode in git-clang-format

2020-08-06 Thread Gvald Ike via Phabricator via cfe-commits
Gvald added reviewers: jmerdich, rsmith, djasper, serge-sans-paille.
Gvald added a comment.

Quiet mode is very useful for scripting, when only the diff format output is 
required, or no output if not formatting is needed.
In case of no modified files, git-clang-format will output to screen even 
though the quiet mode enabled.

This patch changes this behavior, so if quiet flag passes in - no output will 
be available, even if no modified files exists.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85485

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


[PATCH] D85485: Fix quiet mode in git-clang-format

2020-08-06 Thread Gvald Ike via Phabricator via cfe-commits
Gvald created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Gvald requested review of this revision.

Eliminate output in quiet mode, when no modified files exists


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85485

Files:
  clang/tools/clang-format/git-clang-format


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -148,7 +148,8 @@
   for filename in changed_lines:
 print('%s' % filename)
   if not changed_lines:
-print('no modified files to format')
+if opts.verbose >=0:
+  print('no modified files to format')
 return
   # The computed diff outputs absolute paths, so we must cd before accessing
   # those files.


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -148,7 +148,8 @@
   for filename in changed_lines:
 print('%s' % filename)
   if not changed_lines:
-print('no modified files to format')
+if opts.verbose >=0:
+  print('no modified files to format')
 return
   # The computed diff outputs absolute paths, so we must cd before accessing
   # those files.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

2020-08-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D85191#2199574 , @ebevhan wrote:

> In D85191#2197663 , @efriedma wrote:
>
>>> If the intent is for getTypeInfo to always return sizes that are multiples 
>>> of the char size, then the design should be inverted and getTypeInfo should 
>>> simply be calling getTypeInfoInChars and multiply that result by the char 
>>> size. But that isn't how it works.
>>
>> The reason it doesn't work this way is just that someone made the wrong 
>> choice a decade ago, and nobody has spent the time to rewrite it since.  
>> Patch welcome.
>
> This does sound like a good thing to do, but it would be problematic 
> downstream since it would completely prohibit the design that we're trying to 
> use.

That would be a good thing, as the design you're trying to use is not one that 
we intend to support. The size returned by `getTypeInfo` is intended to include 
padding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85191

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


[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

2020-08-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D84364#2176091 , @yaxunl wrote:

> I added a `Deferrable` bit to the diagnostics which can be specified in td 
> files. This can be added to individual diagnostic defs or added to a bunch of 
> diagnostic defs all together.
>
> This field is used to control whether a diagnostic message can be deferred.

This may be a case of "too much, but not enough". It will be unnecessary for 
most of the diagnostics we have. Overload resolution is likely to be the 
primary beneficiary, inline asm and exceptions may be two other classes, but I 
can't think of anything else ATM. 
At the same time it may not be enough, because we also need to take into 
account where and when particular diagnostic is emitted. I.e. the same 
diagnostic may need to be postponed when we emit it from CUDA code, yet we may 
want to *not* postpone it if it's in the code which has nothing to do with 
CUDA. E.g. C++ code which has oveloading-related error in an inline function 
which would not be codegen'ed. I would expect such error to be reported as it 
would be if the same function was compiled in plain C++ mode.

> For now I enabled this bit for the overloading resolution diagnostics since 
> these have been seen as false alarms in host device functions. We could let 
> more diagnostics be deferrable if we see it is necessary.



> I just saw bugzilla bug https://bugs.llvm.org/show_bug.cgi?id=46922
> my patch https://reviews.llvm.org/D77954 is supposed to fix this issue. 
> However since implicit host device functions often cause overloading 
> resolution diagnostics on the device side which are not deferred, my patch 
> caused regressions and was reverted several times. Currently it was still 
> reverted.
>
> I think to fix this issue we need to make overloading resolution diagnostics 
> deferrable.

I'm missing something here. How would deferred diagnostics help with the issue 
in the bug 46922? The HD function there is codegen'ed on both sides, so the 
only difference postponing would make is that we'd emit the diagnostic a bit 
later.
Do you mean that postponed diags patch is not the fix, but rather a 
prerequisite for the overload resolution changes patch?


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

https://reviews.llvm.org/D84364

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


[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

2020-08-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

I just saw bugzilla bug https://bugs.llvm.org/show_bug.cgi?id=46922

my patch https://reviews.llvm.org/D77954 is supposed to fix this issue. However 
since implicit host device functions often cause overloading resolution 
diagnostics on the device side which are not deferred, my patch caused 
regressions and was reverted several times. Currently it was still reverted.

I think to fix this issue we need to make overloading resolution diagnostics 
deferrable.


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

https://reviews.llvm.org/D84364

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


[PATCH] D85390: [Clang] Add note for bad conversion error when expression is of forward-declared type

2020-08-06 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 283744.
zequanwu added a comment.

Add new note and test case.
I think for the class template case, the new note might not be useful, since 
they might simply caused by mismatch of template arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85390

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/Modules/namespaces.cpp
  clang/test/SemaCXX/elaborated-type-specifier.cpp
  clang/test/SemaCXX/pointer-forward-declared-class-conversion.cpp


Index: clang/test/SemaCXX/pointer-forward-declared-class-conversion.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/pointer-forward-declared-class-conversion.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class A1 {};
+class B1; // expected-note{{forward declaration of class here, conversion will 
be valid if the class 'B1' is derived from 'A1'}}
+B1 *b1;
+A1 *a1 = b1; // expected-error{{cannot initialize a variable of type 'A1 *' 
with an lvalue of type 'B1 *'}}
+
+template  class A2 {};
+template  class B2;
+B2 *b2;
+A2 *a2 = b2; // expected-error{{cannot initialize a variable of type 
'A2 *' with an lvalue of type 'B2 *'}}
Index: clang/test/SemaCXX/elaborated-type-specifier.cpp
===
--- clang/test/SemaCXX/elaborated-type-specifier.cpp
+++ clang/test/SemaCXX/elaborated-type-specifier.cpp
@@ -26,7 +26,7 @@
 }
 
 void test_X_elab(NS::X x) {
-  struct S4 *s4 = 0;
+  struct S4 *s4 = 0; // expected-note{{forward declaration of class here, 
conversion will be valid if the struct 'S4' is derived from 'NS::S4'}}
   x.test_elab2(s4); // expected-error{{cannot initialize a parameter of type 
'NS::S4 *' with an lvalue of type 'struct S4 *'}}
 }
 
Index: clang/test/Modules/namespaces.cpp
===
--- clang/test/Modules/namespaces.cpp
+++ clang/test/Modules/namespaces.cpp
@@ -78,7 +78,8 @@
 
 // expected-note@Inputs/namespaces-right.h:60 {{passing argument to parameter 
here}}
 // expected-note@Inputs/namespaces-right.h:67 {{passing argument to parameter 
here}}
-
+// expected-note@Inputs/namespaces-left.h:63 {{forward declaration of class 
here, conversion will be valid if the class 'N11::(anonymous namespace)::Foo' 
is derived from 'N11::(anonymous namespace)::Foo'}}
+// expected-note@Inputs/namespaces-left.h:70 {{forward declaration of class 
here, conversion will be valid if the class 'N12::(anonymous namespace)::Foo' 
is derived from 'N12::(anonymous namespace)::Foo'}}
 // Test that bringing in one name from an overload set does not hide the rest.
 void testPartialImportOfOverloadSet() {
   void (*p)() = N13::p;
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8707,6 +8707,16 @@
 if (entity.getKind() == InitializedEntity::EK_Result)
   S.EmitRelatedResultTypeNoteForReturn(destType);
   }
+  QualType fromType = op->getType();
+  auto *fromDecl = fromType.getTypePtr()->getPointeeCXXRecordDecl();
+  auto *destDecl = destType.getTypePtr()->getPointeeCXXRecordDecl();
+  if (fromDecl && destDecl && fromDecl->getDeclKind() == Decl::CXXRecord &&
+  destDecl->getDeclKind() == Decl::CXXRecord &&
+  !fromDecl->isInvalidDecl() && !destDecl->isInvalidDecl() &&
+  !fromDecl->hasDefinition())
+S.Diag(fromDecl->getLocation(), diag::note_forward_class_conversion)
+<< fromDecl->isStruct() << S.getASTContext().getTagDeclType(fromDecl)
+<< S.getASTContext().getTagDeclType(destDecl);
 }
 
 static void diagnoseListInit(Sema &S, const InitializedEntity &Entity,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2005,6 +2005,9 @@
   "|: different return type%diff{ ($ vs $)|}5,6"
   "|: different qualifiers (%5 vs %6)"
   "|: different exception specifications}4">;
+def note_forward_class_conversion : Note<"forward declaration of class here, "
+  "conversion will be valid if the %select{class|struct}0 %1 is derived from 
%2"
+  >;
 
 def err_lvalue_to_rvalue_ref : Error<"rvalue reference %diff{to type $ cannot "
   "bind to lvalue of type $|cannot bind to incompatible lvalue}0,1">;


Index: clang/test/SemaCXX/pointer-forward-declared-class-conversion.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/pointer-forward-declared-class-conversion.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class A1 {};
+class B1; // expected-note{{forward declaration of class here, conversion will be valid if the class 'B1' i

[PATCH] D85315: [AIX][Clang][Driver] Generate reference to the C++ library on the link step

2020-08-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:210
+  case ToolChain::CST_Libstdcxx:
+llvm::report_fatal_error("linking libstdc++ unimplemented on AIX");
+  }

Should there be a test for the error message?



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:211
+llvm::report_fatal_error("linking libstdc++ unimplemented on AIX");
+  }
+}

I suggest using `return` inside the switch and having `llvm_unreachable` after 
the `switch` if the `switch` is expected to handle all of the possible cases.



Comment at: clang/test/Driver/aix-ld.c:18
 // CHECK-LD32: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32: "-lc"
 

Should there be tests that the C++ library is not included when linking with a 
C invocation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85315

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


[PATCH] D85390: [Clang] Add note for bad conversion error when expression is of forward-declared type

2020-08-06 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:8710
   }
+  QualType fromType = op->getType();
+  auto *fromDecl = fromType.getTypePtr()->getPointeeCXXRecordDecl();

zequanwu wrote:
> hans wrote:
> > Can the reverse situation happen, where it's destType that's forward 
> > declared?
> If we only consider normal class without template, I think it's not necessary.
> Here is an example of forward-declared destType.
> ```
> class B{};
> class A;
> B *b;
> A *a= b;
> ```
> Even if A is defined as `class A: public B{};`, `A *a = b` is still not 
> valid, initializing pointer to derived class with pointer to base class.
The reverse situation could happen in class template.
```
template class A{};
template class B; // template class B : public A{}; 
compiles
B *b;
A *a = b;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85390

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


[PATCH] D84029: [clang][Driver] Default to /usr/bin/ld on Solaris

2020-08-06 Thread Rainer Orth via Phabricator via cfe-commits
ro added a comment.

Ping?  It's been a week...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84029

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


[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

2020-08-06 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4642
+int Num;
+if (V == "future")
+  A->render(Args, CmdArgs);

Another option would be `unstable`.



Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:738
 
-  if (!getContext().getAsmInfo()->useIntegratedAssembler()) {
-// If we are not using the integrated assembler then this symbol might have
+  if (!getContext().getAsmInfo()->useIntegratedASOrGAS(2, 35)) {
+// If we are using GNU as before 2.35, then this symbol might have

I'd prefer to spell it out in full, i.e. 
`getContext().getAsmInfo()->useIntegratedAssembler() || 
!getContext().getAsmInfo()->binutilsVersion(2, 35)`, it's more verbose but it's 
cleaner to me what it does just when looking at this line alone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85474

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


[PATCH] D85390: [Clang] Add note for bad conversion error when expression is of forward-declared type

2020-08-06 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:8710
   }
+  QualType fromType = op->getType();
+  auto *fromDecl = fromType.getTypePtr()->getPointeeCXXRecordDecl();

hans wrote:
> Can the reverse situation happen, where it's destType that's forward declared?
If we only consider normal class without template, I think it's not necessary.
Here is an example of forward-declared destType.
```
class B{};
class A;
B *b;
A *a= b;
```
Even if A is defined as `class A: public B{};`, `A *a = b` is still not valid, 
initializing pointer to derived class with pointer to base class.



Comment at: clang/lib/Sema/SemaInit.cpp:8713
+  if (fromDecl && !fromDecl->hasDefinition() && !fromDecl->isInvalidDecl() &&
+  fromDecl->getDeclKind() == Decl::CXXRecord)
+S.Diag(fromDecl->getLocation(), diag::note_forward_declaration)

hans wrote:
> Is the getDeclKind() still necessary even though you're doing 
> getPointeeCXXRecordDecl() above and fromDecl is os type CXXRecordDecl*?
Because I was thinking about excluding class template. 
getPointeeCXXRecordDecl() might return a `ClassTemplateSpecializationDecl` or 
`ClassTemplatePartialSpecializationDecl`.
For following example, it triggers the same error. Should the new note also be 
emitted for this case? 
```
template class A{};
A *a1;
A *a2 = a1;
```
And the following code compiles:
```
template class A{};
template<> class A{};
template<> class A : public A{};
A *a1;
A *a2 = a1;
```
This seems like the same error as normal class, could resolved by inheritance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85390

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


[PATCH] D79279: Add overloaded versions of builtin mem* functions

2020-08-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D79279#2200916 , @rsmith wrote:

> In D79279#2197176 , @rjmccall wrote:
>
>> I thought part of the point of `__builtin_memcpy` was so that C library 
>> headers could do `#define memcpy(x, y, z) __builtin_memcpy(x, y, z)`.  If 
>> so, the conformance issue touches `__builtin_memcpy` as well, not just calls 
>> to the library builtin.
>
> They would have to declare it as well (because C code can `#undef memcpy` and 
> expect to then be able to call a real function), so the `#define` would be 
> pointless.

It wouldn't be pointless; it would enable optimization of direct calls to 
memcpy (the 99% case) without the compiler having to special-case a function by 
name.  And you don't need an `#undef`, since `&memcpy` doesn't trigger the 
preprocessor when `memcpy` is a function-like macro.  I seem to remember this 
being widely used in some math libraries, but it's entirely possible that it's 
never been used for `memcpy` and the like.  It's also entirely possible that 
I'm passing around folklore.

If we just want memcpy to do the right thing when called directly, that's not 
ridiculous.  I don't think it would actually have any conformance problems: a 
volatile memcpy is just a less optimizable memcpy, and to the extent that an 
address-space-aware memcpy is different from the standard definition, it's 
probably UB to use the standard definition to copy memory in non-generic 
address spaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

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


[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

2020-08-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: jyknight, pcc, phosek.
Herald added subscribers: llvm-commits, cfe-commits, dang, steven.zhang, 
atanasyan, hiraditya, arichardson, sdardis.
Herald added a reviewer: rengolin.
Herald added projects: clang, LLVM.
MaskRay requested review of this revision.

There are two use cases.

Assembler
We have accrued some code gated on MCAsmInfo::useIntegratedAssembler().  Some
features are supported by latest GNU as, but we have to use
MCAsmInfo::useIntegratedAs() because the newer versions have not been widely
adopted (e.g. SHF_LINK_ORDER 'o' and 'unique' linkage in 2.35, 
--compress-debug-sections= in 2.26).

Linker
We want to use features supported only by LLD or very new GNU ld, or don't want
to work around older GNU ld. We currently can't represent that "we don't care
about old GNU ld".  You can find such workarounds in a few other places, e.g.
Mips/MipsAsmprinter.cpp PowerPC/PPCTOCRegDeps.cpp X86/X86MCInstrLower.cpp
Fuchsia has an immediate use case for mixed SHF_LINK_ORDER and
non-SHF_LINK_ORDER components (D76802 ; will 
be supported by LLD in D84001 ;
GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=16833 is 
pending).

This patch adds -fbinutils-version= to clang and -binutils-version to llc.
It changes one codegen place in SHF_MERGE to demonstrate its usage.
-fbinutils-version=2.35 means the produced object file does not care
about GNU ld<2.35 compatibility. When -fno-integrated-as is specified,
the produced assembly does not care about GNU as<2.35 compatibility.

-fbinutils-version=future means that we can freely use unimplemented GNU
as/ld features.

Such command line parsing is usually implemented in
llvm/lib/CodeGen/CommandFlags.cpp (LLVMCodeGen), however, ClangCodeGen does not
depend on LLVMCodeGen. So I add parseBinutilsVersion to
llvm/lib/Target/TargetMachine.cpp (LLVMTarget).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85474

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/MC/MCAsmInfo.h
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/LLVMTargetMachine.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/TargetMachine.cpp
  llvm/test/CodeGen/X86/explicit-section-mergeable.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -81,6 +81,14 @@
  cl::value_desc("N"),
  cl::desc("Repeat compilation N times for timing"));
 
+static cl::opt BinutilsVersion(
+"binutils-version", cl::Hidden,
+cl::desc(
+"Used ELF features need to be supported by GNU ld "
+"from the specified binutils version. 'future' means that "
+"features not implemented by any known release can be used. If "
+"-no-integrated-as is specified, this also affects assembly output"));
+
 static cl::opt
 NoIntegratedAssembler("no-integrated-as", cl::Hidden,
   cl::desc("Disable integrated assembler"));
@@ -425,6 +433,8 @@
   }
 
   TargetOptions Options = codegen::InitTargetOptionsFromCodeGenFlags();
+  Options.BinutilsVersion =
+  TargetMachine::parseBinutilsVersion(BinutilsVersion);
   Options.DisableIntegratedAS = NoIntegratedAssembler;
   Options.MCOptions.ShowMCEncoding = ShowMCEncoding;
   Options.MCOptions.MCUseDwarfDirectory = EnableDwarfDirectory;
Index: llvm/test/CodeGen/X86/explicit-section-mergeable.ll
===
--- llvm/test/CodeGen/X86/explicit-section-mergeable.ll
+++ llvm/test/CodeGen/X86/explicit-section-mergeable.ll
@@ -282,15 +282,19 @@
 ;; --no-integrated-as avoids the use of ",unique," for compatibility with older binutils.
 
 ;; Error if an incompatible symbol is explicitly placed into a mergeable section.
-; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as 2>&1 \
+; RUN: not llc < %s -mtriple=x86_64 --no-integrated-as -binutils-version=2.34 2>&1 \
 ; RUN: | FileCheck %s --check-prefix=NO-I-AS-ERR
 ; NO-I-AS-ERR: error: Symbol 'explicit_default_1' from module '' required a section with entry-size=0 but was placed in section '.rodata.cst16' with entry-size=16: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_default_4' from module '' required a section with entry-size=0 but was placed in section '.debug_str' with entry-size=1: Explicit assignment by pragma or attribute of an incompatible symbol to this section?
 ; NO-I-AS-ERR: error: Symbol 'explicit_implicit_2' from module '' required a section with entry-size=0 but was placed in section 

[PATCH] D85025: [RecoveryExpr]WIP: Support dependence in C-only codepath

2020-08-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Looks reasonable to me. I expect you'll find more places that need to learn how 
to handle dependent types in C, but this looks like a solid start.




Comment at: clang/lib/AST/Expr.cpp:3757-3758
 case NPC_ValueDependentIsNull:
-  if (isTypeDependent() || getType()->isIntegralType(Ctx))
+  if ((!containsErrors() && isTypeDependent()) ||
+  getType()->isIntegralType(Ctx))
 return NPCK_ZeroExpression;

This change appears to be redundant: we handled all `containsErrors()` cases 
before the `switch`.



Comment at: clang/lib/Sema/SemaCast.cpp:2693
+  (DestType->isDependentType() || SrcExpr.get()->isTypeDependent() ||
+   SrcExpr.get()->isValueDependent())) {
+assert((DestType->containsErrors() || SrcExpr.get()->containsErrors() ||

Do we care about value-dependence here? Very little of C cast semantics depends 
on the evaluated value of the expression -- I think this only matters for null 
pointer constants. If we care about the value-dependent cast to pointer case, 
maybe we should special-case that?

(It looks like the special-case handling for OpenCL event_t will also need a 
value-dependence check.)



Comment at: clang/lib/Sema/SemaExpr.cpp:14245-14247
+ExprResult Sema::CreateDependentBinOp(SourceLocation OpLoc,
+  BinaryOperatorKind Opc, Expr *LHS,
+  Expr *RHS) {

This function seems dangerous: in C++, we need to perform unqualified lookups 
from the template definition context when creating a dependent binary operator, 
and it's only correct to use this if such lookup found nothing.

Perhaps you could add something to the name of the function to indicate that it 
should only be used when there are no unqualified lookup results?



Comment at: clang/test/Sema/error-dependence.c:10
+
+  // verify disgnostic "called object type '' is not a function
+  // or function pointer" is not emitted.





Comment at: clang/test/Sema/typo-correction.c:17
 int foobar;  // expected-note {{'foobar' declared here}}
-a = goobar ?: 4;  // expected-warning {{type specifier missing, defaults to 
'int'}} \
+new_a = goobar ?: 4;  // expected-warning {{type specifier missing, defaults 
to 'int'}} \
   // expected-error {{use of undeclared identifier 'goobar'; 
did you mean 'foobar'?}} \

I assume we're getting a redefinition error for `a` now. If so, can you test 
that uses of `a` after line 13 don't produce errors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85025

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


[PATCH] D85473: [Clang] Add option to allow marking pass-by-value args as noalias.

2020-08-06 Thread Florian Hahn via Phabricator via cfe-commits
fhahn created this revision.
fhahn added reviewers: rjmccall, rsmith, hfinkel, jfb.
Herald added subscribers: dang, dexonsmith.
Herald added a project: clang.
fhahn requested review of this revision.

After the recent discussion on cfe-dev 'Can indirect class parameters be
noalias?' [1], it seems like using using noalias is problematic for
current C++, but should be allowed for C-only code.

This patch introduces a new option to let the user indicate that it is
safe to mark indirect class parameters as noalias. Note that this also
applies to external callers, e.g. it might not be safe to use this flag
for C functions that are called by C++ functions.

In targets that allocate indirect arguments in the called function, this
enables more agressive optimizations with respect to memory operations
and brings a ~1% - 2% codesize reduction for some programs.

I am not sure about the best name for the option and description. I
 would appreciate any feedback.

[1] : http://lists.llvm.org/pipermail/cfe-dev/2020-July/066353.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85473

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/pass-by-value-noalias.c


Index: clang/test/CodeGen/pass-by-value-noalias.c
===
--- /dev/null
+++ clang/test/CodeGen/pass-by-value-noalias.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fpass-by-value-noalias -triple arm64-apple-iphoneos 
-emit-llvm -disable-llvm-optzns %s -o - 2>&1 | FileCheck 
--check-prefix=WITH_NOALIAS %s
+// RUN: %clang_cc1 -triple arm64-apple-iphoneos -emit-llvm 
-disable-llvm-optzns %s -o - 2>&1 | FileCheck --check-prefix=NO_NOALIAS %s
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+  int e;
+  int f;
+};
+
+// WITH_NOALIAS: define void @take(%struct.Foo* noalias %arg)
+// NO_NOALIAS: define void @take(%struct.Foo* %arg)
+void take(struct Foo arg) {}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3419,6 +3419,7 @@
   Opts.PCHInstantiateTemplates = Args.hasArg(OPT_fpch_instantiate_templates);
 
   Opts.MatrixTypes = Args.hasArg(OPT_fenable_matrix);
+  Opts.PassByValueNoAlias = Args.hasArg(OPT_fpass_by_value_noalias);
 
   Opts.MaxTokens = getLastArgIntValue(Args, OPT_fmax_tokens_EQ, 0, Diags);
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2184,6 +2184,11 @@
   if (AI.getIndirectByVal())
 Attrs.addByValAttr(getTypes().ConvertTypeForMem(ParamType));
 
+  if (getLangOpts().PassByValueNoAlias)
+// When calling the function, the pointer passed in will be the only
+// reference to the underlying object. Mark it accordingly.
+Attrs.addAttribute(llvm::Attribute::NoAlias);
+
   CharUnits Align = AI.getIndirectAlign();
 
   // In a byval argument, it is important that the required
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4284,6 +4284,10 @@
 def fcompatibility_qualified_id_block_param_type_checking : Flag<["-"], 
"fcompatibility-qualified-id-block-type-checking">,
   HelpText<"Allow using blocks with parameters of more specific type than "
"the type system guarantees when a parameter is qualified id">;
+def fpass_by_value_noalias: Flag<["-"], "fpass-by-value-noalias">,
+  HelpText<"Allows assuming no references to passed by value escape before "
+   "transferring execution to the called function. Note that this "
+   "does not hold for C++">;
 
 // FIXME: Remove these entirely once functionality/tests have been excised.
 def fobjc_gc_only : Flag<["-"], "fobjc-gc-only">, Group,
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -368,6 +368,10 @@
 LANGOPT(RegisterStaticDestructors, 1, 1, "Register C++ static destructors")
 
 LANGOPT(MatrixTypes, 1, 0, "Enable or disable the builtin matrix type")
+LANGOPT(PassByValueNoAlias, 1, 0, "Allows assuming no references to passed by "
+  "value escape before transferring execution "
+  "to the called function. Note that this does 
"
+  "not hold for C++")
 
 COMPATIBLE_VALUE_LANGOPT(MaxTokens, 32, 0, "Max number of tokens per TU or 0")
 


Index: clang/test/CodeGen/pass-by-value-noalias.c

[PATCH] D85471: Make clang HIP headers compatible with C++98

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

LGTM. thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85471

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


[clang] d6492d8 - Add -Wtautological-value-range-compare warning.

2020-08-06 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-08-06T13:28:50-07:00
New Revision: d6492d874478b1d3b1ce3adb4c3044618bec29e9

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

LOG: Add -Wtautological-value-range-compare warning.

This warning diagnoses cases where an expression is compared to a
constant, and the comparison is tautological due to the form of the
expression (but not merely due to its type). This applies in cases such
as comparisons of bit-fields and the result of bit-masks.

The new warning is added to the Clang diagnostic group
-Wtautological-constant-in-range-compare but not to the
formerly-equivalent GCC-compatibility diagnostic group -Wtype-limits,
which retains its old meaning of diagnosing only tautological
comparisons to extremal values of a type (eg, int > INT_MAX).

Reviewed By: rtrieu

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/tautological-constant-compare.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index be62461faef48..5ddd37e9972af 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -555,12 +555,16 @@ def IntInBoolContext : DiagGroup<"int-in-bool-context">;
 def TautologicalTypeLimitCompare : 
DiagGroup<"tautological-type-limit-compare">;
 def TautologicalUnsignedZeroCompare : 
DiagGroup<"tautological-unsigned-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : 
DiagGroup<"tautological-unsigned-enum-zero-compare">;
+// For compatibility with GCC. Tautological comparison warnings for constants
+// that are an extremal value of the type.
+def TypeLimits : DiagGroup<"type-limits", [TautologicalTypeLimitCompare,
+   TautologicalUnsignedZeroCompare,
+   
TautologicalUnsignedEnumZeroCompare]>;
+// Additional tautological comparison warnings based on the expression, not
+// only on its type.
+def TautologicalValueRangeCompare : 
DiagGroup<"tautological-value-range-compare">;
 def TautologicalInRangeCompare : 
DiagGroup<"tautological-constant-in-range-compare",
-   [TautologicalTypeLimitCompare,
-TautologicalUnsignedZeroCompare,
-
TautologicalUnsignedEnumZeroCompare]>;
-// For compatibility with GCC; -Wtype-limits = 
-Wtautological-constant-in-range-compare
-def TypeLimits : DiagGroup<"type-limits", [TautologicalInRangeCompare]>;
+   [TypeLimits, 
TautologicalValueRangeCompare]>;
 def TautologicalOutOfRangeCompare : 
DiagGroup<"tautological-constant-out-of-range-compare">;
 def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare",
 [TautologicalOutOfRangeCompare]>;
@@ -624,7 +628,6 @@ def ImplicitFallthrough  : DiagGroup<"implicit-fallthrough",
 def InvalidPPToken : DiagGroup<"invalid-pp-token">;
 def Trigraphs  : DiagGroup<"trigraphs">;
 
-def : DiagGroup<"type-limits">;
 def UndefinedReinterpretCast : DiagGroup<"undefined-reinterpret-cast">;
 def ReinterpretBaseClass : DiagGroup<"reinterpret-base-class">;
 def Unicode  : DiagGroup<"unicode">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7bcff3eb4d8c5..1aeaf590cbb4b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6588,6 +6588,12 @@ def warn_tautological_compare_objc_bool : Warning<
   "result of comparison of constant %0 with expression of type 'BOOL'"
   " is always %1, as the only well defined values for 'BOOL' are YES and NO">,
   InGroup;
+def subst_int_range : TextSubstitution<"%0-bit %select{signed|unsigned}1 
value">;
+def warn_tautological_compare_value_range : Warning<
+  "result of comparison of "
+  "%select{%4|%sub{subst_int_range}1,2}0 %3 "
+  "%select{%sub{subst_int_range}1,2|%4}0 is always %5">,
+  InGroup, DefaultIgnore;
 
 def warn_mixed_sign_comparison : Warning<
   "comparison of integers of 
diff erent signs: %0 and %1">,

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 5c2092f1447ad..bbd856e9262e0 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10792,15 +10792,16 @@ static bool CheckTautologicalComparison(Sema &S, 
BinaryOperator *E,
   S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType()))
 return false;
 
-  //

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

2020-08-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd6492d874478: Add -Wtautological-value-range-compare 
warning. (authored by rsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/tautological-constant-compare.c

Index: clang/test/Sema/tautological-constant-compare.c
===
--- clang/test/Sema/tautological-constant-compare.c
+++ clang/test/Sema/tautological-constant-compare.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify %s
@@ -545,9 +545,9 @@
   if (maybe >= e)
   return 0;
 
-  // For the time being, use the declared type of bit-fields rather than their
-  // length when determining whether a value is in-range.
-  // FIXME: Reconsider this.
+  // We only warn on out-of-range bitfields and expressions with limited range
+  // under -Wtantological-in-range-compare, not under -Wtype-limits, because
+  // the warning is not based on the type alone.
   struct A {
 int a : 3;
 unsigned b : 3;
@@ -555,13 +555,36 @@
 unsigned long d : 3;
   } a;
   if (a.a < 3) {}
-  if (a.a < 4) {}
+  if (a.a < 4) {} // #bitfield1
   if (a.b < 7) {}
-  if (a.b < 8) {}
+  if (a.b < 8) {} // #bitfield2
   if (a.c < 3) {}
-  if (a.c < 4) {}
+  if (a.c < 4) {} // #bitfield3
   if (a.d < 7) {}
-  if (a.d < 8) {}
+  if (a.d < 8) {} // #bitfield4
+#if TEST == 2
+  // expected-warning@#bitfield1 {{comparison of 3-bit signed value < 4 is always true}}
+  // expected-warning@#bitfield2 {{comparison of 3-bit unsigned value < 8 is always true}}
+  // expected-warning@#bitfield3 {{comparison of 3-bit signed value < 4 is always true}}
+  // expected-warning@#bitfield4 {{comparison of 3-bit unsigned value < 8 is always true}}
+#endif
+
+  if ((s & 0xff) < 0) {} // #valuerange1
+  if ((s & 0xff) < 1) {}
+  if ((s & -3) < -4) {} // #valuerange2
+  if ((s & -3) < -3) {}
+  if ((s & -3) < 4u) {} // (true if s non-negative)
+  if ((s & -3) > 4u) {} // (true if s negative)
+  if ((s & -3) == 4u) {} // #valuerange3 (never true)
+  if ((s & -3) == 3u) {}
+  if ((s & -3) == -5u) {} // #valuerange4
+  if ((s & -3) == -4u) {}
+#if TEST == 2
+  // expected-warning@#valuerange1 {{comparison of 8-bit unsigned value < 0 is always false}}
+  // expected-warning@#valuerange2 {{comparison of 3-bit signed value < -4 is always false}}
+  // expected-warning@#valuerange3 {{comparison of 3-bit signed value == 4 is always false}}
+  // expected-warning@#valuerange4 {{comparison of 3-bit signed value == 4294967291 is always false}}
+#endif
 
   return 1;
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10792,15 +10792,16 @@
   S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType()))
 return false;
 
-  // TODO: Investigate using GetExprRange() to get tighter bounds
-  // on the bit ranges.
+  IntRange OtherValueRange =
+  GetExprRange(S.Context, Other, S.isConstantEvaluated());
+
   QualType OtherT = Other->getType();
   if (const auto *AT = OtherT->getAs())
 OtherT = AT->getValueType();
-  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
+  IntRange OtherTypeRange = IntRange::forValueOfType(S.Context, OtherT);
 
   // Special case for ObjC BOOL on targets where its a typedef for a signed char
-  // (Namely, macOS).
+  // (Namely, macOS). FIXME: IntRange::forValueOfType should do this.
   bool IsObjCSignedCharBool = S.getLangOpts().ObjC &&
   S.NSAPIObj->isObjCBOOLType(OtherT) &&
   OtherT->isSpecificBuiltinType(BuiltinType::SChar);
@@ -10810,17 +10811,32 @@
   bool OtherIsBooleanDespiteType =
   !OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue();
   if (OtherIsBooleanDespiteType || IsObjCSignedCharBool)
-OtherRange = IntRange::forBoolType();
+OtherTypeRange = OtherValueRange =

[PATCH] D85367: [clang, test, Darwin] Fix tests expecting Darwin target

2020-08-06 Thread Steven Wu via Phabricator via cfe-commits
steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85367

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


[clang] e1cad42 - [X86] Make getX86TargetCPU return std::string instead of const char *. Remove call to MakeArgString. NFCI

2020-08-06 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2020-08-06T13:18:15-07:00
New Revision: e1cad4234cf3a3d0747c140e135e413ece22cf63

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

LOG: [X86] Make getX86TargetCPU return std::string instead of const char *. 
Remove call to MakeArgString. NFCI

I believe this function used to be called directly from X86
specific code and was used to immediately create -target-cpu
command line. A later refactoring changed it to to be called from
a generic getCPU function that returns std::string. So on some
paths we created a string using MakeArgString converted that to
std::string then called MakeArgString again from that.

Instead just return std::string directly like the other targets.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/X86.cpp
clang/lib/Driver/ToolChains/Arch/X86.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp 
b/clang/lib/Driver/ToolChains/Arch/X86.cpp
index 2cc44c09917f..5d9a7234b027 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -20,7 +20,7 @@ using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
-const char *x86::getX86TargetCPU(const ArgList &Args,
+std::string x86::getX86TargetCPU(const ArgList &Args,
  const llvm::Triple &Triple) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
 if (StringRef(A->getValue()) != "native")
@@ -33,7 +33,7 @@ const char *x86::getX86TargetCPU(const ArgList &Args,
 // with -native.
 std::string CPU = std::string(llvm::sys::getHostCPUName());
 if (!CPU.empty() && CPU != "generic")
-  return Args.MakeArgString(CPU);
+  return CPU;
   }
 
   if (const Arg *A = Args.getLastArgNoClaim(options::OPT__SLASH_arch)) {
@@ -64,7 +64,7 @@ const char *x86::getX86TargetCPU(const ArgList &Args,
   // Select the default CPU if none was given (or detection failed).
 
   if (!Triple.isX86())
-return nullptr; // This routine is only handling x86 targets.
+return ""; // This routine is only handling x86 targets.
 
   bool Is64Bit = Triple.getArch() == llvm::Triple::x86_64;
 

diff  --git a/clang/lib/Driver/ToolChains/Arch/X86.h 
b/clang/lib/Driver/ToolChains/Arch/X86.h
index 9f9c2b8c4b49..14f0a26c8be4 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.h
+++ b/clang/lib/Driver/ToolChains/Arch/X86.h
@@ -21,7 +21,7 @@ namespace driver {
 namespace tools {
 namespace x86 {
 
-const char *getX86TargetCPU(const llvm::opt::ArgList &Args,
+std::string getX86TargetCPU(const llvm::opt::ArgList &Args,
 const llvm::Triple &Triple);
 
 void getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple,



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


[clang] 4df38a5 - [X86] Optimize out a few extra strlen calls in getX86TargetCPU. NFCI

2020-08-06 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2020-08-06T13:18:15-07:00
New Revision: 4df38a5589f6fa23e161a76bdaa3180ad053791e

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

LOG: [X86] Optimize out a few extra strlen calls in getX86TargetCPU. NFCI

We had a conversion from const char * to StringRef and const char *
to std::string conversion. These both do their own
strlen call if the compiler doens't figure out how to share them.
By adding the temporary StringRef we can convert it to std::string
instead.

The other case is to use a StringSwitch instead of
StringSwitch since the output values of the switch
are string literals. This allows the length to be computed at
compile time. Otherwise we have to convert from const char *
to std::string after the StringSwitch.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/X86.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp 
b/clang/lib/Driver/ToolChains/Arch/X86.cpp
index 5d9a7234b027..c49aaadc42ae 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,41 +23,42 @@ using namespace llvm::opt;
 std::string x86::getX86TargetCPU(const ArgList &Args,
  const llvm::Triple &Triple) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native")
-  return A->getValue();
+StringRef CPU = A->getValue();
+if (CPU != "native")
+  return std::string(CPU);
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.
 //
 // FIXME: We should also incorporate the detected target features for use
 // with -native.
-std::string CPU = std::string(llvm::sys::getHostCPUName());
+CPU = llvm::sys::getHostCPUName();
 if (!CPU.empty() && CPU != "generic")
-  return CPU;
+  return std::string(CPU);
   }
 
   if (const Arg *A = Args.getLastArgNoClaim(options::OPT__SLASH_arch)) {
 // Mapping built by looking at lib/Basic's X86TargetInfo::initFeatureMap().
 StringRef Arch = A->getValue();
-const char *CPU = nullptr;
+StringRef CPU;
 if (Triple.getArch() == llvm::Triple::x86) {  // 32-bit-only /arch: flags.
-  CPU = llvm::StringSwitch(Arch)
+  CPU = llvm::StringSwitch(Arch)
 .Case("IA32", "i386")
 .Case("SSE", "pentium3")
 .Case("SSE2", "pentium4")
-.Default(nullptr);
+.Default("");
 }
-if (CPU == nullptr) {  // 32-bit and 64-bit /arch: flags.
-  CPU = llvm::StringSwitch(Arch)
+if (CPU.empty()) {  // 32-bit and 64-bit /arch: flags.
+  CPU = llvm::StringSwitch(Arch)
 .Case("AVX", "sandybridge")
 .Case("AVX2", "haswell")
 .Case("AVX512F", "knl")
 .Case("AVX512", "skylake-avx512")
-.Default(nullptr);
+.Default("");
 }
-if (CPU) {
+if (!CPU.empty()) {
   A->claim();
-  return CPU;
+  return std::string(CPU);
 }
   }
 



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


[PATCH] D79279: Add overloaded versions of builtin mem* functions

2020-08-06 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment.

In D79279#2200916 , @rsmith wrote:

> In D79279#2197176 , @rjmccall wrote:
>
>> I thought part of the point of `__builtin_memcpy` was so that C library 
>> headers could do `#define memcpy(x, y, z) __builtin_memcpy(x, y, z)`.  If 
>> so, the conformance issue touches `__builtin_memcpy` as well, not just calls 
>> to the library builtin.
>
> They would have to declare it as well (because C code can `#undef memcpy` and 
> expect to then be able to call a real function), so the `#define` would be 
> pointless.  It doesn't look like glibc does anything like this; do you know 
> of a C standard library implementation that does?
>
> If we want to follow that path, then we'll presumably (eventually) want 
> address-space-`_overloaded` versions of all lib builtins that take pointers 
> -- looks like that's around 60 functions total. That said, I do wonder how 
> many of the functions in question that we're implicitly overloading on 
> address space actually support such overloading -- certainly any of them that 
> we lower to a call to a library function is going to go wrong at runtime.
>
> +@tstellar, who added this functionality in r233706 -- what was the intent 
> here?

The goal of this patch was to avoid having to overload all the builtin with 
address spaces, which would be a lot of new builtins, but this functionality 
was added for targets that do not have a memcpy lib call, so I didn't consider 
the case where a libcall would be emitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

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


[PATCH] D84534: [AIX] Static init frontend recovery and backend support

2020-08-06 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L updated this revision to Diff 283712.
Xiangling_L marked an inline comment as done.

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

https://reviews.llvm.org/D84534

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  clang/unittests/CodeGen/IncrementalProcessingTest.cpp
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
  llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
  llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
  llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll

Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+  ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll
@@ -0,0 +1,12 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@v = global i8 0
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+
+define void @foo() {
+  ret void
+}
+
+// CHECK: LLVM ERROR: associated data of XXStructor list is not yet supported on AIX
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,60 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+  ret i32 3
+}
+
+define internal void @init1() {
+  ret void
+}
+
+define internal void @destruct1() {
+  ret void
+}
+
+define internal void @init2() {
+  ret void
+}
+
+define internal void @destruct2() {
+  ret void
+}
+
+; CHECK:   .lglobl	init1[DS]
+; CHECK:   .lglobl	.init1
+; CHECK:   .csect init1[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @init1
+; CHECK: .init1:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK:   .lglobl	destruct1[DS]
+; CHECK:   .lglobl	.destruct1
+; CHECK:   .csect destruct1[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @destruct1
+; CHECK: .destruct1:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK:   .lglobl	init2[DS]
+; CHECK:   .lglobl	.init2
+; CHECK:   .csect init2[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @init2
+; CHECK: .init2:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+; CHECK:   .lglobl	destruct2[DS]
+; CHECK:   .lglobl	.destruct2
+; CHECK:   .csect destruct2[DS]
+; CHECK: __sterm8000_c

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

2020-08-06 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L marked 3 inline comments as done.
Xiangling_L added inline comments.



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+  bool preprocessStructorList(const DataLayout &DL, const Constant *List,
+  SmallVector &Structors);

jasonliu wrote:
> Xiangling_L wrote:
> > jasonliu wrote:
> > > Xiangling_L wrote:
> > > > jasonliu wrote:
> > > > > A doxygen comment describe what this function does, and what its 
> > > > > return value means, and mention `Structors` is an output argument.
> > > > > By looking at what this function does, it seems `buildStructorList` 
> > > > > is a better name.
> > > > I meant to and should've named this function to 
> > > > `preprocessXXStructorList`, let me know if you still prefer 
> > > > `buildStructorList`. And if you do, since the underneath of 
> > > > `SmallVector` is a variable-sized array, maybe we should try 
> > > > `buildSortedStructorArray`?
> > > `preprocess` sounds like we are already having a XXStructorList and now 
> > > we try to do something on it. 
> > > But right now, we are actually passing in an empty StructorList/Array and 
> > > build it from scratch. So I would still prefer the name of `build` in it.
> > > I don't mind changing to a more accurate name as you suggested. 
> > I think we do have a `XXStructorList` here which is the initializer of 
> > `llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is  
> > consistent with other spots.
> My understanding is that before we enter this `preprocessXXStructorList`, we 
> do not have a list of XXStructor. We only have a list of `Constant`. This 
> functions turns a list of `Constant` to a list of `XXStructor`. 
Just leave a record here, as we discussed offline, we agree that the meaning of 
term `XXStructorList` is `the initializer of `llvm.gloal_ctors` or 
`llvm.gloal_dtors` array. So I will keep using `preprocessXXStructorList` as 
the function name.


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

https://reviews.llvm.org/D84534

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


[PATCH] D79279: Add overloaded versions of builtin mem* functions

2020-08-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D79279#2197176 , @rjmccall wrote:

> I thought part of the point of `__builtin_memcpy` was so that C library 
> headers could do `#define memcpy(x, y, z) __builtin_memcpy(x, y, z)`.  If so, 
> the conformance issue touches `__builtin_memcpy` as well, not just calls to 
> the library builtin.

They would have to declare it as well (because C code can `#undef memcpy` and 
expect to then be able to call a real function), so the `#define` would be 
pointless.  It doesn't look like glibc does anything like this; do you know of 
a C standard library implementation that does?

If we want to follow that path, then we'll presumably (eventually) want 
address-space-`_overloaded` versions of all lib builtins that take pointers -- 
looks like that's around 60 functions total. That said, I do wonder how many of 
the functions in question that we're implicitly overloading on address space 
actually support such overloading -- certainly any of them that we lower to a 
call to a library function is going to go wrong at runtime.

+@tstellar, who added this functionality in r233706 -- what was the intent here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279

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


[PATCH] D85471: Make clang HIP headers compatible with C++98

2020-08-06 Thread Siu Chi Chan via Phabricator via cfe-commits
scchan created this revision.
Herald added subscribers: cfe-commits, yaxunl.
Herald added a project: clang.
scchan requested review of this revision.

Automation to detect compiler features, such as CMake's 
target_compile_features, 
would attempt to detect compiler features by explicitly using langugage flags.  
This change ensures that the HIP headers would still work with C++98.

Change-Id: I381f76fdaf42196d9272efeaa5a3a159edca0ab1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85471

Files:
  clang/lib/Headers/__clang_hip_libdevice_declares.h
  clang/lib/Headers/__clang_hip_math.h
  clang/lib/Headers/__clang_hip_runtime_wrapper.h

Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
===
--- clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -28,6 +28,10 @@
 #define __shared__ __attribute__((shared))
 #define __constant__ __attribute__((constant))
 
+#if !defined(__cplusplus) || __cplusplus < 201103L
+  #define nullptr NULL;
+#endif
+
 #if __HIP_ENABLE_DEVICE_MALLOC__
 extern "C" __device__ void *__hip_malloc(size_t __size);
 extern "C" __device__ void *__hip_free(void *__ptr);
Index: clang/lib/Headers/__clang_hip_math.h
===
--- clang/lib/Headers/__clang_hip_math.h
+++ clang/lib/Headers/__clang_hip_math.h
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #pragma push_macro("__DEVICE__")
 #pragma push_macro("__RETURN_TYPE")
@@ -22,6 +23,34 @@
 #define __DEVICE__ static __device__
 #define __RETURN_TYPE bool
 
+#if defined (__cplusplus) && __cplusplus < 201103L
+//emulate static_assert on type sizes
+template
+struct __compare_result{};
+template<>
+struct __compare_result {
+  static const bool valid;
+};
+
+__DEVICE__
+inline void __suppress_unused_warning(bool b) {};
+template
+__DEVICE__
+inline void __static_assert_equal_size() {
+__suppress_unused_warning(__compare_result::valid);
+}
+
+#define __static_assert_type_size_equal(A, B) \
+  __static_assert_equal_size()
+
+#else
+
+#define __static_assert_type_size_equal(A,B) \
+  static_assert((A) == (B), "")
+
+#endif
+
+
 __DEVICE__
 inline uint64_t __make_mantissa_base8(const char *__tagp) {
   uint64_t __r = 0;
@@ -252,9 +281,8 @@
   uint32_t exponent : 8;
   uint32_t sign : 1;
 } bits;
-
-static_assert(sizeof(float) == sizeof(struct ieee_float), "");
   } __tmp;
+  __static_assert_type_size_equal(sizeof(__tmp.val), sizeof(__tmp.bits));
 
   __tmp.bits.sign = 0u;
   __tmp.bits.exponent = ~0u;
@@ -716,8 +744,8 @@
   uint32_t exponent : 11;
   uint32_t sign : 1;
 } bits;
-static_assert(sizeof(double) == sizeof(struct ieee_double), "");
   } __tmp;
+  __static_assert_type_size_equal(sizeof(__tmp.val), sizeof(__tmp.bits));
 
   __tmp.bits.sign = 0u;
   __tmp.bits.exponent = ~0u;
@@ -726,7 +754,7 @@
 
   return __tmp.val;
 #else
-  static_assert(sizeof(uint64_t) == sizeof(double));
+  __static_assert_type_size_equal(sizeof(uint64_t), sizeof(double));
   uint64_t val = __make_mantissa(__tagp);
   val |= 0xFFF << 51;
   return *reinterpret_cast(&val);
Index: clang/lib/Headers/__clang_hip_libdevice_declares.h
===
--- clang/lib/Headers/__clang_hip_libdevice_declares.h
+++ clang/lib/Headers/__clang_hip_libdevice_declares.h
@@ -318,7 +318,7 @@
 __device__ inline __2f16
 __llvm_amdgcn_rcp_2f16(__2f16 __x) // Not currently exposed by ROCDL.
 {
-  return (__2f16){__llvm_amdgcn_rcp_f16(__x.x), __llvm_amdgcn_rcp_f16(__x.y)};
+  return (__2f16)(__llvm_amdgcn_rcp_f16(__x.x), __llvm_amdgcn_rcp_f16(__x.y));
 }
 __device__ __attribute__((const)) __2f16 __ocml_rint_2f16(__2f16);
 __device__ __attribute__((const)) __2f16 __ocml_rsqrt_2f16(__2f16);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-06 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision.
arsenm added a comment.

30eeb742f1d11d7a7036e3b8a3bffc1dfd252082 



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

https://reviews.llvm.org/D79744

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


[clang] 30eeb74 - clang: Use byref for aggregate kernel arguments

2020-08-06 Thread Matt Arsenault via cfe-commits

Author: Matt Arsenault
Date: 2020-08-06T15:52:26-04:00
New Revision: 30eeb742f1d11d7a7036e3b8a3bffc1dfd252082

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

LOG: clang: Use byref for aggregate kernel arguments

Add address space to indirect abi info and use it for kernels.

Previously, indirect arguments assumed assumed a stack passed object
in the alloca address space using byval. A stack pointer is unsuitable
for kernel arguments, which are passed in a separate, constant buffer
with a different address space.

Start using the new byref for aggregate kernel arguments. Previously
these were emitted as raw struct arguments, and turned into loads in
the backend. These will lower identically, although with byref you now
have the option of applying an explicit alignment. In the future, a
reasonable implementation would use byref for all kernel arguments
(this would be a practical problem at the moment due to losing things
like noalias on pointer arguments).

This is mostly to avoid fighting the optimizer's treatment of
aggregate load/store. SROA and instcombine both turn aggregate loads
and stores into a long sequence of element loads and stores, rather
than the optimizable memcpy I would expect in this situation. Now an
explicit memcpy will be introduced up-front which is better understood
and helps eliminate the alloca in more situations.

This skips using byref in the case where HIP kernel pointer arguments
in structs are promoted to global pointers. At minimum an additional
patch is needed to allow coercion with indirect arguments. This also
skips using it for OpenCL due to the current workaround used to
support kernels calling kernels. Distinct function bodies would need
to be generated up front instead of emitting an illegal call.

Added: 


Modified: 
clang/include/clang/CodeGen/CGFunctionInfo.h
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGenCUDA/kernel-args.cu
clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl

Removed: 




diff  --git a/clang/include/clang/CodeGen/CGFunctionInfo.h 
b/clang/include/clang/CodeGen/CGFunctionInfo.h
index eaf5a3d5aad7..253ef946ce15 100644
--- a/clang/include/clang/CodeGen/CGFunctionInfo.h
+++ b/clang/include/clang/CodeGen/CGFunctionInfo.h
@@ -44,10 +44,23 @@ class ABIArgInfo {
 /// but also emit a zero/sign extension attribute.
 Extend,
 
-/// Indirect - Pass the argument indirectly via a hidden pointer
-/// with the specified alignment (0 indicates default alignment).
+/// Indirect - Pass the argument indirectly via a hidden pointer with the
+/// specified alignment (0 indicates default alignment) and address space.
 Indirect,
 
+/// IndirectAliased - Similar to Indirect, but the pointer may be to an
+/// object that is otherwise referenced.  The object is known to not be
+/// modified through any other references for the duration of the call, and
+/// the callee must not itself modify the object.  Because C allows
+/// parameter variables to be modified and guarantees that they have unique
+/// addresses, the callee must defensively copy the object into a local
+/// variable if it might be modified or its address might be compared.
+/// Since those are uncommon, in principle this convention allows programs
+/// to avoid copies in more situations.  However, it may introduce *extra*
+/// copies if the callee fails to prove that a copy is unnecessary and the
+/// caller naturally produces an unaliased object for the argument.
+IndirectAliased,
+
 /// Ignore - Ignore the argument (treat as void). Useful for void and
 /// empty structs.
 Ignore,
@@ -86,6 +99,7 @@ class ABIArgInfo {
 unsigned AllocaFieldIndex; // isInAlloca()
   };
   Kind TheKind;
+  unsigned IndirectAddrSpace : 24; // isIndirect()
   bool PaddingInReg : 1;
   bool InAllocaSRet : 1;// isInAlloca()
   bool InAllocaIndirect : 1;// isInAlloca()
@@ -97,7 +111,8 @@ class ABIArgInfo {
   bool SignExt : 1; // isExtend()
 
   bool canHavePaddingType() const {
-return isDirect() || isExtend() || isIndirect() || isExpand();
+return isDirect() || isExtend() || isIndirect() || isIndirectAliased() ||
+   isExpand();
   }
   void setPaddingType(llvm::Type *T) {
 assert(canHavePaddingType());
@@ -112,9 +127,10 @@ class ABIArgInfo {
 public:
   ABIArgInfo(Kind K = Direct)
   : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0), TheKind(K),
-PaddingInReg(false), InAllocaSRet(false), InAllocaIndirect(false),
-IndirectByVal(false), IndirectRealign(false), SRetAfterThis(false),
-InReg(false), CanBeFlattened(false), SignExt(false) {}
+IndirectAddrSpace(0), PaddingInReg(false), InAlloc

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

The diagnostic we get for the case of three or more comparison operators here 
doesn't seem ideal. Perhaps we could do this check as part of the 
`SemaChecking` pass over the completed expression rather than doing it as we 
form the individual comparisons? That way we'll have the contextual information 
necessary to find the complete chained comparison; we'd also be able to detect 
the special case where the operator sequence is the operand of an `&`/`|`/`^` 
so that we need parentheses in the fixit.




Comment at: clang/docs/DiagnosticsReference.rst:1-5
 ..
   ---
   NOTE: This file is automatically generated by running clang-tblgen
   -gen-diag-docs. Do not edit this file by hand!!
   ---

Please note this :)

You can revert the changes to this file; it's auto-generated. (We only check in 
a version because the clang website infrastructure isn't yet able to generate 
it on-demand.)



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6137
+def note_compare_with_parens : Note<
+  "place parentheses around the '%0' comparison to silence this warning">;
+def note_compare_seperate_sides : Note<

If you're going to quote the operator here, you should prepend "first" or 
similar if both operators are the same.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6139
+def note_compare_seperate_sides : Note<
+  "seperate the expression into two clauses to give it the intended 
mathematical meaning">;
+

Quuxplusone wrote:
> Both in the string and in the identifier, s/seperate/separate/.
> I would also s/sides/clauses/ or s/sides/expression/ just to avoid giving too 
> many different names to the same entity.
Don't say "intended". We don't know the intent.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6135
+  "comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'">,
+  InGroup, DefaultIgnore;
+

vabridgers wrote:
> Quuxplusone wrote:
> > Why is this `x<=y<=z` instead of the simpler `x > "half-open range" common case)?
> > IMHO you should mention the name "chained comparisons" here, since I think 
> > that's what people coming from such languages will understand.
> Thanks Arthur. I modeled the warning message after gcc's warning message. We 
> found internally that while gcc detected this, clang did not. See 
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#Warning-Options ...
> 
> -Wparentheses
> Warn if parentheses are omitted in certain contexts, such as when there is an 
> assignment in a context where a truth value is expected, or when operators 
> are nested whose precedence people often get confused about.
> 
> Also warn if a comparison like x<=y<=z appears; this is equivalent to (x<=y ? 
> 1 : 0) <= z, which is a different interpretation from that of ordinary 
> mathematical notation.
> ...
> 
> While this is the gcc documentation, we can craft whatever message we see fit 
> at this point in time :)  I'll add "chained" comparison for this next update, 
> we can tailor the message as we see fit. Thanks!
> 
Per our diagnostic best practices 
(http://clang.llvm.org/docs/InternalsManual.html#the-format-string), don't 
repeat the code in the diagnostic (or even an analogy of it such as 'x<=y<=z'); 
instead, consider underlining the chained comparison operators in the snippet. 
Also keep in mind that the developer will see the diagnostic and the notes 
together; you can use the notes to help clarify the meaning of the diagnostic:

```
error: chained comparison does not have its mathematical meaning
  if (a <= b < c)
 ^~   ~
note: perform two separate comparisons to compare the same operand twice
  if (a <= b < c)
 ^
 && b
note: place parentheses around the first comparison to compare against its 
'bool' result and silence this warning
  if (a <= b < c)
  ^
  ( )
```



Comment at: clang/lib/Sema/SemaExpr.cpp:14008-14010
+// Accepts (x ['<'|'<='|'>'|'>='] y ['<'|'<='|'>'|'>='] z), suggests:
+// 1) ((x ['<'|'<='|'>'|'>='] y) ['<'|'<='|'>'|'>='] z), or
+// 2) (x ['<'|'<='|'>'|'>='] (y ['<'|'<='|'>'|'>='] z)). or

Why not also `==` and `!=` here? `a < b == c < d` is a perfectly reasonable 
mathematical equation (modulo the spelling of `==`), and it would seem sensible 
to catch that with the same diagnostic.



Comment at: clang/lib/Sema/SemaExpr.cpp:14031
+
+  Self.Diag(LHSExpr->getBeginLoc(), diag::note_compare_seperate_sides)
+  << FixItHint::CreateInsertion(LHSExpr->getBeginLoc(), "(")

I think we should consider omitting the fixit if the `y` expression is long or 
has side-effects.



Comment at: clang/lib/Sema/SemaExpr.cpp:14032-14035
+  << 

[PATCH] D85247: [Darwin] [Driver] Clang should invoke dsymutil for multiarch builds

2020-08-06 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments.



Comment at: clang/test/Driver/darwin-dsymutil.c:74
+// CHECK-DSYMUTIL-MULTIARCH: "/usr/bin/ld" "-demangle" "-object_path_lto"
+// CHECK-DSYMUTIL-MULTIARCH: "/usr/bin/dsymutil" "-o" "a.out.dSYM" "a.out"

Is a.out the `lipo`ed fat binary? If yes this LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85247

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


[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Are you seriously adding an attribute to literally every argument and return 
value?  Why is this the right representation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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


[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

2020-08-06 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added a comment.

If changes like this are required for all these tests this is going to be a 
complete pain for downstream forks like ours. At the very least, make whatever 
script you used to update these public, as I don't want to have to recreate it 
from scratch when merging this in. I had enough "fun" with the LLD 
mass-renaming (UpperCamel -> lowerCamel) and that was _with_ a 
supposedly-working script (it didn't quite do the right thing and I seem to 
recall there being two refactoring commits, only one of which had a script); I 
do not want a repeat of that experience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317

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


[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-06 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM.


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

https://reviews.llvm.org/D79744

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


[PATCH] D84534: [AIX] Static init frontend recovery and backend support

2020-08-06 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+  bool preprocessStructorList(const DataLayout &DL, const Constant *List,
+  SmallVector &Structors);

Xiangling_L wrote:
> jasonliu wrote:
> > Xiangling_L wrote:
> > > jasonliu wrote:
> > > > A doxygen comment describe what this function does, and what its return 
> > > > value means, and mention `Structors` is an output argument.
> > > > By looking at what this function does, it seems `buildStructorList` is 
> > > > a better name.
> > > I meant to and should've named this function to 
> > > `preprocessXXStructorList`, let me know if you still prefer 
> > > `buildStructorList`. And if you do, since the underneath of `SmallVector` 
> > > is a variable-sized array, maybe we should try `buildSortedStructorArray`?
> > `preprocess` sounds like we are already having a XXStructorList and now we 
> > try to do something on it. 
> > But right now, we are actually passing in an empty StructorList/Array and 
> > build it from scratch. So I would still prefer the name of `build` in it.
> > I don't mind changing to a more accurate name as you suggested. 
> I think we do have a `XXStructorList` here which is the initializer of 
> `llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is  
> consistent with other spots.
My understanding is that before we enter this `preprocessXXStructorList`, we do 
not have a list of XXStructor. We only have a list of `Constant`. This 
functions turns a list of `Constant` to a list of `XXStructor`. 



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107
+  if (!isa(List))
+return false;
 

Xiangling_L wrote:
> jasonliu wrote:
> > Return of boolean seems unnecessary. 
> > Callee could check the size of the Structors to decide if they want an 
> > early return or not (or in this particular case, the for loop would just do 
> > nothing and no need for extra condition if you don't mind the call to 
> > getPointerPrefAlignment or assign to 0 to Index)?
> > So we could just return void for this function?
> Yeah, we could do that. But it looks a boolean return value makes the code 
> flow natural. And if any target does want to control the early return in the 
> future, it's flexbile for them to do that. I am wondering is there any big 
> difference between bool and void return value for this function? 
If target need to control early return in the future, they could still do so by 
querying if the output `Structors` is empty or not. 
I just don't want unnecessary returns, as it's one more thing that user of the 
function need to care about when they examine this function. And the user of 
this function would have the same question of why we need to return a boolean 
here.


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

https://reviews.llvm.org/D84534

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


[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-06 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6139
+def note_compare_seperate_sides : Note<
+  "seperate the expression into two clauses to give it the intended 
mathematical meaning">;
+

Both in the string and in the identifier, s/seperate/separate/.
I would also s/sides/clauses/ or s/sides/expression/ just to avoid giving too 
many different names to the same entity.



Comment at: clang/lib/Sema/SemaExpr.cpp:14034
+  << FixItHint::CreateInsertion(LHSExpr->getEndLoc(), ") && ")
+  << FixItHint::CreateInsertion(LHSBO->getRHS()->getBeginLoc(), "(y ")
+  << FixItHint::CreateInsertion(RHSExpr->getEndLoc(), ")");

vabridgers wrote:
> njames93 wrote:
> > You don't want to insert `y` but the source code for `y`
> Gotcha, I'll address.
Please make sure to add at least one regression test to catch this (i.e. some 
test that tests the wording of the fixit and uses a name for the middle term 
that is not `y`). I don't know how to test a fixit, but there must be a way.



Comment at: clang/lib/Sema/SemaExpr.cpp:14211
+
+  // Warn on ambiguous compares, loosely x<=y<=z
+  if (BinaryOperator::isRelationalOp(Opc))

FWIW, I would s/loosely/such as/.



Comment at: clang/lib/Sema/SemaExpr.cpp:14010
+  << Bop->getSourceRange() << OpLoc;
+  SuggestParentheses(Self, Bop->getOperatorLoc(),
+ Self.PDiag(diag::note_precedence_silence)

vabridgers wrote:
> vabridgers wrote:
> > njames93 wrote:
> > > Quuxplusone wrote:
> > > > vabridgers wrote:
> > > > > vabridgers wrote:
> > > > > > lebedev.ri wrote:
> > > > > > > Should we also suggest the fix to rewrite into what user likely 
> > > > > > > intended?
> > > > > > > `(x op1 y) && (y op2 z)`
> > > > > > I'll work on this, post in a next update. Thank you!
> > > > > Hi @lebedev.ri , the warning emits a note that says "place 
> > > > > parentheses around the '' expression to silence this warning" 
> > > > > (see the test cases below). Is this sufficient, or are you looking 
> > > > > for something else?
> > > > https://godbolt.org/z/d1dG1o
> > > > For the very similar situation `(x & 1 == 0)`, Clang suggests //two 
> > > > different fixits//, and I believe Roman was suggesting that you should 
> > > > do the same kind of thing here.
> > > > ```
> > > > :3:16: warning: & has lower precedence than ==; == will be 
> > > > evaluated first [-Wparentheses]
> > > > int y = (x & 1 == 0);
> > > >^~~~
> > > > :3:16: note: place parentheses around the '==' expression to 
> > > > silence this warning
> > > > int y = (x & 1 == 0);
> > > >^
> > > >  ( )
> > > > :3:16: note: place parentheses around the & expression to 
> > > > evaluate it first
> > > > int y = (x & 1 == 0);
> > > >^
> > > >  ()
> > > > ```
> > > > For our situation here it would be something like
> > > > ```
> > > > :3:16: warning: chained comparisons like 'x<=y<=z' are 
> > > > interpreted as '(x<=y ? 1 : 0) <= z' [-Wcompare-op-parentheses]
> > > > int w = (x < y < z);
> > > >^~~~
> > > > :3:16: note: place parentheses around the first '<' expression 
> > > > to silence this warning
> > > > int w = (x < y < z);
> > > >  ^
> > > >  ()
> > > > :3:16: note: separate the expression into two clauses to give 
> > > > it the mathematical meaning
> > > > int y = (x < y < z);
> > > >^
> > > >  () && (y)
> > > > ```
> > > > Watch out that you don't suggest a wrong fixit for `(0 <= sideeffect() 
> > > > < 10)`, though. I seem to recall another warning that recently had a 
> > > > lot of trouble phrasing its fixit in a way that wouldn't invoke UB or 
> > > > change the behavior of the code in corner cases, but I forget the 
> > > > details.
> > > Surely you'd just need to check `y` for side effects before creating the 
> > > fix-it.
> > Still working on these, thanks
> still working on these, thanks
My understanding is that option (2) `(x <= (y <= z))` is never suggested. It's 
reasonable not to suggest it, because it is neither "what the compiler sees, 
elucidated for the human reader" nor "what the programmer meant, corrected for 
the compiler." However, as it's not suggested, it shouldn't be documented in 
this comment.



Comment at: clang/test/Misc/warning-wall.c:86
 CHECK-NEXT:  -Wparentheses
+CHECK-NEXT:-Wcompare-op-parentheses
 CHECK-NEXT:-Wlogical-op-parentheses

My impression is that you (Vincent) are tending toward just adding `x == y == 
z` and `x <= y == z` to this same `-Wcompare-op-parentheses`. That's also my 
preference (except that I wish it could be done all together in one big pull 
request instead of being split up and perhaps deferred).  Have we conclusively 
established t

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

2020-08-06 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L marked 5 inline comments as done.
Xiangling_L added inline comments.



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+  bool preprocessStructorList(const DataLayout &DL, const Constant *List,
+  SmallVector &Structors);

jasonliu wrote:
> Xiangling_L wrote:
> > jasonliu wrote:
> > > A doxygen comment describe what this function does, and what its return 
> > > value means, and mention `Structors` is an output argument.
> > > By looking at what this function does, it seems `buildStructorList` is a 
> > > better name.
> > I meant to and should've named this function to `preprocessXXStructorList`, 
> > let me know if you still prefer `buildStructorList`. And if you do, since 
> > the underneath of `SmallVector` is a variable-sized array, maybe we should 
> > try `buildSortedStructorArray`?
> `preprocess` sounds like we are already having a XXStructorList and now we 
> try to do something on it. 
> But right now, we are actually passing in an empty StructorList/Array and 
> build it from scratch. So I would still prefer the name of `build` in it.
> I don't mind changing to a more accurate name as you suggested. 
I think we do have a `XXStructorList` here which is the initializer of 
`llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is  
consistent with other spots.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107
+  if (!isa(List))
+return false;
 

jasonliu wrote:
> Return of boolean seems unnecessary. 
> Callee could check the size of the Structors to decide if they want an early 
> return or not (or in this particular case, the for loop would just do nothing 
> and no need for extra condition if you don't mind the call to 
> getPointerPrefAlignment or assign to 0 to Index)?
> So we could just return void for this function?
Yeah, we could do that. But it looks a boolean return value makes the code flow 
natural. And if any target does want to control the early return in the future, 
it's flexbile for them to do that. I am wondering is there any big difference 
between bool and void return value for this function? 


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

https://reviews.llvm.org/D84534

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


[clang-tools-extra] 9f24148 - [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

2020-08-06 Thread Aleksandr Platonov via cfe-commits

Author: Aleksandr Platonov
Date: 2020-08-06T21:45:21+03:00
New Revision: 9f24148b212698aca220ac923d215c2073e443ce

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

LOG: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

Inside clangd, clang-tidy checks don't see preprocessor events in the preamble.
This leads to `Token::PtrData == nullptr` for tokens that the macro is defined 
to.
E.g. `#define SIGTERM 15`:
- Token::Kind == tok::numeric_constant (Token::isLiteral() == true)
- Token::UintData == 2
- Token::PtrData == nullptr

As the result of this, bugprone-bad-signal-to-kill-thread check crashes at 
null-dereference inside clangd.

Reviewed By: hokein

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
index 0720c7e689a1..3833640b3de3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,7 +39,7 @@ void BadSignalToKillThreadCheck::check(const 
MatchFinder::MatchResult &Result) {
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
-if (!T.isLiteral())
+if (!T.isLiteral() || !T.getLiteralData())
   return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index fcc6d40a988a..e8757079c675 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -438,6 +438,21 @@ TEST(DiagnosticTest, 
ClangTidySuppressionCommentTrumpsWarningAsError) {
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
+  Annotations Main(R"cpp(
+#define SIGTERM 15
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:



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


[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

2020-08-06 Thread Aleksandr Platonov via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f24148b2126: [clangd] Fix crash in 
bugprone-bad-signal-to-kill-thread clang-tidy check. (authored by ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85417

Files:
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -438,6 +438,21 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
+  Annotations Main(R"cpp(
+#define SIGTERM 15
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,7 +39,7 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
-if (!T.isLiteral())
+if (!T.isLiteral() || !T.getLiteralData())
   return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -438,6 +438,21 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
+  Annotations Main(R"cpp(
+#define SIGTERM 15
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -39,7 +39,7 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
-if (!T.isLiteral())
+if (!T.isLiteral() || !T.getLiteralData())
   return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

2020-08-06 Thread Peter Smith via Phabricator via cfe-commits
psmith added a comment.

I agree it's not what I'd like. I'm not sure how to react to MaskRays comment. 
The top of the ClangCommandLineReference.rst says:

  ---
  NOTE: This file is automatically generated by running clang-tblgen
  -gen-opt-docs. Do not edit this file by hand!!
  ---

This uses Options.td to generate the file. I don't know how true this is 
anymore given that both files are checked in. They may have been separated. 
I've seen at least one instance in the log where the whole file has been 
regenerated.

I'm happy to revert the Options.td change if there is another way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85239

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


[PATCH] D85453: [PowerPC] Implement __int128 vector divide operations

2020-08-06 Thread Albion Fung via Phabricator via cfe-commits
Conanap created this revision.
Conanap added reviewers: PowerPC, saghir, nemanjai, hfinkel.
Conanap added projects: LLVM, clang, PowerPC.
Herald added a subscriber: kbarton.
Conanap requested review of this revision.
Herald added a subscriber: wuzish.

This patch implements __int128 vector divide operations for ISA3.1.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85453

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-divide.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-divide.ll
===
--- llvm/test/CodeGen/PowerPC/p10-vector-divide.ll
+++ llvm/test/CodeGen/PowerPC/p10-vector-divide.ll
@@ -49,3 +49,21 @@
   %div = sdiv <4 x i32> %a, %b
   ret <4 x i32> %div
 }
+
+define <1 x i128> @test_vdivsq(<1 x i128> %x, <1 x i128> %y) nounwind readnone {
+; CHECK-LABEL: test_vdivsq:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vdivsq v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = sdiv <1 x i128> %x, %y
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vdivuq(<1 x i128> %x, <1 x i128> %y) nounwind readnone {
+; CHECK-LABEL: test_vdivuq:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vdivuq v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = udiv <1 x i128> %x, %y
+  ret <1 x i128> %tmp
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -1124,9 +1124,11 @@
(ins vrrc:$vA, vrrc:$vB, vrrc:$vC),
"vmsumcud $vD, $vA, $vB, $vC", IIC_VecGeneral, []>;
   def VDIVSQ : VXForm_1<267, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
-"vdivsq $vD, $vA, $vB", IIC_VecGeneral, []>;
+"vdivsq $vD, $vA, $vB", IIC_VecGeneral,
+[(set v1i128:$vD, (sdiv v1i128:$vA, v1i128:$vB))]>;
   def VDIVUQ : VXForm_1<11, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
-"vdivuq $vD, $vA, $vB", IIC_VecGeneral, []>;
+"vdivuq $vD, $vA, $vB", IIC_VecGeneral,
+[(set v1i128:$vD, (udiv v1i128:$vA, v1i128:$vB))]>;
   def VDIVESQ : VXForm_1<779, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
  "vdivesq $vD, $vA, $vB", IIC_VecGeneral, []>;
   def VDIVEUQ : VXForm_1<523, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -836,6 +836,8 @@
   setOperationAction(ISD::SREM, MVT::v2i64, Legal);
   setOperationAction(ISD::UREM, MVT::v4i32, Legal);
   setOperationAction(ISD::SREM, MVT::v4i32, Legal);
+  setOperationAction(ISD::UDIV, MVT::v1i128, Legal);
+  setOperationAction(ISD::SDIV, MVT::v1i128, Legal);
 }
 
 setOperationAction(ISD::MUL, MVT::v8i16, Legal);
Index: clang/test/CodeGen/builtins-ppc-p10vector.c
===
--- clang/test/CodeGen/builtins-ppc-p10vector.c
+++ clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -17,6 +17,7 @@
 vector unsigned int vuia, vuib, vuic;
 vector signed long long vslla, vsllb;
 vector unsigned long long vulla, vullb, vullc;
+vector signed __int128 vsi128a, vsi128b;
 vector unsigned __int128 vui128a, vui128b, vui128c;
 vector float vfa, vfb;
 vector double vda, vdb;
@@ -61,6 +62,18 @@
   return vec_div(vulla, vullb);
 }
 
+vector unsigned __int128 test_vec_div_u128(void) {
+  // CHECK: udiv <1 x i128>
+  // CHECK-NEXT: ret <1 x i128>
+  return vec_div(vui128a, vui128b);
+}
+
+vector signed __int128 test_vec_div_s128(void) {
+  // CHECK: sdiv <1 x i128>
+  // CHECK-NEXT: ret <1 x i128>
+  return vec_div(vsi128a, vsi128b);
+}
+
 vector signed int test_vec_mod_si(void) {
   // CHECK: srem <4 x i32>
   // CHECK-NEXT: ret <4 x i32>
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -3288,6 +3288,18 @@
 }
 #endif
 
+#ifdef __POWER10_VECTOR__
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_div(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return __a / __b;
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_div(vector signed __int128 __a, vector signed __int128 __b) {
+  return __a / __b;
+}
+#endif __POWER10_VECTOR__
+
 /* vec_dss */
 
 #define vec_dss __builtin_altivec_dss
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

2020-08-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84458

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


[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

2020-08-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80263

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


[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

2020-08-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a reviewer: theraven.
rjmccall added a comment.

One thing that's come up so far: you generally need to be looking through 
non-runtime protocols, not ignoring them.  This matters when non-runtime 
protocols inherit from ordinary protocols.  It may be useful to provide a 
generic function that walks an array of protocols and calls a callback with the 
unique ordinary protocols it implies.

You should also implement this for non-Apple runtimes, which should be 
straightforward with that generic function; it's just a matter of testing it.  
CC'ing David Chisnall.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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


[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

2020-08-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:839-840
 
+  // Skip host-only functions in the CUDA device compilation and device-only
+  // functions in the host compilation.
+  if (CGM.getLangOpts().CUDA &&

hliao wrote:
> tra wrote:
> > hliao wrote:
> > > tra wrote:
> > > > We will still have around some functions that may never be used on the 
> > > > host side (HD functions referenced from device code only).  I'm not 
> > > > sure if that's a problem for profiling, though. I wonder if we can 
> > > > somehow tie `skipRegionMappingForDecl` to whether we've actually 
> > > > codegen'ed the function. 
> > > Skipping wrong-side functions here just makes the report not confusing as 
> > > these functions are not emitted at all and are supposed never running on 
> > > the host/device side. If we still create the mapping for them, e.g., we 
> > > may report they have 0 runs instead of reporting nothing (just like 
> > > comments between function.) That looks a little bit confusing.
> > > It seems the current PGO adds everything for coverage mapping and late 
> > > prune them based on checks here. Just try to follow that logic to skip 
> > > wrong-side functions. If we need to revise the original logic and 
> > > generate coverage mapping for emitted functions only, the change here is 
> > > unnecessary.
> > I'd add a comment here that this 'filter' is just a rough best-effort 
> > approximation that still allows some effectively device-only Decls through.
> > The output should still be correct, even though the functions will never be 
> > used. Maybe add a TODO to deal with it if/when we know if the Decl was 
> > codegen'ed.
> > 
> Add that comment. But, I tend to not deal that "effectively" 
> host-only/device-only ones as that should be developers' responsibility to 
> handle them. The additional zero coverage mapping may be useful as well. If a 
> function is really device-only but is attributed with HD, the 0 coverage may 
> help developers correcting them.
It will be rather noisy in practice. A lot of code has either has been written 
for NVCC or has to compile with it. NVCC does not have target overloads, so 
sticking HD everywhere  is pretty much the only practical way to do it in 
complicated enough C++ code.  Anything that uses Eigen or Thrust will have tons 
of HD functions that are actually used only on one side. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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


[PATCH] D85450: Support the standards-based dates for __has_c_attribute

2020-08-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added a reviewer: rsmith.
Herald added a subscriber: krytarowski.
aaron.ballman requested review of this revision.

WG14 N2481 was adopted with minor modifications at this week's WG14 meetings. 
The only modification to the paper was to correct the date for the `deprecated` 
attribute to be `201904L` (the corrected date value will be present in WG14 
N2553 when it gets published).

Clang already supported __has_c_attribute, but was missing support for 
returning specific values for the dates when an attribute was adopted while 
waiting to see if WG14 wanted that functionality. Now that WG14 has accepted 
the proposal for C2x, this patch adds the dates from the paper.


https://reviews.llvm.org/D85450

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Preprocessor/has_c_attribute.c
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3064,18 +3064,22 @@
 // attribute version information should be taken from the SD-6 standing
 // document, which can be found at:
 // https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations
+//
+// C2x-style attributes have the same kind of version information
+// associated with them. The unscoped attribute version information should
+// be taken from the specification of the attribute in the C Standard.
 int Version = 1;
 
-if (Variety == "CXX11") {
-std::vector Spellings = Attr->getValueAsListOfDefs("Spellings");
-for (const auto &Spelling : Spellings) {
-  if (Spelling->getValueAsString("Variety") == "CXX11") {
-Version = static_cast(Spelling->getValueAsInt("Version"));
-if (Scope.empty() && Version == 1)
-  PrintError(Spelling->getLoc(), "C++ standard attributes must "
-  "have valid version information.");
-break;
-  }
+if (Variety == "CXX11" || Variety == "C2x") {
+  std::vector Spellings = Attr->getValueAsListOfDefs("Spellings");
+  for (const auto &Spelling : Spellings) {
+if (Spelling->getValueAsString("Variety") == Variety) {
+  Version = static_cast(Spelling->getValueAsInt("Version"));
+  if (Scope.empty() && Version == 1)
+PrintError(Spelling->getLoc(), "Standard attributes must have "
+   "valid version information.");
+  break;
+}
   }
 }
 
Index: clang/test/Preprocessor/has_c_attribute.c
===
--- clang/test/Preprocessor/has_c_attribute.c
+++ clang/test/Preprocessor/has_c_attribute.c
@@ -1,22 +1,44 @@
-// RUN: %clang_cc1 -fdouble-square-bracket-attributes -std=c11 -E %s -o - | FileCheck %s
-// RUN: %clang_cc1 -std=c2x -E %s -o - | FileCheck %s
-
-// CHECK: has_fallthrough
-#if __has_c_attribute(fallthrough)
-  int has_fallthrough();
-#endif
-
-// CHECK: does_not_have_selectany
-#if !__has_c_attribute(selectany)
-  int does_not_have_selectany();
-#endif
-
-// CHECK: has_nodiscard_underscore
-#if __has_c_attribute(__nodiscard__)
-  int has_nodiscard_underscore();
-#endif
-
-// CHECK: has_clang_annotate
-#if __has_c_attribute(clang::annotate)
-  int has_clang_annotate();
-#endif
+// RUN: %clang_cc1 -fdouble-square-bracket-attributes -std=c11 -E -P %s -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c2x -E -P %s -o - | FileCheck %s
+
+#define C2x(x) x: __has_c_attribute(x)
+
+// CHECK: fallthrough: 201904L
+C2x(fallthrough)
+
+// CHECK: __nodiscard__: 201904L
+C2x(__nodiscard__)
+
+// CHECK: selectany: 0
+C2x(selectany); // Known attribute not supported in C mode
+
+// CHECK: frobble: 0
+C2x(frobble) // Unknown attribute
+
+// CHECK: frobble::frobble: 0
+C2x(frobble::frobble) // Unknown vendor namespace
+
+// CHECK: clang::annotate: 1
+C2x(clang::annotate)
+
+// CHECK: deprecated: 201904L
+C2x(deprecated)
+
+// CHECK: maybe_unused: 201904L
+C2x(maybe_unused)
+
+// CHECK: __gnu__::warn_unused_result: 201904L
+C2x(__gnu__::warn_unused_result)
+
+// CHECK: gnu::__warn_unused_result__: 201904L
+C2x(gnu::__warn_unused_result__)
+
+// We do somewhat support the __clang__ vendor namespace, but it is a
+// predefined macro and thus we encourage users to use _Clang instead.
+// Because of this, we do not support __has_c_attribute for that
+// vendor namespace.
+//
+// Note, we can't use C2x here because it will expand __clang__ to 1
+// too early.
+// CHECK: 1::fallthrough: 0
+__clang__::fallthrough: __has_c_attribute(__clang__::fallthrough)
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -267,8 +267,10 @@
   string Namespace = namespace;
   int Version = version;

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

2020-08-06 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

Whoa--that's the *help* text?  Well, if that's the only documentation for 
options that there is, I guess it has to go there; but it seems pretty 
excessive for the (ideally concise) output of `clang --help`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85239

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


[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:789
+// Remove "::" from the `SourceRange`
+SR.setEnd(SR.getEnd().getLocWithOffset(-1));
+

Newbie mistake. Corrected in latter commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348

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


[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:247
+
+  TerminationKind getTerminationKind();
+

gribozavr2 wrote:
> I just realized that the rest of the syntax tree API does not use the `get~` 
> prefix. However, most of Clang does, as far as I know. Which way should we 
> repaint?
Let's go with Clang! I can make this change in another patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85295

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


[PATCH] D85427: [SyntaxTree][NFC] remove redundant namespace-specifiers

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 283659.
eduucaldas added a comment.

removed namespace specifiers `llvm::`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85427

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  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
@@ -40,7 +40,7 @@
 using namespace clang;
 
 namespace {
-static llvm::ArrayRef tokens(syntax::Node *N) {
+static ArrayRef tokens(syntax::Node *N) {
   assert(N->isOriginal() && "tokens of modified nodes are not well-defined");
   if (auto *L = dyn_cast(N))
 return llvm::makeArrayRef(L->token(), 1);
@@ -53,7 +53,7 @@
public ::testing::WithParamInterface {
 protected:
   // Build a syntax tree for the code.
-  syntax::TranslationUnit *buildTree(llvm::StringRef Code,
+  syntax::TranslationUnit *buildTree(StringRef Code,
  const TestClangConfig &ClangConfig) {
 // FIXME: this code is almost the identical to the one in TokensTest. Share
 //it.
@@ -169,7 +169,7 @@
   }
 
   // Adds a file to the test VFS.
-  void addFile(llvm::StringRef Path, llvm::StringRef Contents) {
+  void addFile(StringRef Path, StringRef Contents) {
 if (!FS->addFile(Path, time_t(),
  llvm::MemoryBuffer::getMemBufferCopy(Contents))) {
   ADD_FAILURE() << "could not add a file to VFS: " << Path;
@@ -179,7 +179,7 @@
   /// 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);
+ArrayRef Toks = tokens(Root);
 
 if (Toks.front().location().isFileID() &&
 Toks.back().location().isFileID() &&
@@ -198,15 +198,14 @@
   }
 
   // Data fields.
-  llvm::IntrusiveRefCntPtr DiagOpts =
-  new DiagnosticOptions();
-  llvm::IntrusiveRefCntPtr Diags =
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  IntrusiveRefCntPtr Diags =
   new DiagnosticsEngine(new DiagnosticIDs, DiagOpts.get());
   IntrusiveRefCntPtr FS =
   new llvm::vfs::InMemoryFileSystem;
-  llvm::IntrusiveRefCntPtr FileMgr =
+  IntrusiveRefCntPtr FileMgr =
   new FileManager(FileSystemOptions(), FS);
-  llvm::IntrusiveRefCntPtr SourceMgr =
+  IntrusiveRefCntPtr SourceMgr =
   new SourceManager(*Diags, *FileMgr);
   std::shared_ptr Invocation;
   // Set after calling buildTree().
Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -36,11 +36,9 @@
  const TokenBuffer &Tokens)
 : SourceMgr(SourceMgr), LangOpts(LangOpts), Tokens(Tokens) {}
 
-const clang::syntax::TokenBuffer &syntax::Arena::tokenBuffer() const {
-  return Tokens;
-}
+const syntax::TokenBuffer &syntax::Arena::tokenBuffer() const { return Tokens; }
 
-std::pair>
+std::pair>
 syntax::Arena::lexBuffer(std::unique_ptr Input) {
   auto FID = SourceMgr.createFileID(std::move(Input));
   auto It = ExtraTokens.try_emplace(FID, tokenize(FID, SourceMgr, LangOpts));
@@ -135,7 +133,7 @@
 }
 
 namespace {
-static void dumpTokens(llvm::raw_ostream &OS, ArrayRef Tokens,
+static void dumpTokens(raw_ostream &OS, ArrayRef Tokens,
const SourceManager &SM) {
   assert(!Tokens.empty());
   bool First = true;
@@ -153,7 +151,7 @@
   }
 }
 
-static void dumpTree(llvm::raw_ostream &OS, const syntax::Node *N,
+static void dumpTree(raw_ostream &OS, const syntax::Node *N,
  const syntax::Arena &A, std::vector IndentMask) {
   std::string Marks;
   if (!N->isOriginal())
@@ -165,13 +163,13 @@
   if (!Marks.empty())
 OS << Marks << ": ";
 
-  if (auto *L = llvm::dyn_cast(N)) {
+  if (auto *L = dyn_cast(N)) {
 dumpTokens(OS, *L->token(), A.sourceManager());
 OS << "\n";
 return;
   }
 
-  auto *T = llvm::cast(N);
+  auto *T = cast(N);
   OS << T->kind() << "\n";
 
   for (auto It = T->firstChild(); It != nullptr; It = It->nextSibling()) {
@@ -205,7 +203,7 @@
   std::string Storage;
   llvm::raw_string_ostream OS(Storage);
   traverse(this, [&](const syntax::Node *N) {
-auto *L = llvm::dyn_cast(N);
+auto *L = dyn_cast(N);
 if (!L)
   return;
 ::dumpTokens(OS, *L->token(), A.sourceManager());
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Sy

[PATCH] D85442: [HIP] Ignore invalid ar linker options

2020-08-06 Thread Aaron Enye Shi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG96c2d5e99e32: [HIP] Ignore invalid ar linker options 
(authored by ashi1).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85442

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/hip-link-static-library.hip


Index: clang/test/Driver/hip-link-static-library.hip
===
--- clang/test/Driver/hip-link-static-library.hip
+++ clang/test/Driver/hip-link-static-library.hip
@@ -25,3 +25,14 @@
 
 // NORDC-NOT: offload bundler
 // NORDC: # "x86_64-unknown-linux-gnu" - "GNU::StaticLibTool", inputs: 
["{{.*o}}"], output: "a.out"
+
+// RUN: %clang --hip-link -### -target x86_64-linux-gnu \
+// RUN:   --emit-static-lib -lgcc \
+// RUN:   -Wl,--enable-new-dtags -Wl,--rpath=/opt \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 -fgpu-rdc %t.o\
+// RUN: 2>&1 | FileCheck -check-prefix=NOFLAG %s
+
+// NOFLAG-NOT: .*lgcc
+// NOFLAG-NOT: .*enable-new-dtags
+// NOFLAG-NOT: .*rpath=/opt
+// NOFLAG: "{{.*}}llvm-ar{{.*}}" "rcsD" "{{.*}}.out" "{{.*o}}" "{{.*o}}"
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -341,12 +341,17 @@
   // Silence warnings when linking C code with a C++ '-stdlib' argument.
   Args.ClaimAllArgs(options::OPT_stdlib_EQ);
 
-  // GNU ar tool command "ar   ".
+  // ar tool command "llvm-ar   ".
   ArgStringList CmdArgs;
   // Create and insert file members with a deterministic index.
   CmdArgs.push_back("rcsD");
   CmdArgs.push_back(Output.getFilename());
-  AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
+
+  for (const auto &II : Inputs) {
+if (II.isFilename()) {
+   CmdArgs.push_back(II.getFilename());
+}
+  }
 
   // Delete old output archive file if it already exists before generating a 
new
   // archive file.


Index: clang/test/Driver/hip-link-static-library.hip
===
--- clang/test/Driver/hip-link-static-library.hip
+++ clang/test/Driver/hip-link-static-library.hip
@@ -25,3 +25,14 @@
 
 // NORDC-NOT: offload bundler
 // NORDC: # "x86_64-unknown-linux-gnu" - "GNU::StaticLibTool", inputs: ["{{.*o}}"], output: "a.out"
+
+// RUN: %clang --hip-link -### -target x86_64-linux-gnu \
+// RUN:   --emit-static-lib -lgcc \
+// RUN:   -Wl,--enable-new-dtags -Wl,--rpath=/opt \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 -fgpu-rdc %t.o\
+// RUN: 2>&1 | FileCheck -check-prefix=NOFLAG %s
+
+// NOFLAG-NOT: .*lgcc
+// NOFLAG-NOT: .*enable-new-dtags
+// NOFLAG-NOT: .*rpath=/opt
+// NOFLAG: "{{.*}}llvm-ar{{.*}}" "rcsD" "{{.*}}.out" "{{.*o}}" "{{.*o}}"
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -341,12 +341,17 @@
   // Silence warnings when linking C code with a C++ '-stdlib' argument.
   Args.ClaimAllArgs(options::OPT_stdlib_EQ);
 
-  // GNU ar tool command "ar   ".
+  // ar tool command "llvm-ar   ".
   ArgStringList CmdArgs;
   // Create and insert file members with a deterministic index.
   CmdArgs.push_back("rcsD");
   CmdArgs.push_back(Output.getFilename());
-  AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
+
+  for (const auto &II : Inputs) {
+if (II.isFilename()) {
+   CmdArgs.push_back(II.getFilename());
+}
+  }
 
   // Delete old output archive file if it already exists before generating a new
   // archive file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 96c2d5e - [HIP] Ignore invalid ar linker options

2020-08-06 Thread Aaron En Ye Shi via cfe-commits

Author: Aaron En Ye Shi
Date: 2020-08-06T17:39:41Z
New Revision: 96c2d5e99e32340be1379959977f2d6247788db6

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

LOG: [HIP] Ignore invalid ar linker options

Instead of accepting the same arguments as regular linker,
the static linker will only accept input files.

Reviewed By: yaxunl

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/test/Driver/hip-link-static-library.hip

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 9ca674511dab..d423a71b5cca 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -341,12 +341,17 @@ void tools::gnutools::StaticLibTool::ConstructJob(
   // Silence warnings when linking C code with a C++ '-stdlib' argument.
   Args.ClaimAllArgs(options::OPT_stdlib_EQ);
 
-  // GNU ar tool command "ar   ".
+  // ar tool command "llvm-ar   ".
   ArgStringList CmdArgs;
   // Create and insert file members with a deterministic index.
   CmdArgs.push_back("rcsD");
   CmdArgs.push_back(Output.getFilename());
-  AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
+
+  for (const auto &II : Inputs) {
+if (II.isFilename()) {
+   CmdArgs.push_back(II.getFilename());
+}
+  }
 
   // Delete old output archive file if it already exists before generating a 
new
   // archive file.

diff  --git a/clang/test/Driver/hip-link-static-library.hip 
b/clang/test/Driver/hip-link-static-library.hip
index 55c5a5acc5ca..e670980fa486 100644
--- a/clang/test/Driver/hip-link-static-library.hip
+++ b/clang/test/Driver/hip-link-static-library.hip
@@ -25,3 +25,14 @@
 
 // NORDC-NOT: offload bundler
 // NORDC: # "x86_64-unknown-linux-gnu" - "GNU::StaticLibTool", inputs: 
["{{.*o}}"], output: "a.out"
+
+// RUN: %clang --hip-link -### -target x86_64-linux-gnu \
+// RUN:   --emit-static-lib -lgcc \
+// RUN:   -Wl,--enable-new-dtags -Wl,--rpath=/opt \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 -fgpu-rdc %t.o\
+// RUN: 2>&1 | FileCheck -check-prefix=NOFLAG %s
+
+// NOFLAG-NOT: .*lgcc
+// NOFLAG-NOT: .*enable-new-dtags
+// NOFLAG-NOT: .*rpath=/opt
+// NOFLAG: "{{.*}}llvm-ar{{.*}}" "rcsD" "{{.*}}.out" "{{.*o}}" "{{.*o}}"



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


[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

2020-08-06 Thread Michael Liao via Phabricator via cfe-commits
hliao added inline comments.



Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:839-840
 
+  // Skip host-only functions in the CUDA device compilation and device-only
+  // functions in the host compilation.
+  if (CGM.getLangOpts().CUDA &&

tra wrote:
> hliao wrote:
> > tra wrote:
> > > We will still have around some functions that may never be used on the 
> > > host side (HD functions referenced from device code only).  I'm not sure 
> > > if that's a problem for profiling, though. I wonder if we can somehow tie 
> > > `skipRegionMappingForDecl` to whether we've actually codegen'ed the 
> > > function. 
> > Skipping wrong-side functions here just makes the report not confusing as 
> > these functions are not emitted at all and are supposed never running on 
> > the host/device side. If we still create the mapping for them, e.g., we may 
> > report they have 0 runs instead of reporting nothing (just like comments 
> > between function.) That looks a little bit confusing.
> > It seems the current PGO adds everything for coverage mapping and late 
> > prune them based on checks here. Just try to follow that logic to skip 
> > wrong-side functions. If we need to revise the original logic and generate 
> > coverage mapping for emitted functions only, the change here is unnecessary.
> I'd add a comment here that this 'filter' is just a rough best-effort 
> approximation that still allows some effectively device-only Decls through.
> The output should still be correct, even though the functions will never be 
> used. Maybe add a TODO to deal with it if/when we know if the Decl was 
> codegen'ed.
> 
Add that comment. But, I tend to not deal that "effectively" 
host-only/device-only ones as that should be developers' responsibility to 
handle them. The additional zero coverage mapping may be useful as well. If a 
function is really device-only but is attributed with HD, the 0 coverage may 
help developers correcting them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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


[PATCH] D84781: [SyntaxTree] Use PointerUnion instead of inheritance for alternative clauses in NNS

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:788-789
-
-// Remove "::" from the `SourceRange`
-SR.setEnd(SR.getEnd().getLocWithOffset(-1));
 

Newbie mistake causing the crash



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:815
+  Builder.markChild(new (allocator()) syntax::EmptyNode,
+syntax::NodeRole::Unknown);
+  return new (allocator()) syntax::NameSpecifier;

I mark `NodeRole` as `Unknown` here, this would need fixing in the future. But 
should we make roles that  mimic the `NodeKind` of the alternative?



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1004-1009
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-template
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->

Here notice the additional level of nesting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84781

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


[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

2020-08-06 Thread Michael Liao via Phabricator via cfe-commits
hliao updated this revision to Diff 283654.
hliao added a comment.

Revise the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

Files:
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/test/CodeGenCUDA/profile-coverage-mapping.cu


Index: clang/test/CodeGenCUDA/profile-coverage-mapping.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/profile-coverage-mapping.cu
@@ -0,0 +1,20 @@
+// RUN: echo "GPU binary would be here" > %t
+// RUN: %clang_cc1 -fprofile-instrument=clang -triple x86_64-linux-gnu 
-target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm -o - %s | 
FileCheck --check-prefix=PGOGEN %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -triple 
x86_64-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm 
-o - %s | FileCheck --check-prefix=COVMAP %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping 
-dump-coverage-mapping -triple x86_64-linux-gnu -target-sdk-version=8.0 
-fcuda-include-gpubinary %t -emit-llvm-only -o - %s | FileCheck 
--check-prefix=MAPPING %s
+
+#include "Inputs/cuda.h"
+
+// PGOGEN-NOT: @__profn_{{.*kernel.*}} =
+// COVMAP-COUNT-2: section "__llvm_covfun", comdat
+// COVMAP-NOT: section "__llvm_covfun", comdat
+// MAPPING-NOT: {{.*dfn.*}}:
+// MAPPING-NOT: {{.*kernel.*}}:
+
+__device__ void dfn(int i) {}
+
+__global__ void kernel(int i) { dfn(i); }
+
+void host(void) {
+  kernel<<<1, 1>>>(1);
+}
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -773,6 +773,11 @@
   if (!D->hasBody())
 return;
 
+  // Skip CUDA/HIP kernel launch stub functions.
+  if (CGM.getLangOpts().CUDA && !CGM.getLangOpts().CUDAIsDevice &&
+  D->hasAttr())
+return;
+
   bool InstrumentRegions = CGM.getCodeGenOpts().hasProfileClangInstr();
   llvm::IndexedInstrProfReader *PGOReader = CGM.getPGOReader();
   if (!InstrumentRegions && !PGOReader)
@@ -831,6 +836,18 @@
   if (!D->getBody())
 return true;
 
+  // Skip host-only functions in the CUDA device compilation and device-only
+  // functions in the host compilation. Just roughly filter them out based on
+  // the function attributes. If there are effectively host-only or device-only
+  // ones, their coverage mapping may still be generated.
+  if (CGM.getLangOpts().CUDA &&
+  ((CGM.getLangOpts().CUDAIsDevice && !D->hasAttr() &&
+!D->hasAttr()) ||
+   (!CGM.getLangOpts().CUDAIsDevice &&
+(D->hasAttr() ||
+ (!D->hasAttr() && D->hasAttr())
+return true;
+
   // Don't map the functions in system headers.
   const auto &SM = CGM.getContext().getSourceManager();
   auto Loc = D->getBody()->getBeginLoc();


Index: clang/test/CodeGenCUDA/profile-coverage-mapping.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/profile-coverage-mapping.cu
@@ -0,0 +1,20 @@
+// RUN: echo "GPU binary would be here" > %t
+// RUN: %clang_cc1 -fprofile-instrument=clang -triple x86_64-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm -o - %s | FileCheck --check-prefix=PGOGEN %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -triple x86_64-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm -o - %s | FileCheck --check-prefix=COVMAP %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -triple x86_64-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm-only -o - %s | FileCheck --check-prefix=MAPPING %s
+
+#include "Inputs/cuda.h"
+
+// PGOGEN-NOT: @__profn_{{.*kernel.*}} =
+// COVMAP-COUNT-2: section "__llvm_covfun", comdat
+// COVMAP-NOT: section "__llvm_covfun", comdat
+// MAPPING-NOT: {{.*dfn.*}}:
+// MAPPING-NOT: {{.*kernel.*}}:
+
+__device__ void dfn(int i) {}
+
+__global__ void kernel(int i) { dfn(i); }
+
+void host(void) {
+  kernel<<<1, 1>>>(1);
+}
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -773,6 +773,11 @@
   if (!D->hasBody())
 return;
 
+  // Skip CUDA/HIP kernel launch stub functions.
+  if (CGM.getLangOpts().CUDA && !CGM.getLangOpts().CUDAIsDevice &&
+  D->hasAttr())
+return;
+
   bool InstrumentRegions = CGM.getCodeGenOpts().hasProfileClangInstr();
   llvm::IndexedInstrProfReader *PGOReader = CGM.getPGOReader();
   if (!InstrumentRegions && !PGOReader)
@@ -831,6 +836,18 @@
   if (!D->getBody())
 return true;
 
+  // Skip host-only functions in the CUDA device compilation and device-only
+  // functions in the host compilation. Just roughly filter them out based on
+  // the function attributes. If there are effectively host-only or device-only
+

[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

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

LGTM. thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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


[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

2020-08-06 Thread Anatoly Trosinenko via Phabricator via cfe-commits
atrosinenko added a comment.

I suspect there might be some terminology clash, so clarifying a bit just in 
case.

It was probably better to refer to these functions as //LibCalls// - functions 
from the compiler support library (such as `libgcc`) such as `__adddf3`. While 
there are `__popcount[sdt]i2` among them, most of the times they are implicitly 
called when the high-level code performs division on `__uint128`, for example. 
Unfortunately, they reside under `compiler-rt/lib/builtins` - so that name was 
used... :) So, if I get it right, you have proposed something like 
`clang/include/clang/Basic/BuiltinsMips.def` and those are another kind of 
builtins.

The `CallingConv::MSP430_BUILTIN` was the name of a calling convention that the 
MSP430 codegen of LLVM had to use //for a small subset of those support 
functions//, as defined by MSP430 EABI. While it can be useful to add some 
other CC for those functions for internal use by compiler-rt someday, now there 
are only two CCs defined for MSP430 LibCalls: that "special convention" for 13 
functions only with two 64-bit arguments (including 64-bit integer shifts) and 
the `CallingConv::C` for everything else both from the support library and 
regular object files.

Technically, these functions could probably be listed by names somewhere in the 
backend code, completely detangling the upstreaming of MSP430 compiler-rt port 
from D84602  (this one), D84605 
 and, the most scary of them, D84636 
. That may probably look a bit hackish, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84602

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


[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

2020-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment.

Could you add unit tests for methods in List and NNS? OK if they are in a 
separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85440

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


[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

2020-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment.

Please also add appropriate handling to `syntax::List::getDelimiterTokenKind`, 
`getTerminationKind`, and `canBeEmpty`.




Comment at: clang/lib/Tooling/Syntax/Nodes.cpp:240
   return Children;
-}
+};
 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85440

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


[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:247
+
+  TerminationKind getTerminationKind();
+

I just realized that the rest of the syntax tree API does not use the `get~` 
prefix. However, most of Clang does, as far as I know. Which way should we 
repaint?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85295

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


[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

2020-08-06 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

In D85276#2200108 , @tra wrote:

> In D85276#2199655 , @yaxunl wrote:
>
>> Do we need to disable pgo and coverage mapping for device compilation? Or it 
>> is already disabled?
>
> We already disable profiling during device compilation for NVIDIA and AMD 
> GPUs:
> https://github.com/llvm/llvm-project/blob/394db2259575ef3cac8d3d37836b11eb2373c435/clang/lib/Driver/ToolChains/Clang.cpp#L4876

Anyway, this patch just fixes the caused by that device stub function. As it's 
"emitted" in the host compilation, we need to skip generating instrumentation 
on it explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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


[PATCH] D85337: [AMDGPU] gfx1031 target

2020-08-06 Thread Stanislav Mekhanoshin via Phabricator via cfe-commits
rampitec added inline comments.



Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1844
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1030),
+  LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_MACH_AMDGCN_GFX1031),
   LLVM_READOBJ_ENUM_ENT(ELF, EF_AMDGPU_XNACK),

jhenderson wrote:
> Where's the dedicated llvm-readobj test for this? Please add one. If there 
> exists one for the other AMDGPU flags, extend that one, otherwise, it might 
> be best to write one for the whole set at once.
It is in the lvm/test/CodeGen/AMDGPU/elf-header-flags-mach.ll above along with 
all other targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85337

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


[clang] 8d072a4 - [OPENMP]Fix for Windows buildbots, NFC.

2020-08-06 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-08-06T12:36:52-04:00
New Revision: 8d072a4405213623a1b13dbac2e451df81457343

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

LOG: [OPENMP]Fix for Windows buildbots, NFC.

Added: 


Modified: 
clang/include/clang/AST/OpenMPClause.h

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 01e915d5ce9d..35ab8ff39efa 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -7884,8 +7884,8 @@ struct TargetOMPContext final : public 
llvm::omp::OMPContext {
 /// any.
 class OMPChildren final
 : private llvm::TrailingObjects {
+  friend TrailingObjects;
   friend class OMPClauseReader;
-  friend class TrailingObjects;
   friend class OMPExecutableDirective;
   template  friend class OMPDeclarativeDirective;
 



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


[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 283643.
eduucaldas added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85440

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp

Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -200,10 +200,6 @@
 return OS << "IdExpression_id";
   case syntax::NodeRole::IdExpression_qualifier:
 return OS << "IdExpression_qualifier";
-  case syntax::NodeRole::NestedNameSpecifier_specifier:
-return OS << "NestedNameSpecifier_specifier";
-  case syntax::NodeRole::NestedNameSpecifier_delimiter:
-return OS << "NestedNameSpecifier_delimiter";
   case syntax::NodeRole::ParenExpression_subExpression:
 return OS << "ParenExpression_subExpression";
   }
@@ -219,23 +215,29 @@
   syntax::SimpleTemplateSpecifier *>::getFromOpaqueValue(firstChild());
 }
 
-std::vector syntax::NestedNameSpecifier::delimiters() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_delimiter);
-Children.push_back(llvm::cast(C));
+// We could have an interator in list to not pay memory costs of temporary
+// vector
+std::vector syntax::NestedNameSpecifier::specifiers() {
+  auto specifiersAsNodes = getElementsAsNodes();
+  std::vector Children;
+  for (const auto &element : specifiersAsNodes) {
+Children.push_back(llvm::cast(element));
   }
   return Children;
 }
 
-std::vector syntax::NestedNameSpecifier::specifiers() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_specifier);
-Children.push_back(llvm::cast(C));
+std::vector>
+syntax::NestedNameSpecifier::specifiersAndDoubleColons() {
+  auto specifiersAsNodesAndDoubleColons = getElementsAsNodesAndDelimiters();
+  std::vector>
+  Children;
+  for (const auto &specifierAndDoubleColon : specifiersAsNodesAndDoubleColons) {
+Children.push_back(
+{llvm::cast(specifierAndDoubleColon.element),
+ specifierAndDoubleColon.delimiter});
   }
   return Children;
-}
+};
 
 syntax::NestedNameSpecifier *syntax::IdExpression::qualifier() {
   return llvm::cast_or_null(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -895,9 +895,8 @@
 for (auto it = QualifierLoc; it; it = it.getPrefix()) {
   auto *NS = BuildNameSpecifier(it);
   assert(NS);
-  Builder.markChild(NS, syntax::NodeRole::NestedNameSpecifier_specifier);
-  Builder.markChildToken(it.getEndLoc(),
- syntax::NodeRole::NestedNameSpecifier_delimiter);
+  Builder.markChild(NS, syntax::NodeRole::List_element);
+  Builder.markChildToken(it.getEndLoc(), syntax::NodeRole::List_delimiter);
 }
 Builder.foldNode(Builder.getRange(QualifierLoc.getSourceRange()),
  new (allocator()) syntax::NestedNameSpecifier,
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -174,8 +174,6 @@
   ParametersAndQualifiers_trailingReturn,
   IdExpression_id,
   IdExpression_qualifier,
-  NestedNameSpecifier_specifier,
-  NestedNameSpecifier_delimiter,
   ParenExpression_subExpression
 };
 /// For debugging purposes.
@@ -235,14 +233,15 @@
 
 /// Models a `nested-name-specifier`. C++ [expr.prim.id.qual]
 /// e.g. the `std::vector::` in `std::vector::size`.
-class NestedNameSpecifier final : public Tree {
+class NestedNameSpecifier final : public List {
 public:
-  NestedNameSpecifier() : Tree(NodeKind::NestedNameSpecifier) {}
+  NestedNameSpecifier() : List(NodeKind::NestedNameSpecifier) {}
   static bool classof(const Node *N) {
 return N->kind() <= NodeKind::NestedNameSpecifier;
   }
   std::vector specifiers();
-  std::vector delimiters();
+  std::vector>
+  specifiersAndDoubleColons();
 };
 
 /// Models an `unqualified-id`. C++ [expr.prim.id.unqual]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 283638.
eduucaldas added a comment.

Clean List specific code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85440

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp

Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -200,10 +200,6 @@
 return OS << "IdExpression_id";
   case syntax::NodeRole::IdExpression_qualifier:
 return OS << "IdExpression_qualifier";
-  case syntax::NodeRole::NestedNameSpecifier_specifier:
-return OS << "NestedNameSpecifier_specifier";
-  case syntax::NodeRole::NestedNameSpecifier_delimiter:
-return OS << "NestedNameSpecifier_delimiter";
   case syntax::NodeRole::ParenExpression_subExpression:
 return OS << "ParenExpression_subExpression";
   }
@@ -219,23 +215,29 @@
   syntax::SimpleTemplateSpecifier *>::getFromOpaqueValue(firstChild());
 }
 
-std::vector syntax::NestedNameSpecifier::delimiters() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_delimiter);
-Children.push_back(llvm::cast(C));
+// We could have an interator in list to not pay memory costs of temporary
+// vector
+std::vector syntax::NestedNameSpecifier::specifiers() {
+  auto specifiersAsNodes = getElementsAsNodes();
+  std::vector Children;
+  for (const auto &element : specifiersAsNodes) {
+Children.push_back(llvm::cast(element));
   }
   return Children;
 }
 
-std::vector syntax::NestedNameSpecifier::specifiers() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_specifier);
-Children.push_back(llvm::cast(C));
+std::vector>
+syntax::NestedNameSpecifier::specifiersAndDoubleColons() {
+  auto specifiersAsNodesAndDoubleColons = getElementsAsNodesAndDelimiters();
+  std::vector>
+  Children;
+  for (const auto &specifierAndDoubleColon : specifiersAsNodesAndDoubleColons) {
+Children.push_back(
+{llvm::cast(specifierAndDoubleColon.element),
+ specifierAndDoubleColon.delimiter});
   }
   return Children;
-}
+};
 
 syntax::NestedNameSpecifier *syntax::IdExpression::qualifier() {
   return llvm::cast_or_null(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -895,9 +895,8 @@
 for (auto it = QualifierLoc; it; it = it.getPrefix()) {
   auto *NS = BuildNameSpecifier(it);
   assert(NS);
-  Builder.markChild(NS, syntax::NodeRole::NestedNameSpecifier_specifier);
-  Builder.markChildToken(it.getEndLoc(),
- syntax::NodeRole::NestedNameSpecifier_delimiter);
+  Builder.markChild(NS, syntax::NodeRole::List_element);
+  Builder.markChildToken(it.getEndLoc(), syntax::NodeRole::List_delimiter);
 }
 Builder.foldNode(Builder.getRange(QualifierLoc.getSourceRange()),
  new (allocator()) syntax::NestedNameSpecifier,
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -174,8 +174,6 @@
   ParametersAndQualifiers_trailingReturn,
   IdExpression_id,
   IdExpression_qualifier,
-  NestedNameSpecifier_specifier,
-  NestedNameSpecifier_delimiter,
   ParenExpression_subExpression
 };
 /// For debugging purposes.
@@ -235,14 +233,15 @@
 
 /// Models a `nested-name-specifier`. C++ [expr.prim.id.qual]
 /// e.g. the `std::vector::` in `std::vector::size`.
-class NestedNameSpecifier final : public Tree {
+class NestedNameSpecifier final : public List {
 public:
-  NestedNameSpecifier() : Tree(NodeKind::NestedNameSpecifier) {}
+  NestedNameSpecifier() : List(NodeKind::NestedNameSpecifier) {}
   static bool classof(const Node *N) {
 return N->kind() <= NodeKind::NestedNameSpecifier;
   }
   std::vector specifiers();
-  std::vector delimiters();
+  std::vector>
+  specifiersAndDoubleColons();
 };
 
 /// Models an `unqualified-id`. C++ [expr.prim.id.unqual]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

2020-08-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D85276#2199655 , @yaxunl wrote:

> Do we need to disable pgo and coverage mapping for device compilation? Or it 
> is already disabled?

We already disable profiling during device compilation for NVIDIA and AMD GPUs:
https://github.com/llvm/llvm-project/blob/394db2259575ef3cac8d3d37836b11eb2373c435/clang/lib/Driver/ToolChains/Clang.cpp#L4876


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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


[PATCH] D85439: [SyntaxTree] Expand support for `NestedNameSpecifier`

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added a reviewer: gribozavr2.
eduucaldas added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:227-254
+namespace llvm {
+template <> struct DenseMapInfo {
+  using FirstInfo = DenseMapInfo;
+  using SecondInfo = DenseMapInfo;
+
+  static inline NestedNameSpecifierLoc getEmptyKey() {
+return NestedNameSpecifierLoc(FirstInfo::getEmptyKey(),

Inpired on the definition of: 
* `template struct DenseMapInfo`
* `template struct DenseMapInfo>`

Please tell me if this is all silly.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1235
 | | | | |   |-(
-| | | | |   |-IdExpression
-| | | | |   | `-UnqualifiedId
-| | | | |   |   `-s
+| | | | |   |-s
 | | | | |   `-)

standard `TraverseNestedNameSpecifierLoc` fired `TraverseTypeLoc`, once we 
override it we lost this. This will be fixed when refining the Node for 
`DecltypeSpecifier`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85439

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


[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

2020-08-06 Thread Simon Moll via Phabricator via cfe-commits
simoll updated this revision to Diff 283636.
simoll added a comment.

NFC. Cleanup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int &reference_to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto T = CmpMul(a, b);
   // CHECK: store <4 x i8> 
Index: clang/test/CodeGen/debug-info-v

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

2020-08-06 Thread Simon Moll via Phabricator via cfe-commits
simoll updated this revision to Diff 283632.
simoll added a comment.

Fixed debug info representation for bool vectors.
Interpret 8*N with the N in vector_size(N) as the bool numbers of bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int &reference_to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: 

[PATCH] D85440: [SyntaxTree] Implement `NNS` using the `List` base API

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85440

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp

Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -200,10 +200,6 @@
 return OS << "IdExpression_id";
   case syntax::NodeRole::IdExpression_qualifier:
 return OS << "IdExpression_qualifier";
-  case syntax::NodeRole::NestedNameSpecifier_specifier:
-return OS << "NestedNameSpecifier_specifier";
-  case syntax::NodeRole::NestedNameSpecifier_delimiter:
-return OS << "NestedNameSpecifier_delimiter";
   case syntax::NodeRole::ParenExpression_subExpression:
 return OS << "ParenExpression_subExpression";
   }
@@ -219,23 +215,29 @@
   syntax::SimpleTemplateSpecifier *>::getFromOpaqueValue(firstChild());
 }
 
-std::vector syntax::NestedNameSpecifier::delimiters() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_delimiter);
-Children.push_back(llvm::cast(C));
+// We could have an interator in list to not pay memory costs of temporary
+// vector
+std::vector syntax::NestedNameSpecifier::specifiers() {
+  auto specifiersAsNodes = getElementsAsNodes();
+  std::vector Children;
+  for (const auto &element : specifiersAsNodes) {
+Children.push_back(llvm::cast(element));
   }
   return Children;
 }
 
-std::vector syntax::NestedNameSpecifier::specifiers() {
-  std::vector Children;
-  for (auto *C = firstChild(); C; C = C->nextSibling()) {
-assert(C->role() == syntax::NodeRole::NestedNameSpecifier_specifier);
-Children.push_back(llvm::cast(C));
+std::vector>
+syntax::NestedNameSpecifier::specifiersAndDoubleColons() {
+  auto specifiersAsNodesAndDoubleColons = getElementsAsNodesAndDelimiters();
+  std::vector>
+  Children;
+  for (const auto &specifierAndDoubleColon : specifiersAsNodesAndDoubleColons) {
+Children.push_back(
+{llvm::cast(specifierAndDoubleColon.element),
+ specifierAndDoubleColon.delimiter});
   }
   return Children;
-}
+};
 
 syntax::NestedNameSpecifier *syntax::IdExpression::qualifier() {
   return llvm::cast_or_null(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -895,9 +895,8 @@
 for (auto it = QualifierLoc; it; it = it.getPrefix()) {
   auto *NS = BuildNameSpecifier(it);
   assert(NS);
-  Builder.markChild(NS, syntax::NodeRole::NestedNameSpecifier_specifier);
-  Builder.markChildToken(it.getEndLoc(),
- syntax::NodeRole::NestedNameSpecifier_delimiter);
+  Builder.markChild(NS, syntax::NodeRole::List_element);
+  Builder.markChildToken(it.getEndLoc(), syntax::NodeRole::List_delimiter);
 }
 Builder.foldNode(Builder.getRange(QualifierLoc.getSourceRange()),
  new (allocator()) syntax::NestedNameSpecifier,
Index: clang/include/clang/Tooling/Syntax/Tree.h
===
--- clang/include/clang/Tooling/Syntax/Tree.h
+++ clang/include/clang/Tooling/Syntax/Tree.h
@@ -214,6 +214,7 @@
 /// canBeEmpty() returning `true`
 /// getDelimiterTokenKind() returning `,`
 class List : public Tree {
+public:
   template  struct ElementAndDelimiter {
 Element *element;
 Leaf *delimiter;
@@ -225,6 +226,7 @@
 Separated,
   };
 
+  using Tree::Tree;
   /// Returns the elements and corresponding delimiters. Missing elements
   /// and delimiters are represented as null pointers.
   ///
Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -174,8 +174,6 @@
   ParametersAndQualifiers_trailingReturn,
   IdExpression_id,
   IdExpression_qualifier,
-  NestedNameSpecifier_specifier,
-  NestedNameSpecifier_delimiter,
   ParenExpression_subExpression
 };
 /// For debugging purposes.
@@ -235,14 +233,15 @@
 
 /// Models a `nested-name-specifier`. C++ [expr.prim.id.qual]
 /// e.g. the `std::vector::` in `std::vector::size`.
-class NestedNameSpecifier final : public Tree {
+class NestedNameSpecifier final : public List {
 public:
-  NestedNameSpecifier() : Tree(NodeKind::NestedNameSpecifier) {}
+  NestedNameSpecifier() : List(NodeKind::NestedNameSpecifier) {}
   static bool classof(const Node *N) {
 return N->kind() <= NodeKind::NestedNameSpecifier;
   }
   std::vector specifiers();
-  std::

[PATCH] D85439: [SyntaxTree] Expand support for `NestedNameSpecifier`

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

We want NestedNameSpecifier syntax nodes to be generally supported, not
only for `DeclRefExpr` and `DependentScopedDeclRefExpr`.

To achieve this we:

- Use the `RecursiveASTVisitor`'s API to traverse

`NestedNameSpecifierLoc`s and automatically create its syntax nodes

- Add links from the `NestedNameSpecifierLoc`s to their syntax nodes.

In this way, from any semantic construct that has a `NestedNameSpecifier`,
we implicitly generate its syntax node via RAV and we can easily access
this syntax node via the links we added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85439

Files:
  clang/lib/Tooling/Syntax/BuildTree.cpp
  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
@@ -1232,9 +1232,7 @@
 | | | | | `-DecltypeSpecifier
 | | | | |   |-decltype
 | | | | |   |-(
-| | | | |   |-IdExpression
-| | | | |   | `-UnqualifiedId
-| | | | |   |   `-s
+| | | | |   |-s
 | | | | |   `-)
 | | | | `-::
 | | | `-UnqualifiedId
@@ -2980,7 +2978,8 @@
 `-UsingNamespaceDirective
   |-using
   |-namespace
-  |-::
+  |-NestedNameSpecifier
+  | `-::
   |-ns
   `-;
 )txt"));
@@ -3009,8 +3008,10 @@
 | `-}
 `-UsingDeclaration
   |-using
-  |-ns
-  |-::
+  |-NestedNameSpecifier
+  | |-NameSpecifier
+  | | `-ns
+  | `-::
   |-a
   `-;
 )txt"));
@@ -3214,11 +3215,14 @@
   |->
   `-SimpleDeclaration
 |-struct
-|-X
-|-<
-|-T
-|->
-|-::
+|-NestedNameSpecifier
+| |-NameSpecifier
+| | `-SimpleTemplateSpecifier
+| |   |-X
+| |   |-<
+| |   |-T
+| |   `->
+| `-::
 |-Y
 |-{
 |-}
@@ -3252,15 +3256,19 @@
 |-{
 |-UsingDeclaration
 | |-using
-| |-T
-| |-::
+| |-NestedNameSpecifier
+| | |-NameSpecifier
+| | | `-T
+| | `-::
 | |-foo
 | `-;
 |-UsingDeclaration
 | |-using
 | |-typename
-| |-T
-| |-::
+| |-NestedNameSpecifier
+| | |-NameSpecifier
+| | | `-T
+| | `-::
 | |-bar
 | `-;
 |-}
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -224,6 +224,34 @@
   return SourceRange(Start, End);
 }
 
+namespace llvm {
+template <> struct DenseMapInfo {
+  using FirstInfo = DenseMapInfo;
+  using SecondInfo = DenseMapInfo;
+
+  static inline NestedNameSpecifierLoc getEmptyKey() {
+return NestedNameSpecifierLoc(FirstInfo::getEmptyKey(),
+  SecondInfo::getEmptyKey());
+  }
+
+  static inline NestedNameSpecifierLoc getTombstoneKey() {
+return NestedNameSpecifierLoc(FirstInfo::getTombstoneKey(),
+  SecondInfo::getTombstoneKey());
+  }
+
+  static unsigned getHashValue(const clang::NestedNameSpecifierLoc &PairVal) {
+return detail::combineHashValue(
+FirstInfo::getHashValue(PairVal.getNestedNameSpecifier()),
+SecondInfo::getHashValue(PairVal.getOpaqueData()));
+  }
+
+  static bool isEqual(const NestedNameSpecifierLoc &LHS,
+  const NestedNameSpecifierLoc &RHS) {
+return LHS == RHS;
+  }
+};
+} // namespace llvm
+
 namespace {
 /// All AST hierarchy roots that can be represented as pointers.
 using ASTPtr = llvm::PointerUnion;
@@ -243,8 +271,22 @@
 
   syntax::Tree *find(ASTPtr P) const { return Nodes.lookup(P); }
 
+  void add(NestedNameSpecifierLoc From, syntax::Tree *To) {
+assert(To != nullptr);
+assert(From.hasQualifier());
+
+bool Added = NNSNodes.insert({From, To}).second;
+(void)Added;
+assert(Added && "mapping added twice");
+  }
+
+  syntax::Tree *find(NestedNameSpecifierLoc P) const {
+return NNSNodes.lookup(P);
+  }
+
 private:
   llvm::DenseMap Nodes;
+  llvm::DenseMap NNSNodes;
 };
 } // namespace
 
@@ -289,9 +331,11 @@
   }
 
   void foldNode(llvm::ArrayRef Range, syntax::Tree *New,
-NestedNameSpecifierLoc L) {
-// FIXME: add mapping for NestedNameSpecifierLoc
-foldNode(Range, New, nullptr);
+NestedNameSpecifierLoc From) {
+assert(New);
+Pending.foldChildren(Arena, Range, New);
+if (From)
+  Mapping.add(From, New);
   }
   /// Notifies that we should not consume trailing semicolon when computing
   /// token range of \p D.
@@ -315,6 +359,9 @@
   /// Set role for the syntax node matching \p N.
   void markChild(ASTPtr N, NodeRole R);
 
+  /// Set role for the syntax node matching \p N.
+  void markChild(NestedNameSpecifierLoc N, NodeRole R);
+
   /// Finish building the tree and consume the root node.
   syntax::TranslationUnit *fina

[PATCH] D84781: Use PointerUnion instead of inheritance for alternative clauses in NNS

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 283627.
eduucaldas added a comment.

- [SyntaxTree] Fix crash on name specifier.

This diff revision is based on https://reviews.llvm.org/D84348


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84781

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  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
@@ -994,18 +994,19 @@
 | | |-IdExpression
 | | | |-NestedNameSpecifier
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-n
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | |-::
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-template
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-template
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
@@ -1016,17 +1017,18 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-n
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | |-::
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
@@ -1037,13 +1039,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | `-::
 | | | `-UnqualifiedId
@@ -1058,13 +1061,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | `-::
 | | | |-template
@@ -1122,15 +1126,16 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | |-::
-  | | | | |-SimpleTemplateNameSpecifier
-  | | | | | |-template
-  | | | | | |-U
-  | | | | | |-<
-  | | | | | |-int
-  | | | | | `->
+  | | | | |-NameSpecifier
+  | | | | | `-SimpleTemplateSpecifier
+  | | | | |   |-template
+  | | | | |   |-U
+  | | | | |   |-<
+  | | | | |   |-int
+  | | | | |   `->
   | | | | `-::
   | | | `-UnqualifiedId
   | | |   `-f
@@ -1141,10 +1146,10 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | |-::
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-U
   | | | | `-::
   | | | `-UnqualifiedId
@@ -1156,7 +1161,7 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | `-::
   | | | |-template
@@ -1223,13 +1228,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-DecltypeNameSpecifier
-| | | | | |-decltype
-| | | | | |-(
-| | | | | |-IdExpression
-| | | | | | `-UnqualifiedId
-| | | | | |   `-s
-| | | | | `-)
+| | | | |-NameSpecifier
+| | | | | `-DecltypeSpecifier
+| | | | |   |-decltype
+| | | | |   |-(
+| | | | |   |-IdExpression
+| | | | |   | `-UnqualifiedId
+| | | | |   |   `-s
+

[PATCH] D84811: [clangd] Add an option to exclude symbols outside of project root from index

2020-08-06 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev planned changes to this revision.
kbobyrev added a comment.

In D84811#2199891 , @hokein wrote:

> In D84811#2199829 , @kbobyrev wrote:
>
>> In D84811#2199820 , @hokein wrote:
>>
>>> In D84811#2199510 , @kbobyrev 
>>> wrote:
>>>
 Even despite `FileFilter` not being fully implemented yet (an issue for a 
 separate patch) I think this change should still be correct and is 
 probably OK to land, WDYT @hokein?
>>>
>>> Yes, personally no objection on landing this, but landing it doesn't seem 
>>> to help much for your remote-index case (STL symbols are not filtered out)?
>>
>> True, it doesn't filter _all_ of them, but it partially filters some symbols 
>> which is still a win I guess :) My rationale is probably that having this 
>> blocked on more implementation details would be more reasonable if there was 
>> no implementation at all but since it still filters out some symbols this 
>> should probably be fine.
>
> It just reduces symptoms, not solving the problem. I think remote-index is 
> also blocked the FileFiltering stuff (we can't launch remote-index without 
> fixing this issue entirely).
>
> As you may have noticed implementing FileFiltering is tricky, I think the 
> indexer here is a good opportunity to test/verify your implementation 
> (comparing the index data before vs after), so my preference would be to 
> implement the FileFilter, test it with this patch together, then finally land 
> this patch after making sure everything works.

Fair enough, this is a viable strategy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84811

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


[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 283622.
eduucaldas added a comment.

- Update comments to reflect change in API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  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
@@ -874,24 +874,47 @@
   }
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
-namespace a {
+namespace n {
   struct S {
 template
-static T f(){}
+struct ST {
+  static void f();
+};
   };
 }
+template
+struct ST {
+  struct S {
+template
+static U f();
+  };
+};
 void test() {
-  ::  // global-namespace-specifier
-  a:: // namespace-specifier
-  S:: // type-name-specifier
+  :: // global-namespace-specifier
+  n::// namespace-specifier
+  S::// type-name-specifier
+  template ST:: // type-template-instantiation-specifier
+  f();
+
+  n::// namespace-specifier
+  S::// type-name-specifier
+  ST::  // type-template-instantiation-specifier
+  f();
+
+  ST:: // type-name-specifier
+  S::   // type-name-specifier
   f();
+
+  ST:: // type-name-specifier
+  S::   // type-name-specifier
+  template f();
 }
 )cpp",
   R"txt(
 *: TranslationUnit
 |-NamespaceDefinition
 | |-namespace
-| |-a
+| |-n
 | |-{
 | |-SimpleDeclaration
 | | |-struct
@@ -905,19 +928,58 @@
 | | | | `-T
 | | | |->
 | | | `-SimpleDeclaration
-| | |   |-static
-| | |   |-T
-| | |   |-SimpleDeclarator
-| | |   | |-f
-| | |   | `-ParametersAndQualifiers
-| | |   |   |-(
-| | |   |   `-)
-| | |   `-CompoundStatement
-| | | |-{
-| | | `-}
+| | |   |-struct
+| | |   |-ST
+| | |   |-{
+| | |   |-SimpleDeclaration
+| | |   | |-static
+| | |   | |-void
+| | |   | |-SimpleDeclarator
+| | |   | | |-f
+| | |   | | `-ParametersAndQualifiers
+| | |   | |   |-(
+| | |   | |   `-)
+| | |   | `-;
+| | |   |-}
+| | |   `-;
 | | |-}
 | | `-;
 | `-}
+|-TemplateDeclaration
+| |-template
+| |-<
+| |-UnknownDeclaration
+| | |-typename
+| | `-T
+| |->
+| `-SimpleDeclaration
+|   |-struct
+|   |-ST
+|   |-{
+|   |-SimpleDeclaration
+|   | |-struct
+|   | |-S
+|   | |-{
+|   | |-TemplateDeclaration
+|   | | |-template
+|   | | |-<
+|   | | |-UnknownDeclaration
+|   | | | |-typename
+|   | | | `-U
+|   | | |->
+|   | | `-SimpleDeclaration
+|   | |   |-static
+|   | |   |-U
+|   | |   |-SimpleDeclarator
+|   | |   | |-f
+|   | |   | `-ParametersAndQualifiers
+|   | |   |   |-(
+|   | |   |   `-)
+|   | |   `-;
+|   | |-}
+|   | `-;
+|   |-}
+|   `-;
 `-SimpleDeclaration
   |-void
   |-SimpleDeclarator
@@ -931,14 +993,81 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-NameSpecifier
-| | | | | `-::
-| | | | |-NameSpecifier
-| | | | | |-a
-| | | | | `-::
-| | | | `-NameSpecifier
-| | | |   |-S
-| | | |   `-::
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-template
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-IdentifierNameSpecifier
+| | | | | `-n
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | |-::
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   `-f
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | | | `-::
+| | | `-UnqualifiedId
+| | |   |-f
+| | |   |-<
+| | |   |-int
+| | |   `->
+| | |-(
+| | `-)
+| `-;
+|-ExpressionStatement
+| |-UnknownExpression
+| | |-IdExpression
+| | | |-NestedNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
+| | | | | |-ST
+| | | | | |-<
+| | | | | |-int
+| | | | | `->
+| | | | |-::
+| | | | |-IdentifierNameSpecifier
+| | | | | `-S
+| | |

[PATCH] D84811: [clangd] Add an option to exclude symbols outside of project root from index

2020-08-06 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

In D84811#2199829 , @kbobyrev wrote:

> In D84811#2199820 , @hokein wrote:
>
>> In D84811#2199510 , @kbobyrev wrote:
>>
>>> Even despite `FileFilter` not being fully implemented yet (an issue for a 
>>> separate patch) I think this change should still be correct and is probably 
>>> OK to land, WDYT @hokein?
>>
>> Yes, personally no objection on landing this, but landing it doesn't seem to 
>> help much for your remote-index case (STL symbols are not filtered out)?
>
> True, it doesn't filter _all_ of them, but it partially filters some symbols 
> which is still a win I guess :) My rationale is probably that having this 
> blocked on more implementation details would be more reasonable if there was 
> no implementation at all but since it still filters out some symbols this 
> should probably be fine.

It just reduces symptoms, not solving the problem. I think remote-index is also 
blocked the FileFiltering stuff (we can't launch remote-index without fixing 
this issue entirely).

As you may have noticed implementing FileFiltering is tricky, I think the 
indexer here is a good opportunity to test/verify your implementation 
(comparing the index data before vs after), so my preference would be to 
implement the FileFilter, test it with this patch together, then finally land 
this patch after making sure everything works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84811

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


[PATCH] D85429: [OpenCL] Allow for variadic macros in C++ for OpenCL

2020-08-06 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

Can you extend test test/Preprocessor/macro_variadic.cl to cover C++ for OpenCL 
mode, otherwise LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85429

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


[PATCH] D82502: [PowerPC] Implement Load VSX Vector and Sign Extend and Zero Extend

2020-08-06 Thread Amy Kwan via Phabricator via cfe-commits
amyk accepted this revision.
amyk added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:703
+vector signed __int128 test_vec_xl_sext_i8(void) {
+  // CHECK: load i8
+  // CHECK: sext i8

It would be good to be a little more specific with the CHECK lines. 



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:13500
   SDValue Ext2 = N->getOperand(1).getOperand(0);
-  if(Ext1.getOpcode() != ISD::EXTRACT_VECTOR_ELT ||
- Ext2.getOpcode() != ISD::EXTRACT_VECTOR_ELT)
+  if (Ext1.getOpcode() != ISD::EXTRACT_VECTOR_ELT ||
+  Ext2.getOpcode() != ISD::EXTRACT_VECTOR_ELT)

This line here and below looks like unintended changes from clang-format I am 
guessing? I am OK with them being removed during commit. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82502

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


[PATCH] D85426: [clangd] Implement FileFilter for the indexer

2020-08-06 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:643
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+return nullptr;

hokein wrote:
> A drive-by comment from D84811: the file granularity vs symbol granularity is 
> tricky here.
> 
> Note that a *full* symbol (with declaration, definition, etc) may be formed 
> from different files (.h, .cc), thinking of a following case:
> 
> ```
> // foo.h
> void func();
> 
> // user.cc
> #include "foo.h"
> 
> // foo.cc
> #include "foo.h"
> void func() {}
> ```
> 
> if our indexer indexes `user.cc` first, then `foo.h` is considered indexed, 
> later when indexing `foo.cc`, we will skip the `func` symbol. so the symbol 
> `foo` will not have definition.
>  
> 
Oh, you're right, good catch! That's why the post-filtering would probably work 
but maybe won't be as fancy :(

Thank you for mentioning it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85426

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


[PATCH] D85285: [clangd] WIP experimentation with finding static grpc++ libraries

2020-08-06 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev planned changes to this revision.
kbobyrev added a comment.

WIP, nothing important here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85285

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


  1   2   >