[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-04-01 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment.

Right, sorry for the late reply, @NoQ.
I will get to it once I get these assignments off my head.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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


[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 334604.
tbaeder added a comment.

I know it's been a while, but here's an update on that patch.

Now that I've got https://reviews.llvm.org/D97362 and 
https://reviews.llvm.org/D97371 pushed, this is  a much simpler change and does 
not break any of the existing tests anymore.


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

https://reviews.llvm.org/D75844

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Parse/ParseDecl.cpp
  clang/test/AST/sourceranges.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -185,8 +185,11 @@
   return 1;
   [[clang::fallthrough]];  // expected-warning{{fallthrough annotation in unreachable code}}
 case 222:
+  return 2;
+  __attribute__((fallthrough)); // expected-warning{{fallthrough annotation in unreachable code}}
+case 223:
   n += 400;
-case 223:  // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+case 224:  // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
   ;
   }
 
@@ -341,3 +344,16 @@
   }
   return n;
 }
+
+int fallthrough_in_unreachable_code(int n) {
+  switch (n) {
+case 1:
+#ifndef MAYBE_THIS_EXISTS
+  return -1;
+#endif
+  __attribute__((fallthrough));
+case 2:
+  return 3;
+  }
+  return n;
+}
Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -108,6 +108,25 @@
   }
 }
 
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_case
+namespace attributed_case {
+  void f(int n) {
+switch(n) {
+  case 0:
+n--;
+// CHECK: AttributedStmt {{.*}} 
+// CHECK: FallThroughAttr {{.*}} 
+__attribute__((fallthrough))
+// CHECK: FallThroughAttr {{.*}} 
+  __attribute__((fallthrough))
+  ;
+  case 1:
+n++;
+break;
+}
+  }
+}
+
 // CHECK: NamespaceDecl {{.*}} attributed_stmt
 namespace attributed_stmt {
   // In DO_PRAGMA and _Pragma cases, `LoopHintAttr` comes from 
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -162,12 +162,17 @@
 ///',' or ')' are ignored, otherwise they produce a parse error.
 ///
 /// We follow the C++ model, but don't allow junk after the identifier.
-void Parser::ParseGNUAttributes(ParsedAttributes &attrs,
+void Parser::ParseGNUAttributes(ParsedAttributesWithRange &attrs,
 SourceLocation *endLoc,
 LateParsedAttrList *LateAttrs,
 Declarator *D) {
   assert(Tok.is(tok::kw___attribute) && "Not a GNU attribute list!");
 
+  SourceLocation StartLoc = Tok.getLocation(), Loc;
+
+  if (!endLoc)
+endLoc = &Loc;
+
   while (Tok.is(tok::kw___attribute)) {
 SourceLocation AttrTokLoc = ConsumeToken();
 unsigned OldNumAttrs = attrs.size();
@@ -260,6 +265,8 @@
   }
 }
   }
+
+  attrs.Range = SourceRange(StartLoc, *endLoc);
 }
 
 /// Determine whether the given attribute has an identifier argument.
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -1034,6 +1034,27 @@
   mutable AttributePool pool;
 };
 
+struct ParsedAttributesWithRange : ParsedAttributes {
+  ParsedAttributesWithRange(AttributeFactory &factory)
+: ParsedAttributes(factory) {}
+
+  void clear() {
+ParsedAttributes::clear();
+Range = SourceRange();
+  }
+
+  SourceRange Range;
+};
+struct ParsedAttributesViewWithRange : ParsedAttributesView {
+  ParsedAttributesViewWithRange() : ParsedAttributesView() {}
+  void clearListOnly() {
+ParsedAttributesView::clearListOnly();
+Range = SourceRange();
+  }
+
+  SourceRange Range;
+};
+
 /// These constants match the enumerated choices of
 /// err_attribute_argument_n_type and err_attribute_argument_type.
 enum AttributeArgumentNType {
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -1572,27 +1572,6 @@
 
   //======//
   // C99 6

[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 334605.

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

https://reviews.llvm.org/D75844

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Parse/ParseDecl.cpp
  clang/test/AST/sourceranges.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -185,9 +185,12 @@
   return 1;
   [[clang::fallthrough]];  // expected-warning{{fallthrough annotation in unreachable code}}
 case 222:
+  return 2;
+  __attribute__((fallthrough)); // expected-warning{{fallthrough annotation in unreachable code}}
+case 223:
   n += 400;
-case 223:  // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
-  ;
+case 224: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+;
   }
 
   long p = static_cast(n) * n;
@@ -341,3 +344,16 @@
   }
   return n;
 }
+
+int fallthrough_in_unreachable_code(int n) {
+  switch (n) {
+  case 1:
+#ifndef MAYBE_THIS_EXISTS
+return -1;
+#endif
+__attribute__((fallthrough));
+  case 2:
+return 3;
+  }
+  return n;
+}
Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -108,6 +108,24 @@
   }
 }
 
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_case
+namespace attributed_case {
+void f(int n) {
+  switch (n) {
+  case 0:
+n--;
+// CHECK: AttributedStmt {{.*}} 
+// CHECK: FallThroughAttr {{.*}} 
+__attribute__((fallthrough))
+// CHECK: FallThroughAttr {{.*}} 
+__attribute__((fallthrough));
+  case 1:
+n++;
+break;
+  }
+}
+} // namespace attributed_case
+
 // CHECK: NamespaceDecl {{.*}} attributed_stmt
 namespace attributed_stmt {
   // In DO_PRAGMA and _Pragma cases, `LoopHintAttr` comes from 
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -162,12 +162,16 @@
 ///',' or ')' are ignored, otherwise they produce a parse error.
 ///
 /// We follow the C++ model, but don't allow junk after the identifier.
-void Parser::ParseGNUAttributes(ParsedAttributes &attrs,
+void Parser::ParseGNUAttributes(ParsedAttributesWithRange &attrs,
 SourceLocation *endLoc,
-LateParsedAttrList *LateAttrs,
-Declarator *D) {
+LateParsedAttrList *LateAttrs, Declarator *D) {
   assert(Tok.is(tok::kw___attribute) && "Not a GNU attribute list!");
 
+  SourceLocation StartLoc = Tok.getLocation(), Loc;
+
+  if (!endLoc)
+endLoc = &Loc;
+
   while (Tok.is(tok::kw___attribute)) {
 SourceLocation AttrTokLoc = ConsumeToken();
 unsigned OldNumAttrs = attrs.size();
@@ -260,6 +264,8 @@
   }
 }
   }
+
+  attrs.Range = SourceRange(StartLoc, *endLoc);
 }
 
 /// Determine whether the given attribute has an identifier argument.
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -1034,6 +1034,27 @@
   mutable AttributePool pool;
 };
 
+struct ParsedAttributesWithRange : ParsedAttributes {
+  ParsedAttributesWithRange(AttributeFactory &factory)
+  : ParsedAttributes(factory) {}
+
+  void clear() {
+ParsedAttributes::clear();
+Range = SourceRange();
+  }
+
+  SourceRange Range;
+};
+struct ParsedAttributesViewWithRange : ParsedAttributesView {
+  ParsedAttributesViewWithRange() : ParsedAttributesView() {}
+  void clearListOnly() {
+ParsedAttributesView::clearListOnly();
+Range = SourceRange();
+  }
+
+  SourceRange Range;
+};
+
 /// These constants match the enumerated choices of
 /// err_attribute_argument_n_type and err_attribute_argument_type.
 enum AttributeArgumentNType {
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -1572,27 +1572,6 @@
 
   //======//
   // C99 6.9: External Definitions.
-  struct ParsedAttributesWithRange : ParsedAttributes {
-ParsedAttributesWithRange(AttributeFactory &factory)
-  : ParsedAttributes(factory) {}
-
-void clear

[PATCH] D97264: [RISCV] Define types for Zvlsseg.

2021-04-01 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:1486
+  BuiltinType::Kind K, unsigned NF) {
+  auto TypeIter = llvm::find_if(Types, [&K](Type *Ty) {
+ if (Ty->isBuiltinType()) {

I'm confused by this lookup.

If you add `RvvInt8mf8Ty` in the macros above will you be able to use it for 
types such as `RvvInt8mf8x2Ty`, `RvvInt8mf8x3Ty`, etc. without having to search 
it in all the types?

AFAICT `ElemId` seems only used in this case so perhaps we can use the "name of 
the field of ASTContext" (a `CanQualType`) directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97264

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


[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

2021-04-01 Thread Balázs Kéri via Phabricator via cfe-commits
balazske reopened this revision.
balazske added a comment.
This revision is now accepted and ready to land.

Test should be updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98502

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


[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

2021-04-01 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 334609.
balazske added a comment.

Split the test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98502

Files:
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/pthreadlock_state.c
  clang/test/Analysis/pthreadlock_state_nottracked.c

Index: clang/test/Analysis/pthreadlock_state_nottracked.c
===
--- /dev/null
+++ clang/test/Analysis/pthreadlock_state_nottracked.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | FileCheck %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+#define NULL 0
+
+void clang_analyzer_printState();
+
+void test(pthread_mutex_t *mtx) {
+  int ret = pthread_mutex_destroy(mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "SymRegion{reg_$[[REG:[0-9]+]]}: not tracked, possibly destroyed",
+  // CHECK-NEXT:  "Mutexes in unresolved possibly destroyed state:",
+  // CHECK-NEXT:  "SymRegion{reg_$[[REG]]}: conj_$
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+  if (ret)
+return;
+  pthread_mutex_init(mtx, NULL);
+}
Index: clang/test/Analysis/pthreadlock_state.c
===
--- /dev/null
+++ clang/test/Analysis/pthreadlock_state.c
@@ -0,0 +1,60 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | FileCheck %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+#define NULL 0
+
+void clang_analyzer_printState();
+
+pthread_mutex_t mtx;
+
+void test() {
+  clang_analyzer_printState();
+  // CHECK:"checker_messages": null
+
+  pthread_mutex_init(&mtx, NULL);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  pthread_mutex_lock(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: locked",
+  // CHECK-NEXT:  "Mutex lock order:",
+  // CHECK-NEXT:  "mtx",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  pthread_mutex_unlock(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  int ret = pthread_mutex_destroy(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked, possibly destroyed",
+  // CHECK-NEXT:  "Mutexes in unresolved possibly destroyed state:",
+  // CHECK-NEXT:  "mtx: conj_$
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  if (ret)
+return;
+
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: destroyed",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -339,7 +339,16 @@
 }
   }
 
-  // TODO: Dump destroyed mutex symbols?
+  DestroyRetValTy DRV = State->get();
+  if (!DRV.isEmpty()) {
+Out << Sep << "Mutexes in unresolved possibly destroyed state:" << NL;
+for (auto I : DRV) {
+  I.first->dumpToStream(Out);
+  Out << ": ";
+  I.second->dumpToStream(Out);
+  Out << NL;
+}
+  }
 }
 
 void PthreadLockChecker::AcquirePthreadLock(const CallEvent &Call,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

2021-04-01 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision.
steakhal added inline comments.



Comment at: clang/test/Analysis/pthreadlock_state.c:1
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | 
FileCheck %s
+

AFAIK `core` should be enabled for all checks. Same for the other file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98502

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


Re: [clang] 9320ac9 - [Clang] Only run test when X86 backend is built.

2021-04-01 Thread Florian Hahn via cfe-commits
Hi,

On Tue, Mar 30, 2021 at 7:52 PM David Blaikie  wrote:

> Is there a more reliable remark that could be tested for? (Clang shouldn't
> be testing all remarks - just that the remark infrastructure in general is
> wired up (specific remarks should be tested in llvm) - so picking some
> really stable remark would be great)
>
> maybe there's a remark for "this always_inline thing can't be inlined
> because it's recursive" for instance?
>
>
That's a great point, there certainly are more stable remarks, e.g.
inlining as you suggested or GVN. I can add a separate test for that, so we
can still keep testing the vectorization remark. WDYT?

Cheers,
Florian

> On Mon, Mar 29, 2021 at 9:29 AM Florian Hahn via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>> Author: Florian Hahn
>> Date: 2021-03-29T17:27:01+01:00
>> New Revision: 9320ac9b4965d769632398b620ca3e4af9b56b12
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/9320ac9b4965d769632398b620ca3e4af9b56b12
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/9320ac9b4965d769632398b620ca3e4af9b56b12.diff
>>
>> LOG: [Clang] Only run test when X86 backend is built.
>>
>> After c773d0f97304 the remark is only emitted if the loop is profitable
>> to vectorize, but cannot be vectorized. Hence, it depends on
>> X86-specific cost-modeling.
>>
>> Added:
>>
>>
>> Modified:
>> clang/test/Frontend/optimization-remark-options.c
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/clang/test/Frontend/optimization-remark-options.c
>> b/clang/test/Frontend/optimization-remark-options.c
>> index 38dbbfbaccec0..f222eff37a5ef 100644
>> --- a/clang/test/Frontend/optimization-remark-options.c
>> +++ b/clang/test/Frontend/optimization-remark-options.c
>> @@ -1,3 +1,4 @@
>> +// REQUIRES: x86-registered-target
>>  // RUN: %clang -O1 -fvectorize -target x86_64-unknown-unknown
>> -Rpass-analysis=loop-vectorize -emit-llvm -S %s -o - 2>&1 | FileCheck %s
>>
>>  // CHECK: {{.*}}:9:11: remark: loop not vectorized: cannot prove it is
>> safe to reorder floating-point operations; allow reordering by specifying
>> '#pragma clang loop vectorize(enable)' before the loop or by providing the
>> compiler option '-ffast-math'.
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>

-- 

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


[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

2021-04-01 Thread Wang Tianqing via Phabricator via cfe-commits
tianqing created this revision.
tianqing added reviewers: pengfei, LuoYuanke, craig.topper.
Herald added a subscriber: hiraditya.
tianqing requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Add __uintr_frame structure and use UIRET instruction for functions with
x86 interrupt calling convention when UINTR is present.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99708

Files:
  clang/lib/Headers/uintrintrin.h
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/test/CodeGen/X86/x86-64-intrcc-uintr.ll

Index: llvm/test/CodeGen/X86/x86-64-intrcc-uintr.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/x86-64-intrcc-uintr.ll
@@ -0,0 +1,111 @@
+; RUN: llc < %s | FileCheck %s
+; RUN: llc -O0 < %s | FileCheck %s -check-prefix=CHECK0
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.__uintr_frame = type { i64, i64, i64 }
+
+; #include 
+;
+; void
+; __attribute__ ((interrupt))
+; test_uintr_isr_cc_empty(struct __uintr_frame *frame, unsigned long long uirrv)
+; {
+; }
+
+define dso_local x86_intrcc void @test_uintr_isr_cc_empty(%struct.__uintr_frame* nocapture byval(%struct.__uintr_frame) %frame, i64 %uirrv) #0 {
+; CHECK-LABEL: test_uintr_isr_cc_empty:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:pushq %rax
+; CHECK-NEXT:cld
+; CHECK-NEXT:addq $16, %rsp
+; CHECK-NEXT:uiret
+;
+; CHECK0-LABEL: test_uintr_isr_cc_empty:
+; CHECK0:   # %bb.0: # %entry
+; CHECK0-NEXT:pushq %rax
+; CHECK0-NEXT:cld
+; CHECK0-NEXT:addq $16, %rsp
+; CHECK0-NEXT:uiret
+entry:
+  ret void
+}
+
+; unsigned long long g_rip;
+; unsigned long long g_rflags;
+; unsigned long long g_rsp;
+; unsigned long long g_uirrv;
+;
+; void
+; __attribute__((interrupt))
+; test_uintr_isr_cc_args(struct __uintr_frame *frame, unsigned long long uirrv)
+; {
+;   g_rip = frame->rip;
+;   g_rflags = frame->rflags;
+;   g_rsp = frame->rsp;
+;   g_uirrv = uirrv;
+; }
+@g_rip = dso_local local_unnamed_addr global i64 0, align 8
+@g_rflags = dso_local local_unnamed_addr global i64 0, align 8
+@g_rsp = dso_local local_unnamed_addr global i64 0, align 8
+@g_uirrv = dso_local local_unnamed_addr global i64 0, align 8
+
+define dso_local x86_intrcc void @test_uintr_isr_cc_args(%struct.__uintr_frame* nocapture readonly byval(%struct.__uintr_frame) %frame, i64 %uirrv) #0 {
+; CHECK-LABEL: test_uintr_isr_cc_args:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:pushq %rax
+; CHECK-NEXT:pushq %rax
+; CHECK-NEXT:pushq %rdx
+; CHECK-NEXT:pushq %rcx
+; CHECK-NEXT:cld
+; CHECK-NEXT:movq 32(%rsp), %rax
+; CHECK-NEXT:movq 40(%rsp), %rcx
+; CHECK-NEXT:movq 48(%rsp), %rdx
+; CHECK-NEXT:movq %rcx, g_rip(%rip)
+; CHECK-NEXT:movq %rdx, g_rflags(%rip)
+; CHECK-NEXT:movq 56(%rsp), %rcx
+; CHECK-NEXT:movq %rcx, g_rsp(%rip)
+; CHECK-NEXT:movq %rax, g_uirrv(%rip)
+; CHECK-NEXT:popq %rcx
+; CHECK-NEXT:popq %rdx
+; CHECK-NEXT:popq %rax
+; CHECK-NEXT:addq $16, %rsp
+; CHECK-NEXT:uiret
+;
+; CHECK0-LABEL: test_uintr_isr_cc_args:
+; CHECK0:   # %bb.0: # %entry
+; CHECK0-NEXT:pushq %rax
+; CHECK0-NEXT:pushq %rax
+; CHECK0-NEXT:pushq %rdx
+; CHECK0-NEXT:pushq %rcx
+; CHECK0-NEXT:cld
+; CHECK0-NEXT:movq 32(%rsp), %rax
+; CHECK0-NEXT:leaq 40(%rsp), %rcx
+; CHECK0-NEXT:movq (%rcx), %rdx
+; CHECK0-NEXT:movq %rdx, g_rip(%rip)
+; CHECK0-NEXT:movq 8(%rcx), %rdx
+; CHECK0-NEXT:movq %rdx, g_rflags(%rip)
+; CHECK0-NEXT:movq 16(%rcx), %rcx
+; CHECK0-NEXT:movq %rcx, g_rsp(%rip)
+; CHECK0-NEXT:movq %rax, g_uirrv(%rip)
+; CHECK0-NEXT:popq %rcx
+; CHECK0-NEXT:popq %rdx
+; CHECK0-NEXT:popq %rax
+; CHECK0-NEXT:addq $16, %rsp
+; CHECK0-NEXT:uiret
+entry:
+  %rip = getelementptr inbounds %struct.__uintr_frame, %struct.__uintr_frame* %frame, i64 0, i32 0
+  %0 = load i64, i64* %rip, align 8
+  store i64 %0, i64* @g_rip, align 8
+  %rflags = getelementptr inbounds %struct.__uintr_frame, %struct.__uintr_frame* %frame, i64 0, i32 1
+  %1 = load i64, i64* %rflags, align 8
+  store i64 %1, i64* @g_rflags, align 8
+  %rsp = getelementptr inbounds %struct.__uintr_frame, %struct.__uintr_frame* %frame, i64 0, i32 2
+  %2 = load i64, i64* %rsp, align 8
+  store i64 %2, i64* @g_rsp, align 8
+  store i64 %uirrv, i64* @g_uirrv, align 8
+  ret void
+}
+
+attributes #0 = { nofree norecurse nounwind willreturn "disable-tail-calls"="true" "frame-pointer"="none" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+uintr" "tune-cpu"="generic" }
Index: llvm/lib/Target/X86/X86ExpandPseudo.cpp
===
--- llvm/lib/Target/X86/X86ExpandPseudo.cpp
+++ llvm/lib/Target/X86/X86ExpandPseudo.cpp
@@ -316,7 +316,9 @@
 X86FL->emitS

[PATCH] D99577: [RFC][OpenCL][PoC] Testing TableGen with diffing

2021-04-01 Thread Anton Zabaznov via Phabricator via cfe-commits
azabaznov added a comment.

Thanks for feedback!

Locally I get really nice result (I am using Ubuntu 20.04):

  $ llvm-lit  SemaOpenCL/compare-header-and-tablegen.cl
  
  -- Testing: 1 tests, 1 workers --
  FAIL: Clang :: SemaOpenCL/compare-header-and-tablegen.cl (1 of 1)
  
  Failed Tests (1):
Clang :: SemaOpenCL/compare-header-and-tablegen.cl
  
  Testing Time: 0.20s

I see no reason why it may take longer on other platforms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99577

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


[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

2021-04-01 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment.

In D52050#2662656 , @hvdijk wrote:

> I have also updated the summary to provide a more complete explanation of the 
> changes, and hope the revised summary will answer @MaskRay's questions.



In D52050#2662648 , @hvdijk wrote:

> In D52050#2662570 , @glaubitz wrote:
>
>> Since I am a big fan of consistency, I would rather leave it as is (4.6) and 
>> then bump to 10.2.0 in a follow-up commit.
>
> You are right that including the bump in this commit would either force an 
> inconsistency with basic_linux_tree, multilib_32bit_linux_tree, and 
> multilib_64bit_linux_tree, or include more changes in here that are not 
> related to x32. However, since we have been testing for x32 support against a 
> pretend installation of GCC 4.6, and GCC 4.6 actually never supported x32 
> (initial support was added in GCC 4.7), leaving it at 4.6.0 means we are 
> testing an impossible scenario. I am aware that we already were doing that 
> before as well, but I do not think that changes much. Personally, I am not 
> very happy with any of the options here.

I was actually wondering which version of GCC introduced x32 support after I 
posted that comment. You're right then, 4.6.0 makes no sense.

I think, however, we should bump the rest of the paths to 10.2.0 if possible.

> I am also a fan of consistency, so to me the least bad option seems to be to 
> update all four at the same time, provided this does not result in an 
> excessively large diff. It looks large-ish but not too bad to me; I will 
> include it in the next update so that we know what sort of size we are 
> dealing with. If you and @MaskRay think it is too large I can easily take it 
> out again.
>
>> Debian actually has a /libx32 folder, but it contains the dynamic loader 
>> only which makes sense because the path to the dynamic loader is baked into 
>> the executable if I remember correctly.
>
> Right, it has a /libx32 directory for exactly that reason. 
> /libx32/ld-linux-x32.so.2 only exists as a compatibility symlink though, the 
> dynamic loader is really stored in /lib/x86_64-linux-gnux32.

Yep, it's just a link - as for the other architectures in Debian as well.

Either way, I'm glad this has finally been approved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

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


[clang] 1d463c2 - [Driver] Fix architecture triplets and search paths for Linux x32

2021-04-01 Thread Harald van Dijk via cfe-commits

Author: Harald van Dijk
Date: 2021-04-01T09:47:56+01:00
New Revision: 1d463c2a386099597a8e2d26b9b964bc8fda8042

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

LOG: [Driver] Fix architecture triplets and search paths for Linux x32

Currently, support for the x32 ABI is handled as a multilib to the
x86_64 target only. However, full self-hosting x32 systems treating it
as a separate architecture with its own architecture triplets as well as
search paths exist as well, in Debian's x32 port and elsewhere.

This adds the missing architecture triplets and search paths so that
clang can work as a native compiler on x32, and updates the tests so
that they pass when using an x32 libdir suffix.

Additionally, we would previously also assume that objects from any
x86_64-linux-gnu GCC installation could be used to target x32. This
changes the logic so that only GCC installations that include x32
support are used when targetting x32, meaning x86_64-linux-gnux32 GCC
installations, and x86_64-linux-gnu and i686-linux-gnu GCC installations
that include x32 multilib support.

Reviewed By: MaskRay

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

Added: 

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/i386-unknown-linux-gnu/10.2.0/crtbegin.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/crtbegin.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/crtbeginT.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/crtfastmath.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/x32/crtbegin.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/x32/crtbeginT.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/x32/crtfastmath.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i386-unknown-linux/10.2.0/crtbegin.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/10.2.0/crtbegin.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/crtbegin.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/crtbeginT.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/crtfastmath.o

clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/lib/gcc/i386-unknown-linux/10.2.0/64/crtbegin.o

clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/lib/gcc/i386-unknown-linux/10.2.0/crtbegin.o

clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/32/crtbegin.o

clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/crtbegin.o

clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/x32/crtbegin.o

clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/10.2.0/32/crtbegin.o

clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/10.2.0/crtbegin.o

clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/10.2.0/x32/crtbegin.o

Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/test/Driver/baremetal.cpp
clang/test/Driver/cl-options.c
clang/test/Driver/cross-linux.c
clang/test/Driver/env.c
clang/test/Driver/linux-ld.c
clang/test/Preprocessor/iwithprefix.c

Removed: 

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/i386-unknown-linux-gnu/4.6.0/crtbegin.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/crtbegin.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/crtbeginT.o

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/crtfastmath.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i386-unknown-linux/4.6.0/crtbegin.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/4.6.0/crtbegin.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/crtbegin.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/crtbeginT.o

clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/crtfastmath.o

clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/lib/gcc/i386-unknown-linux/4.6.0/64/crtbegin.o

clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/lib/gcc/i386-unknown-linux/4.6.0/crtbegin.o

clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/32/crtbegin.o

clang/test/Driver/Inputs/multilib_64bit_linux_

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

2021-04-01 Thread Harald van Dijk 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 rG1d463c2a3860: [Driver] Fix architecture triplets and search 
paths for Linux x32 (authored by hvdijk).

Changed prior to commit:
  https://reviews.llvm.org/D52050?vs=334557&id=334617#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/i386-unknown-linux-gnu/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/i386-unknown-linux-gnu/4.6.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/crtbeginT.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/crtfastmath.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/x32/crtbegin.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/x32/crtbeginT.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/10.2.0/x32/crtfastmath.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/crtbeginT.o
  
clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/crtfastmath.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i386-unknown-linux/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i386-unknown-linux/4.6.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/4.6.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/crtbeginT.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/crtfastmath.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/crtbegin.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/crtbeginT.o
  
clang/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/crtfastmath.o
  
clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/lib/gcc/i386-unknown-linux/10.2.0/64/crtbegin.o
  
clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/lib/gcc/i386-unknown-linux/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/lib/gcc/i386-unknown-linux/4.6.0/64/crtbegin.o
  
clang/test/Driver/Inputs/multilib_32bit_linux_tree/usr/lib/gcc/i386-unknown-linux/4.6.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/10.2.0/x32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/lib/gcc/x86_64-unknown-linux/4.6.0/x32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/10.2.0/32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/10.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/10.2.0/x32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/4.6.0/32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/4.6.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_64bit_linux_tree/usr/libx32/gcc/x86_64-unknown-gnu/4.6.0/x32/crtbegin.o
  clang/test/Driver/baremetal.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/cross-linux.c
  clang/test/Driver/env.c
  clang/test/Driver/linux-ld.c
  clang/test/Preprocessor/iwithprefix.c

Index: clang/test/Preprocessor/iwithprefix.c
===
--- clang/test/Preprocessor/iwithprefix.c
+++ clang/test/Preprocessor/iwithprefix.c
@@ -9,8 +9,6 @@
 
 // CHECK: #include <...> search starts here:
 // CHECK: {{.*}}.tmps/first
-// CHECK: {{/|\\}}lib{{(32|64)?}}{{/|\\}}clang{{/|\\}}{{[.0-9]+}}{{/|\\}}include
+// CHECK: {{/|\\}}clang{{/|\\}}{{[.0-9]+}}{{/|\\}}include
 // CHEC

[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

2021-04-01 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 334619.
balazske added a comment.

Add 'core' to enabled checks in test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98502

Files:
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/pthreadlock_state.c
  clang/test/Analysis/pthreadlock_state_nottracked.c

Index: clang/test/Analysis/pthreadlock_state_nottracked.c
===
--- /dev/null
+++ clang/test/Analysis/pthreadlock_state_nottracked.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | FileCheck %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+#define NULL 0
+
+void clang_analyzer_printState();
+
+void test(pthread_mutex_t *mtx) {
+  int ret = pthread_mutex_destroy(mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "SymRegion{reg_$[[REG:[0-9]+]]}: not tracked, possibly destroyed",
+  // CHECK-NEXT:  "Mutexes in unresolved possibly destroyed state:",
+  // CHECK-NEXT:  "SymRegion{reg_$[[REG]]}: conj_$
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+  if (ret)
+return;
+  pthread_mutex_init(mtx, NULL);
+}
Index: clang/test/Analysis/pthreadlock_state.c
===
--- /dev/null
+++ clang/test/Analysis/pthreadlock_state.c
@@ -0,0 +1,60 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | FileCheck %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+#define NULL 0
+
+void clang_analyzer_printState();
+
+pthread_mutex_t mtx;
+
+void test() {
+  clang_analyzer_printState();
+  // CHECK:"checker_messages": null
+
+  pthread_mutex_init(&mtx, NULL);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  pthread_mutex_lock(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: locked",
+  // CHECK-NEXT:  "Mutex lock order:",
+  // CHECK-NEXT:  "mtx",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  pthread_mutex_unlock(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  int ret = pthread_mutex_destroy(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked, possibly destroyed",
+  // CHECK-NEXT:  "Mutexes in unresolved possibly destroyed state:",
+  // CHECK-NEXT:  "mtx: conj_$
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  if (ret)
+return;
+
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: destroyed",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -339,7 +339,16 @@
 }
   }
 
-  // TODO: Dump destroyed mutex symbols?
+  DestroyRetValTy DRV = State->get();
+  if (!DRV.isEmpty()) {
+Out << Sep << "Mutexes in unresolved possibly destroyed state:" << NL;
+for (auto I : DRV) {
+  I.first->dumpToStream(Out);
+  Out << ": ";
+  I.second->dumpToStream(Out);
+  Out << NL;
+}
+  }
 }
 
 void PthreadLockChecker::AcquirePthreadLock(const CallEvent &Call,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

2021-04-01 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment.

In D52050#2663205 , @glaubitz wrote:

> I think, however, we should bump the rest of the paths to 10.2.0 if possible.

I updated all the Linux trees that were on 4.6.0. The only remaining 4.6.0 
trees are for Hurd, which seems to me like it was just a coincidence that it 
was on the same version. There are other Linux trees used for tests as well, 
but they were already on different versions of GCC so we do not introduce any 
new inconsistencies by leaving those as they are.

I noticed that I left out one of the 4.6.0 directories by mistake in what I put 
up for review, that did not affect test results. I included that as obvious in 
what I pushed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

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


[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

2021-04-01 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment.

In D52050#2663247 , @hvdijk wrote:

> In D52050#2663205 , @glaubitz wrote:
>
>> I think, however, we should bump the rest of the paths to 10.2.0 if possible.
>
> I updated all the Linux trees that were on 4.6.0. The only remaining 4.6.0 
> trees are for Hurd, which seems to me like it was just a coincidence that it 
> was on the same version. There are other Linux trees used for tests as well, 
> but they were already on different versions of GCC so we do not introduce any 
> new inconsistencies by leaving those as they are.
>
> I noticed that I left out one of the 4.6.0 directories by mistake in what I 
> put up for review, that did not affect test results. I included that as 
> obvious in what I pushed.

OK. Glad this has been fixed, in any case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

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


[PATCH] D99711: [RISCV] [2/2] Add intrinsic for Zbc extension

2021-04-01 Thread LevyHsu via Phabricator via cfe-commits
LevyHsu created this revision.
LevyHsu added reviewers: craig.topper, kito-cheng, asb, jrtc27, Jim.
LevyHsu added projects: clang, LLVM.
Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, apazos, 
sameer.abuasal, s.egerton, benna, psnobl, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, niosHD, sabuasal, 
simoncook, johnrusso, rbar, hiraditya.
LevyHsu requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.

Head files are included in the second patch in case the name needs to be 
changed.

RV32 / 64:
clmul
clmulh
clmulr


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99711

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbc.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbc.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoB.td
  llvm/test/CodeGen/RISCV/rv32zbc-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zbc-intrinsic.ll

Index: llvm/test/CodeGen/RISCV/rv64zbc-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv64zbc-intrinsic.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-b -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64IB
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zbc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64IBC
+
+declare i64 @llvm.riscv.clmul.i64(i64 %a, i64 %b)
+
+define i64 @clmul64(i64 %a, i64 %b) nounwind {
+; RV64IB-LABEL: clmul64:
+; RV64IB:   # %bb.0:
+; RV64IB-NEXT:clmul a0, a0, a1
+; RV64IB-NEXT:ret
+;
+; RV64IBC-LABEL: clmul64:
+; RV64IBC:   # %bb.0:
+; RV64IBC-NEXT:clmul a0, a0, a1
+; RV64IBC-NEXT:ret
+  %tmp = call i64 @llvm.riscv.clmul.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.clmul.h.i64(i64 %a, i64 %b)
+
+define i64 @clmul64h(i64 %a, i64 %b) nounwind {
+; RV64IB-LABEL: clmul64h:
+; RV64IB:   # %bb.0:
+; RV64IB-NEXT:clmulh a0, a0, a1
+; RV64IB-NEXT:ret
+;
+; RV64IBC-LABEL: clmul64h:
+; RV64IBC:   # %bb.0:
+; RV64IBC-NEXT:clmulh a0, a0, a1
+; RV64IBC-NEXT:ret
+  %tmp = call i64 @llvm.riscv.clmul.h.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.clmul.r.i64(i64 %a, i64 %b)
+
+define i64 @clmul64r(i64 %a, i64 %b) nounwind {
+; RV64IB-LABEL: clmul64r:
+; RV64IB:   # %bb.0:
+; RV64IB-NEXT:clmulr a0, a0, a1
+; RV64IB-NEXT:ret
+;
+; RV64IBC-LABEL: clmul64r:
+; RV64IBC:   # %bb.0:
+; RV64IBC-NEXT:clmulr a0, a0, a1
+; RV64IBC-NEXT:ret
+  %tmp = call i64 @llvm.riscv.clmul.r.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
Index: llvm/test/CodeGen/RISCV/rv32zbc-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv32zbc-intrinsic.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-b -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32IB
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zbc -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32IBC
+
+declare i32 @llvm.riscv.clmul.i32(i32 %a, i32 %b)
+
+define i32 @clmul32(i32 %a, i32 %b) nounwind {
+; RV32IB-LABEL: clmul32:
+; RV32IB:   # %bb.0:
+; RV32IB-NEXT:clmul a0, a0, a1
+; RV32IB-NEXT:ret
+;
+; RV32IBC-LABEL: clmul32:
+; RV32IBC:   # %bb.0:
+; RV32IBC-NEXT:clmul a0, a0, a1
+; RV32IBC-NEXT:ret
+  %tmp = call i32 @llvm.riscv.clmul.i32(i32 %a, i32 %b)
+ ret i32 %tmp
+}
+
+declare i32 @llvm.riscv.clmul.h.i32(i32 %a, i32 %b)
+
+define i32 @clmul32h(i32 %a, i32 %b) nounwind {
+; RV32IB-LABEL: clmul32h:
+; RV32IB:   # %bb.0:
+; RV32IB-NEXT:clmulh a0, a0, a1
+; RV32IB-NEXT:ret
+;
+; RV32IBC-LABEL: clmul32h:
+; RV32IBC:   # %bb.0:
+; RV32IBC-NEXT:clmulh a0, a0, a1
+; RV32IBC-NEXT:ret
+  %tmp = call i32 @llvm.riscv.clmul.h.i32(i32 %a, i32 %b)
+ ret i32 %tmp
+}
+
+declare i32 @llvm.riscv.clmul.r.i32(i32 %a, i32 %b)
+
+define i32 @clmul32r(i32 %a, i32 %b) nounwind {
+; RV32IB-LABEL: clmul32r:
+; RV32IB:   # %bb.0:
+; RV32IB-NEXT:clmulr a0, a0, a1
+; RV32IB-NEXT:ret
+;
+; RV32IBC-LABEL: clmul32r:
+; RV32IBC:   # %bb.0:
+; RV32IBC-NEXT:clmulr a0, a0, a1
+; RV32IBC-NEXT:ret
+  %tmp = call i32 @llvm.riscv.clmul.r.i32(i32 %a, i32 %b)
+ ret i32 %tmp
+}
Index: llvm/lib/Target/RISCV/RISCVInstrInfoB.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfoB.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfoB.td
@@ -893,3 +893,9 @@
(srl (and GPR:$rs1, 0x), (i64 

[PATCH] D99190: WIP: [SYCL] Add design document for SYCL mode

2021-04-01 Thread Victor Lomuller via Phabricator via cfe-commits
Naghasan added inline comments.



Comment at: clang/docs/SYCLSupport.md:123
+traverse all symbols accessible from kernel functions and add them to the
+"device part" of the code marking them with the new SYCL device attribute.
+

ABataev wrote:
> bader wrote:
> > Naghasan wrote:
> > > OpenMP offload uses a similar approach isn't it? Might be worth to 
> > > describe how the 2 relates to each other and where they diverge. 
> > Do you mean the approach OpenMP compiler uses to outline single-source code 
> > parts to offload?
> > To be honest, I'm not sure... @ABataev, is there any description how OpenMP 
> > compiler outlines device code?
> > https://clang.llvm.org/docs/OpenMPSupport.html doesn't provide much details 
> > unfortunately.
> I don't think we have anything like this. Moreover, currently, there are 2 
> different models, one with outlining by the frontend and another one with the 
> outlining by the LLVM.
I mentioned that as I know there is some support for CUDA and the clang driver 
openmp offload works with multiple frontend passes.
If the model(s) is too different then there is no point going further here. 

> Moreover, currently, there are 2 different models, one with outlining by the 
> frontend and another one with the outlining by the LLVM.

I do recall some brief conversations about that. Are they meant to work in pair 
or one aims to replace the other ?



Comment at: clang/docs/SYCLSupport.md:161
+// Generated kernel function (expressed in OpenCL-like pseudo-code)
+__kernel KernelName(global int* a) {
+  KernelType KernelFuncObj; // Actually kernel function object declaration

bader wrote:
> Naghasan wrote:
> > This is missing  the template instantiation  that will eventually lead to 
> > that lowering.
> > 
> > I would also suggest to split code block in 2 as to mark what is in header 
> > and source file (SYCL code) and what is compiler generated (that pseudo 
> > OpenCL).
> > 
> > Might be good to also mention the glue generated in the integration header 
> > as this is what allows arguments to be set by the runtime (bridge between 
> > the structure in C++ and the SPIR-like kernel arguments).
> > This is missing  the template instantiation  that will eventually lead to 
> > that lowering.
> > I would also suggest to split code block in 2 as to mark what is in header 
> > and source file (SYCL code) and what is compiler generated (that pseudo 
> > OpenCL).
> 
> Do you suggest to sketch an SYCL kernel invocation API usage example with the 
> complete implementation of these methods to demonstrate the full stack? I was 
> trying to avoid runtime/header part of implementation in this section and 
> describe only API compiler relies on or provides for the runtime library. I 
> think these details were in this document before, but moved to 
> https://github.com/intel/llvm/blob/sycl/sycl/doc/KernelParameterPassing.md 
> referenced below. 
> 
> > Might be good to also mention the glue generated in the integration header 
> > as this is what allows arguments to be set by the runtime (bridge between 
> > the structure in C++ and the SPIR-like kernel arguments).
> 
> I've mentioned that, but I hope KernelParameterPassing.md document provides 
> the rest of the details.
> Is that okay?
> Do you suggest to sketch an SYCL kernel invocation API usage example with the 
> complete implementation of these methods to demonstrate the full stack?

Not the full stack, I agree that would be too much. Just the bare minimal that 
can show where the `global int* a` comes from and help connecting the dots. I 
agree details should be kept in the kernel param passing document.

So maybe consider adding something of those lines:

```
class MyObj {
  accessor _ptr; // accessor contains a pointer to the global address 
space.
public:

  void operator()(); // Body of the kernel
};

[...]
MyObj Obj{/*some init*/};
sycl_kernel_function(Obj);
```

This shows the instanciation and one can make the relation between this and the 
pseudo OpenCL kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99190

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


[clang] df4fa53 - [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

2021-04-01 Thread Balázs Kéri via cfe-commits

Author: Balázs Kéri
Date: 2021-04-01T11:59:00+02:00
New Revision: df4fa53fddb61c2514e7e09fb7cdde53edced958

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

LOG: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

Add printing of map 'DestroyRetVal'.

Reviewed By: steakhal

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

Added: 
clang/test/Analysis/pthreadlock_state.c
clang/test/Analysis/pthreadlock_state_nottracked.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
index 88e80c481a5a7..eb10a42b7d3d8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -339,7 +339,16 @@ void PthreadLockChecker::printState(raw_ostream &Out, 
ProgramStateRef State,
 }
   }
 
-  // TODO: Dump destroyed mutex symbols?
+  DestroyRetValTy DRV = State->get();
+  if (!DRV.isEmpty()) {
+Out << Sep << "Mutexes in unresolved possibly destroyed state:" << NL;
+for (auto I : DRV) {
+  I.first->dumpToStream(Out);
+  Out << ": ";
+  I.second->dumpToStream(Out);
+  Out << NL;
+}
+  }
 }
 
 void PthreadLockChecker::AcquirePthreadLock(const CallEvent &Call,

diff  --git a/clang/test/Analysis/pthreadlock_state.c 
b/clang/test/Analysis/pthreadlock_state.c
new file mode 100644
index 0..f6b26e4071b99
--- /dev/null
+++ b/clang/test/Analysis/pthreadlock_state.c
@@ -0,0 +1,60 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | 
FileCheck %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+#define NULL 0
+
+void clang_analyzer_printState();
+
+pthread_mutex_t mtx;
+
+void test() {
+  clang_analyzer_printState();
+  // CHECK:"checker_messages": null
+
+  pthread_mutex_init(&mtx, NULL);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  pthread_mutex_lock(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: locked",
+  // CHECK-NEXT:  "Mutex lock order:",
+  // CHECK-NEXT:  "mtx",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  pthread_mutex_unlock(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  int ret = pthread_mutex_destroy(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked, possibly destroyed",
+  // CHECK-NEXT:  "Mutexes in unresolved possibly destroyed state:",
+  // CHECK-NEXT:  "mtx: conj_$
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  if (ret)
+return;
+
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: destroyed",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+}

diff  --git a/clang/test/Analysis/pthreadlock_state_nottracked.c 
b/clang/test/Analysis/pthreadlock_state_nottracked.c
new file mode 100644
index 0..e6281372e214f
--- /dev/null
+++ b/clang/test/Analysis/pthreadlock_state_nottracked.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | 
FileCheck %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+#define NULL 0
+
+void clang_analyzer_printState();
+
+void test(pthread_mutex_t *mtx) {
+  int ret = pthread_mutex_destroy(mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "SymRegion{reg_$[[REG:[0-9]+]]}: 
not tracked, possibly destroyed",
+  // CHECK-NEXT:  "Mutexes in unresolved possibly destroyed state:",
+  // CHECK-NEXT:  "SymRegion{reg_$[[REG]]}: conj_$
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+  if (ret)
+return;
+  pthread_mutex_init(mtx, NULL);
+}



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


[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

2021-04-01 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf4fa53fddb6: [clang][Checkers] Extend PthreadLockChecker 
state dump (NFC). (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98502

Files:
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/pthreadlock_state.c
  clang/test/Analysis/pthreadlock_state_nottracked.c

Index: clang/test/Analysis/pthreadlock_state_nottracked.c
===
--- /dev/null
+++ clang/test/Analysis/pthreadlock_state_nottracked.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | FileCheck %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+#define NULL 0
+
+void clang_analyzer_printState();
+
+void test(pthread_mutex_t *mtx) {
+  int ret = pthread_mutex_destroy(mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "SymRegion{reg_$[[REG:[0-9]+]]}: not tracked, possibly destroyed",
+  // CHECK-NEXT:  "Mutexes in unresolved possibly destroyed state:",
+  // CHECK-NEXT:  "SymRegion{reg_$[[REG]]}: conj_$
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+  if (ret)
+return;
+  pthread_mutex_init(mtx, NULL);
+}
Index: clang/test/Analysis/pthreadlock_state.c
===
--- /dev/null
+++ clang/test/Analysis/pthreadlock_state.c
@@ -0,0 +1,60 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | FileCheck %s
+
+#include "Inputs/system-header-simulator-for-pthread-lock.h"
+
+#define NULL 0
+
+void clang_analyzer_printState();
+
+pthread_mutex_t mtx;
+
+void test() {
+  clang_analyzer_printState();
+  // CHECK:"checker_messages": null
+
+  pthread_mutex_init(&mtx, NULL);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  pthread_mutex_lock(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: locked",
+  // CHECK-NEXT:  "Mutex lock order:",
+  // CHECK-NEXT:  "mtx",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  pthread_mutex_unlock(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  int ret = pthread_mutex_destroy(&mtx);
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: unlocked, possibly destroyed",
+  // CHECK-NEXT:  "Mutexes in unresolved possibly destroyed state:",
+  // CHECK-NEXT:  "mtx: conj_$
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+
+  if (ret)
+return;
+
+  clang_analyzer_printState();
+  // CHECK:{ "checker": "alpha.core.PthreadLockBase", "messages": [
+  // CHECK-NEXT:  "Mutex states:",
+  // CHECK-NEXT:  "mtx: destroyed",
+  // CHECK-NEXT:  ""
+  // CHECK-NEXT:]}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -339,7 +339,16 @@
 }
   }
 
-  // TODO: Dump destroyed mutex symbols?
+  DestroyRetValTy DRV = State->get();
+  if (!DRV.isEmpty()) {
+Out << Sep << "Mutexes in unresolved possibly destroyed state:" << NL;
+for (auto I : DRV) {
+  I.first->dumpToStream(Out);
+  Out << ": ";
+  I.second->dumpToStream(Out);
+  Out << NL;
+}
+  }
 }
 
 void PthreadLockChecker::AcquirePthreadLock(const CallEvent &Call,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99712: [RISCV] [2/2] Add intrinsic for Zbc extension

2021-04-01 Thread LevyHsu via Phabricator via cfe-commits
LevyHsu created this revision.
LevyHsu added reviewers: craig.topper, kito-cheng, asb, jrtc27, Jim.
LevyHsu added projects: clang, LLVM.
Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, apazos, 
sameer.abuasal, usaxena95, s.egerton, benna, psnobl, kadircet, jocewei, PkmX, 
arphaman, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, 
shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, mgorny.
LevyHsu requested review of this revision.
Herald added a subscriber: cfe-commits.
Herald added a project: clang-tools-extra.

Head files are included in the second patch in case the name needs to be 
changed.

RV32 / 64:
clmul
clmulh
clmulr


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99712

Files:
  clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/riscv_zbc_intrin.h
  clang/lib/Headers/rvintrin.h
  clang/test/Headers/rvintrin.c

Index: clang/test/Headers/rvintrin.c
===
--- /dev/null
+++ clang/test/Headers/rvintrin.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple riscv32 -fsyntax-only \
+// RUN:   -target-feature +experimental-zbc %s
+// RUN: %clang_cc1 -triple riscv64 -fsyntax-only \
+// RUN:   -target-feature +experimental-zbc %s
+
+#include 
Index: clang/lib/Headers/rvintrin.h
===
--- /dev/null
+++ clang/lib/Headers/rvintrin.h
@@ -0,0 +1,26 @@
+/*=== rvintrin.h ---===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __RVINTRIN_H
+#define __RVINTRIN_H
+
+#define int_xlen_t long
+#define uint_xlen_t unsigned int_xlen_t
+
+#define __DEFAULT_FN_ATTRS \
+  __attribute__((__always_inline__, __artificial__, __nodebug__))
+
+#if defined(__riscv_zbc)
+#include "riscv_zbc_intrin.h"
+#endif
+
+#undef __DEFAULT_FN_ATTRS
+#undef uint_xlen_t
+#undef int_xlen_t
+#endif // __RVINTRIN_H
Index: clang/lib/Headers/riscv_zbc_intrin.h
===
--- /dev/null
+++ clang/lib/Headers/riscv_zbc_intrin.h
@@ -0,0 +1,38 @@
+/*=== riscv_zbc_intrin.h ---===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __RVINTRIN_H
+#error "Never use  directly;include  instead."
+#endif
+
+#ifndef __RISCV_ZBC_INTRIN_H
+#define __RISCV_ZBC_INTRIN_H
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+// Zbc
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS _rv_clmul(int_xlen_t rs1, int_xlen_t rs2) {
+  return __builtin_riscv_clmul(rs1, rs2);
+}
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS _rv_clmul_h(int_xlen_t rs1, int_xlen_t rs2) {
+  return __builtin_riscv_clmul_h(rs1, rs2);
+}
+
+static __inline__ int_xlen_t __DEFAULT_FN_ATTRS _rv_clmul_r(int_xlen_t rs1, int_xlen_t rs2) {
+  return __builtin_riscv_clmul_r(rs1, rs2);
+}
+
+#if defined(__cplusplus)
+}
+#endif // if defined(__cplusplus)
+
+#endif // __RISCV_ZBC_INTRIN_H
Index: clang/lib/Headers/CMakeLists.txt
===
--- clang/lib/Headers/CMakeLists.txt
+++ clang/lib/Headers/CMakeLists.txt
@@ -97,6 +97,8 @@
   ptwriteintrin.h
   rdseedintrin.h
   rtmintrin.h
+  rvintrin.h
+  riscv_zbc_intrin.h
   serializeintrin.h
   sgxintrin.h
   s390intrin.h
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -152,6 +152,8 @@
   {"include/prfchwintrin.h", ""},
   {"include/rdseedintrin.h", ""},
   {"include/rtmintrin.h", ""},
+  {"include/rvintrin.h", ""},
+  {"include/riscv_zbc_intrin.h", ""},
   {"include/shaintrin.h", ""},
   {"include/smmintrin.h", ""},
   {"include/stdalign.h", ""},
Index: clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
===
--- clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
+++ clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
@@ -57,6 +57,8 @@
   {"include/prfchwintrin.h$", ""},
   {"include/rdseedintrin.h$", ""},
  

[PATCH] D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions

2021-04-01 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.

LG!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99658

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


[PATCH] D99659: [analyzer][taint] Extent of heap regions should get taint sometimes

2021-04-01 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

I like it, looks good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99659

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


[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

2021-04-01 Thread Marco Gartmann via Phabricator via cfe-commits
mgartmann updated this revision to Diff 334632.
mgartmann marked an inline comment as done.
mgartmann added a comment.

Add isInStdNamespace to matcher so that only global objects in namespace `std` 
are matched and add corresponding tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-std-stream-objects-outside-main.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s misc-std-stream-objects-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+} // namespace std
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void problematic() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-std-stream-objects-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+}
+
+int main() {
+  std::cout << "This should not trigger the check"; // OK
+  std::wcout << "This should not trigger the check";// OK
+  std::cerr << "This should not trigger the check"; // OK
+  std::wcerr << "This should not trigger the check";// OK
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+  std::cin >> Foo; // OK
+  std::wcin >> Foo;// OK
+  arbitrary_namespace::cin >> Foo; // OK
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-std-stream-objects-outside-main.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - misc-std-stream-objects-outside-main
+
+misc-std-stream-objects-outside-main
+=
+
+Diagnoses if a predefined standard stream object (``cin``, ``wcin``,
+``cout``, ``wcout``, ``cerr`` or ``wcerr``) is used outside the ``main`` function.
+
+For instance, in the following code, the use of ``std::cout`` outside of ``main()`` would get
+flagged whereas the use of ``std::cout`` inside ``main()`` is not flagged:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function() {
+std::cout << "This triggers the check.";
+ 
+  }
+
+  int main() {
+std::cout << "This does not trigger the check.";
+  }
+
+Since the predefined standard stream objects are global objects, their use outside of ``main()`` worsens a
+program's testability and is thus discouraged. Instead, those objects should only be used inside ``main()``.
+They can then be passed as arguments to other functions like so:
+
+.. code-block:: c++
+
+  #include 
+
+  void some_function(std::istream & in, std::ostream & out) {
+out << "This does not trigger the check.

[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

2021-04-01 Thread Marco Gartmann via Phabricator via cfe-commits
mgartmann added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:25
+  .bind("match"),
+  this);
+}

mgartmann wrote:
> riccibruno wrote:
> > Will this match `my_namespace::cin`?
> Yes, at the moment this would be matched as well.
Thinking about it, in my opinion, only matching those objects if they are 
coming form the `std` namespace would make more sense and lead to less 
"false-positive" diagnostics. 
Furthermore, [[ https://www.cevelop.com/ | Cevelop IDE ]]'s check, which is 
where the idea for this check came form, also behaves like this.

Thus, I added the `isInStdNamespace()` matcher in the latest diff.

@riccibruno What is your opinion on this? Would it make more sense to also 
match e.g., `my_namespace::cin` in your opinion? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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


[PATCH] D99714: [clang][Analyzer] Handle flexible arrays better in ArrayBoundV2 checker.

2021-04-01 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is an experimental fix for ArrayBoundV2 checker to handle "flexible arrays"
(incomplete array) better. If incomplete array is found to be indexed, the
used size (for out-of-bounds check) will be the dynamic size of base region,
with offset (of the incomplete array) taken into account.
In this was indexing of incomplete arrays is not reported as error,
if the array was allocated using a fixed size.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99714

Files:
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/test/Analysis/array-bound-v2.c

Index: clang/test/Analysis/array-bound-v2.c
===
--- /dev/null
+++ clang/test/Analysis/array-bound-v2.c
@@ -0,0 +1,48 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,alpha.security.ArrayBoundV2 %s
+
+//#include 
+//#include "Inputs/system-header-simulator-cxx.h"
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+struct FlexArr {
+  int n;
+  int s0[2];
+  int s[];
+};
+
+struct Outer {
+  int n;
+  struct FlexArr FA;
+};
+
+void test_flex() {
+  struct FlexArr *p = (struct FlexArr *)malloc(sizeof(struct FlexArr) + sizeof(int) * 10);
+  p->n = 10;
+  p->s[0] = 0;
+  p->s[9] = 9;
+  p->s[10] = 10; // expected-waring{{Out of bound memory access}}
+}
+
+void test_simple() {
+  struct FlexArr *p = (struct FlexArr *)malloc(sizeof(struct FlexArr) + sizeof(int) * 10);
+  p->n = 10;
+  p->s0[0] = 0;
+  p->s0[1] = 1;
+  p->s0[2] = 2; // expected-waring{{Out of bound memory access}}
+}
+
+void test_outer_flex() {
+  struct Outer *p = (struct Outer *)malloc(sizeof(struct Outer) + sizeof(int) * 10);
+  p->FA.s[0] = 0;
+  p->FA.s[9] = 9;
+  p->FA.s[10] = 10; // expected-waring{{Out of bound memory access}}
+}
+
+void test_outer_simple() {
+  struct Outer *p = (struct Outer *)malloc(sizeof(struct Outer) + sizeof(int) * 10);
+  p->FA.s0[0] = 0;
+  p->FA.s0[1] = 1;
+  p->FA.s0[2] = 2; // expected-waring{{Out of bound memory access}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -113,6 +113,30 @@
   return std::pair(offset, extent);
 }
 
+namespace {
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const MemRegion *MRegion) {
+  SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();
+  if (!MRegion)
+return UnknownVal();
+  RegionOffset Offset = MRegion->getAsOffset();
+  if (Offset.hasSymbolicOffset())
+return UnknownVal();
+  const MemRegion *BaseRegion = MRegion->getBaseRegion();
+  if (!BaseRegion)
+return UnknownVal();
+
+  NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
+  Offset.getOffset() /
+  MRegion->getMemRegionManager().getContext().getCharWidth());
+  DefinedOrUnknownSVal ExtentInBytes =
+  getDynamicSize(State, BaseRegion, SvalBuilder);
+
+  return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
+   ExtentInBytes, OffsetInBytes,
+   SvalBuilder.getArrayIndexType());
+}
+} // namespace
+
 void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
 const Stmt* LoadS,
 CheckerContext &checkerContext) const {
@@ -137,6 +161,11 @@
 
   NonLoc rawOffsetVal = rawOffset.getByteOffset();
 
+  bool IsIncompleteArray = false;
+  if (const auto *ASExpr = dyn_cast_or_null(LoadS))
+IsIncompleteArray = isa(
+ASExpr->getBase()->IgnoreParenImpCasts()->getType().getCanonicalType());
+
   // CHECK LOWER BOUND: Is byteOffset < extent begin?
   //  If so, we are doing a load/store
   //  before the first valid offset in the memory region.
@@ -180,6 +209,13 @@
 // we are doing a load/store after the last valid offset.
 const MemRegion *MR = rawOffset.getRegion();
 DefinedOrUnknownSVal Size = getDynamicSize(state, MR, svalBuilder);
+
+if (IsIncompleteArray) {
+  SVal DSWithOffset = getDynamicSizeWithOffset(state, MR);
+  if (auto Val = DSWithOffset.getAs())
+Size = *Val;
+}
+
 if (!Size.getAs())
   break;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99160: [X86][FastISEL] Support DW_TAG_call_site_parameter with FastISEL

2021-04-01 Thread Alok Kumar Sharma via Phabricator via cfe-commits
alok updated this revision to Diff 334636.
alok added a comment.

Re-based and minor changes.


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

https://reviews.llvm.org/D99160

Files:
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/test/CodeGen/X86/call-site-info-output.ll
  llvm/test/DebugInfo/X86/callsitepar-fastisel.ll

Index: llvm/test/DebugInfo/X86/callsitepar-fastisel.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/callsitepar-fastisel.ll
@@ -0,0 +1,200 @@
+;; This test checks if DW_TAG_GNU_call_site_parameter is generated for fastIsel.
+; REQUIRES: x86_64-linux
+;RUN: llc -mtriple=x86_64-pc-linux-gnu -emit-call-site-info -fast-isel=true -filetype=obj %s -o - | llvm-dwarfdump - | FileCheck %s
+;CHECK: DW_TAG_GNU_call_site
+;CHECK: DW_AT_abstract_origin
+;CHECK-SAME: "foo"
+;CHECK: DW_AT_low_pc
+;CHECK: DW_TAG_GNU_call_site_parameter
+;CHECK: DW_AT_location (DW_OP_reg4 RSI)
+;CHECK: DW_AT_GNU_call_site_value
+
+;;The IR is generated from below source program
+;
+;;subroutine foo (array)
+;;  integer :: array(:,:)
+;;  array(:,:) = 5
+;;  array(1,1) = 30
+;;end subroutine
+;;
+;;program vla_sub
+;;  interface
+;;subroutine foo (array)
+;;  integer :: array (:,:)
+;;end subroutine
+;;  end interface
+;;
+;;  integer :: sub_arr(42,42)
+;;  sub_arr(:,:) = 1
+;;  call foo(sub_arr)
+;;end program vla_sub
+;
+
+; ModuleID = 'fast.ll'
+source_filename = "/tmp/fast.ll"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.BSS2 = type <{ [7056 x i8] }>
+
+@.BSS2 = internal global %struct.BSS2 zeroinitializer, align 32, !dbg !0
+@.C359_MAIN_ = internal constant i64 42
+@.C333_MAIN_ = internal constant i64 1
+@.C368_MAIN_ = internal constant i64 4
+@.C367_MAIN_ = internal constant i64 25
+@.C331_MAIN_ = internal constant i64 0
+@.C330_MAIN_ = internal constant i32 0
+
+; Function Attrs: nofree norecurse nounwind
+define void @foo_(i64* noalias nocapture %array, i64* noalias nocapture readonly %"array$sd") local_unnamed_addr !dbg !15 {
+L.entry:
+  call void @llvm.dbg.value(metadata i64* %array, metadata !28, metadata !DIExpression()), !dbg !29
+  call void @llvm.dbg.declare(metadata i64* %"array$sd", metadata !30, metadata !DIExpression()), !dbg !29
+  %0 = getelementptr i64, i64* %"array$sd", i64 11
+  %1 = load i64, i64* %0, align 8
+  call void @llvm.dbg.value(metadata i64 %1, metadata !21, metadata !DIExpression()), !dbg !29
+  %2 = getelementptr i64, i64* %"array$sd", i64 20
+  %3 = load i64, i64* %2, align 8
+  %4 = getelementptr i64, i64* %"array$sd", i64 17
+  %5 = load i64, i64* %4, align 8
+  call void @llvm.dbg.value(metadata i64 %5, metadata !24, metadata !DIExpression()), !dbg !29
+  %6 = getelementptr i64, i64* %"array$sd", i64 7
+  %7 = load i64, i64* %6, align 8
+  %8 = getelementptr i64, i64* %"array$sd", i64 16
+  %9 = load i64, i64* %8, align 8
+  %10 = add nsw i64 %9, -1
+  %11 = mul nsw i64 %10, %3
+  %12 = getelementptr i64, i64* %"array$sd", i64 10
+  %13 = load i64, i64* %12, align 8
+  %14 = add i64 %7, -1
+  %15 = add i64 %14, %13
+  %16 = add i64 %15, %11
+  %17 = icmp slt i64 %1, 1, !dbg !35
+  %18 = icmp slt i64 %5, 1, !dbg !35
+  %19 = or i1 %17, %18, !dbg !35
+  br i1 %19, label %L.LB1_372, label %L.LB1_371.preheader, !dbg !35
+
+L.LB1_371.preheader:  ; preds = %L.entry
+  %20 = bitcast i64* %array to i8*
+  %21 = getelementptr i8, i8* %20, i64 -4
+  %22 = bitcast i8* %21 to i32*
+  br label %L.LB1_371
+
+L.LB1_371:; preds = %L.LB1_371.preheader, %L.LB1_426
+  %"i$a_368.0" = phi i64 [ %28, %L.LB1_426 ], [ 1, %L.LB1_371.preheader ], !dbg !35
+  %23 = mul nsw i64 %"i$a_368.0", %3
+  %24 = add i64 %23, %16
+  br label %L.LB1_374
+
+L.LB1_374:; preds = %L.LB1_374, %L.LB1_371
+  %"i$b_369.0" = phi i64 [ 1, %L.LB1_371 ], [ %27, %L.LB1_374 ], !dbg !35
+  %25 = add i64 %24, %"i$b_369.0", !dbg !35
+  %26 = getelementptr i32, i32* %22, i64 %25, !dbg !35
+  store i32 5, i32* %26, align 4, !dbg !35
+  %27 = add nuw i64 %"i$b_369.0", 1, !dbg !35
+  %exitcond.not = icmp eq i64 %"i$b_369.0", %1, !dbg !35
+  br i1 %exitcond.not, label %L.LB1_426, label %L.LB1_374, !dbg !35
+
+L.LB1_426:; preds = %L.LB1_374
+  %28 = add nuw i64 %"i$a_368.0", 1, !dbg !35
+  %exitcond9.not = icmp eq i64 %"i$a_368.0", %5, !dbg !35
+  br i1 %exitcond9.not, label %L.LB1_372, label %L.LB1_371, !dbg !35
+
+L.LB1_372:; preds = %L.LB1_426, %L.entry
+  %29 = add nsw i64 %16, %3, !dbg !36
+  %30 = bitcast i64* %array to i32*, !dbg !36
+  %31 = getelementptr i32, i32* %30, i64 %29, !dbg !36
+  store i32 30, i32* %31, align 4, !dbg !36
+  ret void, !dbg 

[PATCH] D99715: [CMake] Respect LLVM_MINIMUM_PYTHON_VERSION in Tooling/CMakeLists.txt

2021-04-01 Thread Dominik Montada via Phabricator via cfe-commits
gargaroff created this revision.
gargaroff added reviewers: serge-sans-paille, dblaikie, jyknight, JDevlieghere, 
yln, efriedma, ctetreau.
Herald added a subscriber: mgorny.
gargaroff requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This will override the found Python3 from llvm/CMakeLists.txt and cause
tests to fail if that version turns out to be lower than the minimum
version required to run tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99715

Files:
  clang/lib/Tooling/CMakeLists.txt


Index: clang/lib/Tooling/CMakeLists.txt
===
--- clang/lib/Tooling/CMakeLists.txt
+++ clang/lib/Tooling/CMakeLists.txt
@@ -13,7 +13,7 @@
 add_subdirectory(DependencyScanning)
 add_subdirectory(Transformer)
 
-find_package(Python3 COMPONENTS Interpreter)
+find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION} COMPONENTS Interpreter)
 
 # Replace the last lib component of the current binary directory with include
 string(FIND ${CMAKE_CURRENT_BINARY_DIR} "/lib/" PATH_LIB_START REVERSE)


Index: clang/lib/Tooling/CMakeLists.txt
===
--- clang/lib/Tooling/CMakeLists.txt
+++ clang/lib/Tooling/CMakeLists.txt
@@ -13,7 +13,7 @@
 add_subdirectory(DependencyScanning)
 add_subdirectory(Transformer)
 
-find_package(Python3 COMPONENTS Interpreter)
+find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION} COMPONENTS Interpreter)
 
 # Replace the last lib component of the current binary directory with include
 string(FIND ${CMAKE_CURRENT_BINARY_DIR} "/lib/" PATH_LIB_START REVERSE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D95635: [CMake] Require python 3.6 if enabling LLVM test targets

2021-04-01 Thread Dominik Montada via Phabricator via cfe-commits
gargaroff added a comment.

This still breaks for us. The `find_package(Python3 ...)` from 
`Tooling/CMakeLists.txt` does not look for the minimum version and overrides 
the version that was already found through `llvm/CMakeLists.txt`. I opened 
D99715  to fix this and added the original 
reviewers of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95635

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


[PATCH] D99715: [CMake] Respect LLVM_MINIMUM_PYTHON_VERSION in Tooling/CMakeLists.txt

2021-04-01 Thread Dominik Montada via Phabricator via cfe-commits
gargaroff added a comment.

Although I find it strange, that `find_package` is called again, even though it 
is already set through `llvm/CMakeLists.txt`, this patch does not try to 
refactor this in any way. The sole purpose is to make tests executable again in 
our downstream setup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99715

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


[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

2021-04-01 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:25
+  .bind("match"),
+  this);
+}

mgartmann wrote:
> mgartmann wrote:
> > riccibruno wrote:
> > > Will this match `my_namespace::cin`?
> > Yes, at the moment this would be matched as well.
> Thinking about it, in my opinion, only matching those objects if they are 
> coming form the `std` namespace would make more sense and lead to less 
> "false-positive" diagnostics. 
> Furthermore, [[ https://www.cevelop.com/ | Cevelop IDE ]]'s check, which is 
> where the idea for this check came form, also behaves like this.
> 
> Thus, I added the `isInStdNamespace()` matcher in the latest diff.
> 
> @riccibruno What is your opinion on this? Would it make more sense to also 
> match e.g., `my_namespace::cin` in your opinion? 
That was my point. I don't think that `my_namespace::cin` should be matched.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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


[PATCH] D99238: [DebugInfo] Enable the call site parameter feature by default

2021-04-01 Thread Alok Kumar Sharma via Phabricator via cfe-commits
alok added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1645
 
-  if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
+  if (Opts.hasReducedDebugInfo() &&
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))

djtodoro wrote:
> I am a bit confused now :)
> Is `flang` using this clang's library?
> I think we should be careful here, since I am not sure C/C++ -O0 code has any 
> benefits of using this, but the DWARF size might be increased insanely.
Classic flang calls same library. Below is the verbose output of "flang" 
command.
**flang** -g -O0 -v vla-sub.f90
 "/tmp/bin/**flang1**" vla-sub.f90 -x 47 0x1000 -opt 0 -terse 1 -inform 
warn -nohpf -nostatic -cmdline "'+flang -g -O0 -v vla-sub.f90'" -inform warn 
-disable-vectorize-pragmas -x 120 0x100 -debug -y 129 2 -x 19 0x40 
-quad -x 7 0x10 -x 68 0x1 -x 59 4 -x 15 2 -x 49 0x44 -x 51 0x20 -x 57 
0x48 -x 58 0x1 -x 124 0x1000 -tp px -x 57 0xfb -x 58 0x78031040 -x 47 
0x08 -x 48 4608 -x 49 0x100 -stdinc 
/tmp/bin/../include:/usr/local/include:/tmp/d/lib/clang/13.0.0/include:/usr/include/x86_64-linux-gnu:/include:/usr/include
 -def unix -def __unix -def __unix__ -def linux -def __linux -def __linux__ 
-def __NO_MATH_INLINES -def __LP64__ -def __LONG_MAX__=9223372036854775807L 
-def "__SIZE_TYPE__=unsigned long int" -def "__PTRDIFF_TYPE__=long int" -def 
__x86_64 -def __x86_64__ -def __amd_64__amd64__ -def __k8 -def __k8__ -def 
__THROW= -def __extension__= -def __PGLLVM__ -freeform -vect 48 -x 54 1 -x 70 
0x4000 -y 163 0xc000 -x 189 0x10 -stbfile vla-sub-051e06.stb -modexport 
vla-sub-051e06.cmod -modindex vla-sub-051e06.cmdx -output vla-sub-051e06.ilm
 "/tmp/bin/**flang2**" vla-sub-051e06.ilm -y 129 2 -x 6 0x100 -x 42 0x40 -y 
129 4 -x 129 0x400 -y 216 1 -ieee 1 -fn vla-sub.f90 -opt 0 -terse 1 -inform 
warn -cmdline "'+flang -g -O0 -v vla-sub.f90'" -inform warn 
-disable-vectorize-pragmas -x 120 0x100 -debug -y 129 2 -x 68 0x1 -x 51 
0x20 -x 119 0xa1 -x 122 0x40 -x 123 0x1000 -x 127 4 -x 127 17 -x 19 
0x40 -x 28 0x4 -x 120 0x1000 -x 70 0x8000 -x 122 1 -x 125 0x2 
-x 164 0x80 -quad -x 59 4 -tp px -x 120 0x1000 -x 124 0x1400 -y 15 2 -x 57 
0x3b -x 58 0x4800 -x 49 0x100 -astype 0 -x 183 4 -x 121 0x800 -x 54 
0x10 -x 70 0x4000 -x 249 1023 -x 124 1 -y 163 0xc000 -x 189 0x10 -y 189 
0x400 -x 183 0x10 -stbfile vla-sub-051e06.stb -asm /tmp/vla-sub-051e06.ll
 "/tmp/bin/**clang-12**" -cc1 -triple x86_64-unknown-linux-gnu -emit-obj 
-mrelax-all --mrelax-relocations -disable-free -main-file-name vla-sub.f90 
-mrelocation-model static -mframe-pointer=all -fmath-errno -fno-rounding-math 
-mconstructor-aliases -munwind-tables -target-cpu x86-64 -tune-cpu generic 
-debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -v 
-fcoverage-compilation-dir=/tmp/build.shared.d -resource-dir 
/tmp/lib/clang/13.0.0 -O0 -fdebug-compilation-dir=/tmp/build.shared.d 
-ferror-limit 19 -fgnuc-version=4.2.1 -fcolor-diagnostics -itodcalls 
-itodcallsbyclone -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o 
/tmp/vla-sub-cdf94f.o -x ir /tmp/vla-sub-051e06.ll
clang -cc1 version 13.0.0 based upon LLVM 13.0.0git default target 
x86_64-unknown-linux-gnu
 "/tmp/bin/**ld.lld**" -z relro --hash-style=gnu --eh-frame-hdr -m elf_x86_64 
-dynamic-linker /lib64/ld-linux-x86-64.so.2 -o a.out 
/usr/lib/x86_64-linux-gnu/crt1.o /usr/lib/x86_64-linux-gnu/crti.o 
/usr/lib/gcc/x86_64-linux-gnu/7.5.0/crtbegin.o 
-L/usr/lib/gcc/x86_64-linux-gnu/7.5.0 -L/lib/x86_64-linux-gnu -L/lib/../lib64 
-L/usr/lib/x86_64-linux-gnu -L/usr/lib/gcc/x86_64-linux-gnu/7.5.0/../../.. 
-L/home/alok/amdllvm/install/d/bin/../lib -L/lib -L/usr/lib 
/tmp/vla-sub-cdf94f.o -lflangmain -lflang -lflangrti -lpgmath -lquadmath 
-lompstub -lm -lrt -lpthread -rpath /tmp/bin/../lib -lgcc --as-needed -lgcc_s 
--no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed 
/usr/lib/gcc/x86_64-linux-gnu/7.5.0/crtend.o /usr/lib/x86_64-linux-gnu/crtn.o


Yes, I agree. I think there should be cases when even in C/C++ code this will 
be beneficial but not sure how frequent it shall be. We need to think about 
trade-off. Do you think it should be enabled only for Fortran or a command line 
option should be introduced to enable this otherwise it should disabled by 
default ? 



Comment at: llvm/test/DebugInfo/X86/callsitepar-fastisel.ll:12
 
+;; This test checks if DW_TAG_GNU_call_site_parameter is generated for option 
-O0
+;RUN: clang -cc1 -triple x86_64-unknown-linux-gnu -emit-obj  -main-file-name 
%s -debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -O0 -x ir %s 
-o - | llvm-dwarfdump - | FileCheck %s --check-prefix=CLANG

djtodoro wrote:
> I don't think this should be tested this way. We should be testing `Driver` 
> itself.
Sure, I shall remove this test from here.


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

https://revi

[PATCH] D99238: [DebugInfo] Enable the call site parameter feature by default

2021-04-01 Thread Alok Kumar Sharma via Phabricator via cfe-commits
alok updated this revision to Diff 334642.
alok added a comment.

Updated to address comments from @djtodoro


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

https://reviews.llvm.org/D99238

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Target/TargetOptions.h


Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -303,8 +303,8 @@
 std::shared_ptr BBSectionsFuncListBuf;
 
 /// The flag enables call site info production. It is used only for debug
-/// info, and it is restricted only to optimized code. This can be used for
-/// something else, so that should be controlled in the frontend.
+/// info. This can be used for something else, so that should be controlled
+/// in the frontend.
 unsigned EmitCallSiteInfo : 1;
 /// Set if the target supports the debug entry values by default.
 unsigned SupportsDebugEntryValues : 1;
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1645,7 +1645,7 @@
   llvm::Triple::arm, llvm::Triple::armeb, llvm::Triple::mips,
   llvm::Triple::mipsel, llvm::Triple::mips64, llvm::Triple::mips64el};
 
-  if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
+  if (Opts.hasReducedDebugInfo() &&
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EmitCallSiteInfo = true;
 
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -68,8 +68,7 @@
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
-CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info only in the case of
-   ///< '-g' + 'O>0' level.
+CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info.
 CODEGENOPT(EnableDIPreservationVerify, 1, 0) ///< Enable di preservation verify
  ///< each (it means check
  ///< the original debug info


Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -303,8 +303,8 @@
 std::shared_ptr BBSectionsFuncListBuf;
 
 /// The flag enables call site info production. It is used only for debug
-/// info, and it is restricted only to optimized code. This can be used for
-/// something else, so that should be controlled in the frontend.
+/// info. This can be used for something else, so that should be controlled
+/// in the frontend.
 unsigned EmitCallSiteInfo : 1;
 /// Set if the target supports the debug entry values by default.
 unsigned SupportsDebugEntryValues : 1;
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1645,7 +1645,7 @@
   llvm::Triple::arm, llvm::Triple::armeb, llvm::Triple::mips,
   llvm::Triple::mipsel, llvm::Triple::mips64, llvm::Triple::mips64el};
 
-  if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
+  if (Opts.hasReducedDebugInfo() &&
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EmitCallSiteInfo = true;
 
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -68,8 +68,7 @@
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
-CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info only in the case of
-   ///< '-g' + 'O>0' level.
+CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info.
 CODEGENOPT(EnableDIPreservationVerify, 1, 0) ///< Enable di preservation verify
  ///< each (it means check
  ///< the original debug info
___
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

2021-04-01 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added a comment.

I'm not working on this currently, but I would appreciate and support such 
change. It's a big diff, but IMHO it is the cleanest, most maintainable 
approach.

Gui, WDYT?


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] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

Ericson2314 wrote:
> ldionne wrote:
> > compnerd wrote:
> > > Ericson2314 wrote:
> > > > compnerd wrote:
> > > > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can 
> > > > > altering the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> > > > It is sometimes modified to be per target when multiple targets are 
> > > > being used at once. All things `CMAKE_INSTALL_*` are globally scoped so 
> > > > in general the combination builds are quite awkward.
> > > > 
> > > > (Having worked on Meson, I am really missing 
> > > > https://mesonbuild.com/Subprojects.html which is exactly what's needed 
> > > > to do this without these bespoke idioms that never work well enough . 
> > > > Alas...)
> > > I don't think that bringing up other build systems is particularly 
> > > helpful.
> > > 
> > > I do expect it to be modified, and I suspect that this is used 
> > > specifically for builds that @ldionne supports.
> > Actually, I've never used it myself, but @phosek seems to be using it for 
> > the Runtimes build to output one set of headers for each target, as 
> > mentioned above.
> > 
> > It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when driving 
> > the libc++ build from the runtimes build would be more in-line with the 
> > CMake way of doing things (one configuration == one build), but I'm curious 
> > to hear what @phosek has to say about that.
> > It seems to me that tweaking CMAKE_INSTALL_PREFIX globally when driving the 
> > libc++ build from the runtimes build would be more in-line with the CMake 
> > way of doing things (one configuration == one buid)
> 
> You mean trying to mutate it during the libc++ CMake eval and then set it 
> back after? That would keep the intended meaning of  `CMAKE_INSTALL_PREFIX` 
> being the actual prefix, but that feels awfully fragile to me. Or do you mean 
> something else?
I keep forgetting that the runtimes build uses `add_subdirectory` to include 
each sub-project instead of driving a proper CMake build for each of those.

Basically, what I'm saying is that whatever place we decide to build for N 
multiple targets, we should perform N different CMake builds setting the 
appropriate `CMAKE_INSTALL_PREFIX`, one for each target. But that discussion 
should happen elsewhere, not on this review.

As far as this review is concerned, I do believe you want instead:

```
${CMAKE_INSTALL_INCLUDEDIR}${LIBCXX_INSTALL_HEADER_PREFIX}
```

(reversed the order of variables)

We should have @phosek confirm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-01 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

ldionne wrote:
> Ericson2314 wrote:
> > ldionne wrote:
> > > compnerd wrote:
> > > > Ericson2314 wrote:
> > > > > compnerd wrote:
> > > > > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can 
> > > > > > altering the value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
> > > > > It is sometimes modified to be per target when multiple targets are 
> > > > > being used at once. All things `CMAKE_INSTALL_*` are globally scoped 
> > > > > so in general the combination builds are quite awkward.
> > > > > 
> > > > > (Having worked on Meson, I am really missing 
> > > > > https://mesonbuild.com/Subprojects.html which is exactly what's 
> > > > > needed to do this without these bespoke idioms that never work well 
> > > > > enough . Alas...)
> > > > I don't think that bringing up other build systems is particularly 
> > > > helpful.
> > > > 
> > > > I do expect it to be modified, and I suspect that this is used 
> > > > specifically for builds that @ldionne supports.
> > > Actually, I've never used it myself, but @phosek seems to be using it for 
> > > the Runtimes build to output one set of headers for each target, as 
> > > mentioned above.
> > > 
> > > It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when driving 
> > > the libc++ build from the runtimes build would be more in-line with the 
> > > CMake way of doing things (one configuration == one build), but I'm 
> > > curious to hear what @phosek has to say about that.
> > > It seems to me that tweaking CMAKE_INSTALL_PREFIX globally when driving 
> > > the libc++ build from the runtimes build would be more in-line with the 
> > > CMake way of doing things (one configuration == one buid)
> > 
> > You mean trying to mutate it during the libc++ CMake eval and then set it 
> > back after? That would keep the intended meaning of  `CMAKE_INSTALL_PREFIX` 
> > being the actual prefix, but that feels awfully fragile to me. Or do you 
> > mean something else?
> I keep forgetting that the runtimes build uses `add_subdirectory` to include 
> each sub-project instead of driving a proper CMake build for each of those.
> 
> Basically, what I'm saying is that whatever place we decide to build for N 
> multiple targets, we should perform N different CMake builds setting the 
> appropriate `CMAKE_INSTALL_PREFIX`, one for each target. But that discussion 
> should happen elsewhere, not on this review.
> 
> As far as this review is concerned, I do believe you want instead:
> 
> ```
> ${CMAKE_INSTALL_INCLUDEDIR}${LIBCXX_INSTALL_HEADER_PREFIX}
> ```
> 
> (reversed the order of variables)
> 
> We should have @phosek confirm.
@ldionne `CMAKE_INSTALL_PREFIX` cannot be used for this purpose. When using the 
multiarch layout with the runtimes build, we want to place build artifacts in 
`${BUILD_DIR}/{include,lib}//...` and install them to 
`${CMAKE_INSTALL_PREFIX}/{include,lib}//...`. There are two issues:
1. `CMAKE_INSTALL_PREFIX` only comes into play during install step, but we want 
the build layout to match the installation layout so we need a different 
variable to control the output directory.
2. We already use `CMAKE_INSTALL_PREFIX` for the toolchain itself and there's 
no way to use it hierarchically, that is you cannot do something like 
`${SUPER_CMAKE_INSTALL_PREFIX}/{include,lib}/${CMAKE_INSTALL_PREFIX}/...` which 
is why we need a separate variable to control the installation directory.

Regarding `LIBCXX_INSTALL_HEADER_PREFIX`, I haven't found any current uses of 
that variable. I introduced it in D59168 which was an earlier attempt at having 
per-target headers, but D89013 is strictly better for the reasons I described 
in https://reviews.llvm.org/D89013#2662578 (no duplicate headers) so I think we 
can just remove this variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-01 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+DESTINATION 
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
 COMPONENT cxx-headers

phosek wrote:
> ldionne wrote:
> > Ericson2314 wrote:
> > > ldionne wrote:
> > > > compnerd wrote:
> > > > > Ericson2314 wrote:
> > > > > > compnerd wrote:
> > > > > > > @ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used?  Can 
> > > > > > > altering the value of `CMAKE_INSTALL_INCLUDEDIR` serve the 
> > > > > > > purpose?
> > > > > > It is sometimes modified to be per target when multiple targets are 
> > > > > > being used at once. All things `CMAKE_INSTALL_*` are globally 
> > > > > > scoped so in general the combination builds are quite awkward.
> > > > > > 
> > > > > > (Having worked on Meson, I am really missing 
> > > > > > https://mesonbuild.com/Subprojects.html which is exactly what's 
> > > > > > needed to do this without these bespoke idioms that never work well 
> > > > > > enough . Alas...)
> > > > > I don't think that bringing up other build systems is particularly 
> > > > > helpful.
> > > > > 
> > > > > I do expect it to be modified, and I suspect that this is used 
> > > > > specifically for builds that @ldionne supports.
> > > > Actually, I've never used it myself, but @phosek seems to be using it 
> > > > for the Runtimes build to output one set of headers for each target, as 
> > > > mentioned above.
> > > > 
> > > > It seems to me that tweaking `CMAKE_INSTALL_PREFIX` globally when 
> > > > driving the libc++ build from the runtimes build would be more in-line 
> > > > with the CMake way of doing things (one configuration == one build), 
> > > > but I'm curious to hear what @phosek has to say about that.
> > > > It seems to me that tweaking CMAKE_INSTALL_PREFIX globally when driving 
> > > > the libc++ build from the runtimes build would be more in-line with the 
> > > > CMake way of doing things (one configuration == one buid)
> > > 
> > > You mean trying to mutate it during the libc++ CMake eval and then set it 
> > > back after? That would keep the intended meaning of  
> > > `CMAKE_INSTALL_PREFIX` being the actual prefix, but that feels awfully 
> > > fragile to me. Or do you mean something else?
> > I keep forgetting that the runtimes build uses `add_subdirectory` to 
> > include each sub-project instead of driving a proper CMake build for each 
> > of those.
> > 
> > Basically, what I'm saying is that whatever place we decide to build for N 
> > multiple targets, we should perform N different CMake builds setting the 
> > appropriate `CMAKE_INSTALL_PREFIX`, one for each target. But that 
> > discussion should happen elsewhere, not on this review.
> > 
> > As far as this review is concerned, I do believe you want instead:
> > 
> > ```
> > ${CMAKE_INSTALL_INCLUDEDIR}${LIBCXX_INSTALL_HEADER_PREFIX}
> > ```
> > 
> > (reversed the order of variables)
> > 
> > We should have @phosek confirm.
> @ldionne `CMAKE_INSTALL_PREFIX` cannot be used for this purpose. When using 
> the multiarch layout with the runtimes build, we want to place build 
> artifacts in `${BUILD_DIR}/{include,lib}//...` and install them to 
> `${CMAKE_INSTALL_PREFIX}/{include,lib}//...`. There are two issues:
> 1. `CMAKE_INSTALL_PREFIX` only comes into play during install step, but we 
> want the build layout to match the installation layout so we need a different 
> variable to control the output directory.
> 2. We already use `CMAKE_INSTALL_PREFIX` for the toolchain itself and there's 
> no way to use it hierarchically, that is you cannot do something like 
> `${SUPER_CMAKE_INSTALL_PREFIX}/{include,lib}/${CMAKE_INSTALL_PREFIX}/...` 
> which is why we need a separate variable to control the installation 
> directory.
> 
> Regarding `LIBCXX_INSTALL_HEADER_PREFIX`, I haven't found any current uses of 
> that variable. I introduced it in D59168 which was an earlier attempt at 
> having per-target headers, but D89013 is strictly better for the reasons I 
> described in https://reviews.llvm.org/D89013#2662578 (no duplicate headers) 
> so I think we can just remove this variable.
I have sent D99697 for review which removes these variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-01 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
 get_compiler_rt_target(${arch} target)
-set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+set(${install_dir} 
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()

Ericson2314 wrote:
> ldionne wrote:
> > lebedev.ri wrote:
> > > This looks suspect
> > Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like 
> > elsewhere.
> See the non-line comment I responded to @lebidev.ri with. In sort if
> 
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
> 
> is a relative path, then we end up with
> 
> ```
> ${CMAKE_INSTALL_PREFIX}/${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
> 
> instead of 
> 
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
> 
> as we do with the other per-package prefixes. Also if `CMAKE_INSTALL_LIBDIR` 
> is already an absolute path, then
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target}
> ```
> is the same thing, and closer to the second than the first.
I'm not sure if that's desirable. I'm not sure if we still need 
`COMPILER_RT_INSTALL_PATH`. That variable is only used by 
`clang/runtime/CMakeLists.txt` which predates `runtimes/CMakeLists.txt` and was 
AFAIK only ever used by Apple. I think we should consider removing 
`COMPILER_RT_INSTALL_PATH`. We'll need to check if 
`clang/runtime/CMakeLists.txt` is still being used or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-01 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 updated this revision to Diff 334576.
Ericson2314 added a comment.

Put on top of D99697 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/lib/Headers/CMakeLists.txt
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/include/CMakeLists.txt
  compiler-rt/lib/dfsan/CMakeLists.txt
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/tools/f18/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  libc/CMakeLists.txt
  libc/lib/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/include/CMakeLists.txt
  libcxx/src/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  lld/tools/lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/tools/intel-features/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/AddSphinxTarget.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake
  llvm/docs/CMake.rst
  llvm/examples/Bye/CMakeLists.txt
  llvm/include/llvm/CMakeLists.txt
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/tools/lto/CMakeLists.txt
  llvm/tools/opt-viewer/CMakeLists.txt
  llvm/tools/remarks-shlib/CMakeLists.txt
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/ve/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/cmake/polly_macros.cmake
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -81,15 +83,15 @@
 install(EXPORT ParallelSTLTargets
 FILE ParallelSTLTargets.cmake
 NAMESPACE pstl::
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(FILES "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfig.cmake"
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -275,7 +275,7 @@
 install(DIRECTORY
   ${ISL_SOURCE_DIR}/include/
   ${ISL_BINARY_DIR}/include/
-  DESTINATION include/polly
+  DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/polly
   FILES_MATCHING
   PATTERN "*.h"
   PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/polly_macros.cmake
===
--- polly/cmake/polly_macros.cmake
+++ polly/cmake/polly_macros.cmake
@@ -44,8 +44,8 @@

[PATCH] D99400: [debug-info] support new tuning debugger type DBX for XCOFF DWARF

2021-04-01 Thread ChenZheng 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 rGbfcd21876adc: [debug-info] support new tuning debugger type 
DBX for XCOFF DWARF (authored by shchenz).

Changed prior to commit:
  https://reviews.llvm.org/D99400?vs=333505&id=334583#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99400

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AIX.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/debug-options.c
  llvm/include/llvm/Target/TargetOptions.h

Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -105,10 +105,11 @@
   /// in DwarfDebug; if a given feature has a more specific command-line option,
   /// that option should take precedence over the tuning.
   enum class DebuggerKind {
-Default,  // No specific tuning requested.
-GDB,  // Tune debug info for gdb.
-LLDB, // Tune debug info for lldb.
-SCE   // Tune debug info for SCE targets (e.g. PS4).
+Default, ///< No specific tuning requested.
+GDB, ///< Tune debug info for gdb.
+LLDB,///< Tune debug info for lldb.
+SCE, ///< Tune debug info for SCE targets (e.g. PS4).
+DBX  ///< Tune debug info for dbx.
   };
 
   /// Enable abort calls when global instruction selection fails to lower/select
Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -18,6 +18,8 @@
 // RUN: | FileCheck -check-prefix=G_STANDALONE -check-prefix=G_LLDB %s
 // RUN: %clang -### -c -gsce %s -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_SCE %s
+// RUN: %clang -### -c -gdbx %s -target x86_64-linux-gnu 2>&1 \
+// RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
 
 // Android.
 // Android should always generate DWARF4.
@@ -102,6 +104,12 @@
 // RUN: %clang -### -c %s -gsce -target x86_64-unknown-linux 2>&1 \
 // RUN: | FileCheck -check-prefix=NOCI %s
 
+// On the AIX, -g defaults to -gdbx and limited debug info.
+// RUN: %clang -### -c -g %s -target powerpc-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
+// RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
@@ -294,6 +302,7 @@
 // G_GDB:  "-debugger-tuning=gdb"
 // G_LLDB: "-debugger-tuning=lldb"
 // G_SCE:  "-debugger-tuning=sce"
+// G_DBX:  "-debugger-tuning=dbx"
 //
 // G_NOTUNING: "-cc1"
 // G_NOTUNING-NOT: "-debugger-tuning="
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -550,6 +550,8 @@
   CmdArgs.push_back("-plugin-opt=-debugger-tune=lldb");
 else if (A->getOption().matches(options::OPT_gsce))
   CmdArgs.push_back("-plugin-opt=-debugger-tune=sce");
+else if (A->getOption().matches(options::OPT_gdbx))
+  CmdArgs.push_back("-plugin-opt=-debugger-tune=dbx");
 else
   CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb");
   }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1051,6 +1051,9 @@
   case llvm::DebuggerKind::SCE:
 CmdArgs.push_back("-debugger-tuning=sce");
 break;
+  case llvm::DebuggerKind::DBX:
+CmdArgs.push_back("-debugger-tuning=dbx");
+break;
   default:
 break;
   }
@@ -3895,6 +3898,8 @@
 DebuggerTuning = llvm::DebuggerKind::LLDB;
   else if (A->getOption().matches(options::OPT_gsce))
 DebuggerTuning = llvm::DebuggerKind::SCE;
+  else if (A->getOption().matches(options::OPT_gdbx))
+DebuggerTuning = llvm::DebuggerKind::DBX;
   else
 DebuggerTuning = llvm::DebuggerKind::GDB;
 }
Index: clang/lib/Driver/ToolChains/AIX.h
===
--- clang/lib/Driver/ToolChains/AIX.h
+++ clang/lib/Driver/ToolChains/AIX.h
@@ -77,6 +77,10 @@
   // Set default DWARF version to 3 for now as latest AIX OS supports version 3.
   unsigned GetDefaultDwarfVersion() const override { return 3; }
 
+  llvm::DebuggerKind getDefaultDe

[PATCH] D99400: [debug-info] support new tuning debugger type DBX for XCOFF DWARF

2021-04-01 Thread ChenZheng via Phabricator via cfe-commits
shchenz added inline comments.



Comment at: llvm/include/llvm/Target/TargetOptions.h:112
+SCE,  // Tune debug info for SCE targets (e.g. PS4).
+DBX   // Tune debug info for dbx.
   };

aprantl wrote:
> Nit: It would be nice to reformat all these comments for Doxygen:
> `DBX   ///< Tune debug info for dbx.`
Thanks. Done in the commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99400

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


[PATCH] D99703: [debug-info][XCOFF] set `-gno-column-info` by default for DBX

2021-04-01 Thread ChenZheng via Phabricator via cfe-commits
shchenz created this revision.
shchenz added reviewers: dblaikie, aprantl, jsji, probinson, echristo, PowerPC.
shchenz added a project: debug-info.
shchenz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For DBX, it does not handle column info well. Set `-gno-column-info` by default 
for DBX.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99703

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -110,6 +110,16 @@
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff 2>&1 \
 // RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
 
+// On the AIX, -g defaults to -gno-column-info.
+// RUN: %clang -### -c -g %s -target powerpc-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=NOCI %s
+// RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=NOCI %s
+// RUN: %clang -### -c -g %s -target powerpc-ibm-aix-xcoff -gcolumn-info 2>&1 \
+// RUN: | FileCheck -check-prefix=CI %s
+// RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff -gcolumn-info \
+// RUN: 2>&1 | FileCheck -check-prefix=CI %s
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3970,12 +3970,15 @@
   // Column info is included by default for everything except SCE and
   // CodeView. Clang doesn't track end columns, just starting columns, which,
   // in theory, is fine for CodeView (and PDB).  In practice, however, the
-  // Microsoft debuggers don't handle missing end columns well, so it's better
-  // not to include any column info.
+  // Microsoft debuggers don't handle missing end columns well, and the AIX
+  // debugger DBX also doesn't handle the columns well, so it's bettre not to
+  // include any column info.
   if (const Arg *A = Args.getLastArg(options::OPT_gcolumn_info))
 (void)checkDebugInfoOption(A, Args, D, TC);
   if (!Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-!EmitCodeView && DebuggerTuning != 
llvm::DebuggerKind::SCE))
+!EmitCodeView &&
+(DebuggerTuning != llvm::DebuggerKind::SCE &&
+ DebuggerTuning != llvm::DebuggerKind::DBX)))
 CmdArgs.push_back("-gno-column-info");
 
   // FIXME: Move backend command line options to the module.


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -110,6 +110,16 @@
 // RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff 2>&1 \
 // RUN: | FileCheck -check-prefix=G_LIMITED -check-prefix=G_DBX %s
 
+// On the AIX, -g defaults to -gno-column-info.
+// RUN: %clang -### -c -g %s -target powerpc-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=NOCI %s
+// RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff 2>&1 \
+// RUN: | FileCheck -check-prefix=NOCI %s
+// RUN: %clang -### -c -g %s -target powerpc-ibm-aix-xcoff -gcolumn-info 2>&1 \
+// RUN: | FileCheck -check-prefix=CI %s
+// RUN: %clang -### -c -g %s -target powerpc64-ibm-aix-xcoff -gcolumn-info \
+// RUN: 2>&1 | FileCheck -check-prefix=CI %s
+
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3970,12 +3970,15 @@
   // Column info is included by default for everything except SCE and
   // CodeView. Clang doesn't track end columns, just starting columns, which,
   // in theory, is fine for CodeView (and PDB).  In practice, however, the
-  // Microsoft debuggers don't handle missing end columns well, so it's better
-  // not to include any column info.
+  // Microsoft debuggers don't handle missing end columns well, and the AIX
+  // debugger DBX also doesn't handle the columns well, so it's bettre not to
+  // include any column info.
   if (const Arg *A = Args.getLastArg(options::OPT_gcolumn_info))
 (void)checkDebugInfoOption(A, Args, D, TC);
   if (!Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-!EmitCodeView && DebuggerTuning != llvm::DebuggerKind::SCE))
+!EmitCodeView &&
+(Debugg

[PATCH] D99668: [RISCV][Clang] Add some RVV Floating-Point intrinsic functions.

2021-04-01 Thread Zakk Chen via Phabricator via cfe-commits
khchen updated this revision to Diff 334644.
khchen added a comment.

Fix vfrdiv.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99668

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfdiv.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmul.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfrdiv.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfrsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfsgnj.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfdiv.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmul.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfrdiv.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfrsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfsgnj.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfsub.c

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


[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

2021-04-01 Thread Serguei Katkov via Phabricator via cfe-commits
skatkov added a comment.

First iteration: style comments.




Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:124
   if (BBGuards)
 for (auto &BB : *BBGuards) {
   if (BB.second.isPoisoned())

redundant {}
consider using any_of



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:1017
 
+struct PreservedCFGCheckerAnalysis
+: public AnalysisInfoMixin {

Describe the idea of this analisys


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

https://reviews.llvm.org/D91327

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


[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

2021-04-01 Thread Serguei Katkov via Phabricator via cfe-commits
skatkov added a comment.

Consider landing tests update for "-verify-cfg-preserved=0" in a separate 
commit. This will significantly reduce the size of review.


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

https://reviews.llvm.org/D91327

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


[PATCH] D99488: [SYCL][Doc] Add address space handling section to SYCL documentation

2021-04-01 Thread Victor Lomuller via Phabricator via cfe-commits
Naghasan added inline comments.



Comment at: clang/docs/SYCLSupport.md:909
+| `__attribute__((opencl_local))` | local_space |
+| `__attribute__((opencl_private))` | private_space |
+

bader wrote:
> Anastasia wrote:
> > Since SYCL spec has constant AS you should explain whether it is going to 
> > be supported or not and if so then how.
> The first raw of this table covers mapping between SYCL constant_space and 
> address space attribute.
> Could you clarify what else do we need?
To be more specific here, the OpenCL constant address space no longer have an 
equivalent in the SYCL core spec memory model 
https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#_sycl_device_memory_model.

The `multi_ptr` for that address space is now deprecated and implementors can 
map it the global address space (as described here).



Comment at: clang/docs/SYCLSupport.md:914-919
+Default address space represents "Generic-memory", which is a virtual address
+space which overlaps the global, local and private address spaces. SYCL mode
+enables conversion to/from default address space from/to address space
+attributed type.
+
+SPIR target allocates SYCL namespace scope variables in global address space.

bader wrote:
> bader wrote:
> > bader wrote:
> > > Anastasia wrote:
> > > > Naghasan wrote:
> > > > > aaron.ballman wrote:
> > > > > > 
> > > > > I think this section should be extended.
> > > > > 
> > > > > Pointers to `Default` address space should get lowered into a pointer 
> > > > > to a generic address space (or flat to reuse more general 
> > > > > terminology).
> > > > > But depending on the allocation context, the `default` address space 
> > > > > of a non-pointer type is assigned to a specific address space. This 
> > > > > is described in 
> > > > > https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#subsec:commonAddressSpace.
> > > > > 
> > > > > This is also in line with the behaviour of CUDA (small example 
> > > > > https://godbolt.org/z/veqTfo9PK).
> > > > Ok, if the implementation plans to follow the spec precisely then just 
> > > > adding a reference should be sufficient. 
> > > > 
> > > > Do I understand it correctly that your implementation will use the 
> > > > first approach from the two described in:
> > > > > If the target of the SYCL backend can represent the generic address 
> > > > > space, then the "common address space deduction rules" in Section 
> > > > > 5.9.2 and the "generic as default address space rules" in Section 
> > > > > 5.9.3 apply. If the target of the SYCL backend cannot represent the 
> > > > > generic address space, then the "common address space deduction 
> > > > > rules" in Section 5.9.2 and the "inferred address space rules" in 
> > > > > Section 5.9.4 apply.
> > > > 
> > > > This should be added to the documentation btw.
> > > > 
> > > > 
> > > > Btw does this statement in any way relate to the following statement:
> > > > 
> > > > > Within kernels, the underlying C++ pointer types can be obtained from 
> > > > > an accessor. The pointer types will contain a compile-time deduced 
> > > > > address space. So, for example, if a C++ pointer is obtained from an 
> > > > > accessor to global memory, the C++ pointer type will have a global 
> > > > > address space attribute attached to it. The address space attribute 
> > > > > will be compile-time propagated to other pointer values when one 
> > > > > pointer is initialized to another pointer value using a defined 
> > > > > algorithm.
> > > > 
> > > > from 
> > > > https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#_access_to_memory
> > > > 
> > > > Or if not where can I find the algorithm it refers to?
> > > > I think this section should be extended.
> > > > 
> > > > Pointers to `Default` address space should get lowered into a pointer 
> > > > to a generic address space (or flat to reuse more general terminology).
> > > > But depending on the allocation context, the `default` address space of 
> > > > a non-pointer type is assigned to a specific address space. This is 
> > > > described in 
> > > > https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#subsec:commonAddressSpace.
> > > > 
> > > > This is also in line with the behaviour of CUDA (small example 
> > > > https://godbolt.org/z/veqTfo9PK).
> > > 
> > > I've added this text to the document.
> > > 
> > > > Ok, if the implementation plans to follow the spec precisely then just 
> > > > adding a reference should be sufficient. 
> > > > 
> > > > Do I understand it correctly that your implementation will use the 
> > > > first approach from the two described in:
> > > > > If the target of the SYCL backend can represent the generic address 
> > > > > space, then the "common address space deduction rules" in Section 
> > > > > 5.9.2 and the "generic as default address space rules" in Section 
> > > > > 5.9.3 apply. If the targ

[PATCH] D99714: [clang][Analyzer] Handle flexible arrays better in ArrayBoundV2 checker.

2021-04-01 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

The tests are really promising! :)




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:117
+namespace {
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const MemRegion *MRegion) 
{
+  SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();

Do we need an overload perhaps in `DynamicSize.h` of 
```
SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV)
``` 
that takes a MemRegion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99714

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


[PATCH] D99526: [RISCV][Clang] Add RVV Widening Integer Add/Subtract intrinsic functions.

2021-04-01 Thread Zakk Chen via Phabricator via cfe-commits
khchen added inline comments.



Comment at: clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vwadd.c:35
+vint16mf4_t test_vwadd_vx_i16mf4(vint8mf8_t op1, int8_t op2, size_t vl) {
+  return vwadd_vx(op1, op2, vl);
+}

craig.topper wrote:
> Why do scalars require _wx or _vx, but vector don't need a suffix?
https://github.com/riscv/rvv-intrinsic-doc/blob/master/rvv-intrinsic-rfc.md#widening-vector-scalar-arithmetic-instructions
There is different overloading naming rule for _wx and _vx, and I just keep the 
default naming rule for vector version because I felt the overloading rule did 
not care about the naming consistent. 

But in https://github.com/riscv/rvv-intrinsic-doc/pull/76 we start to make 
overloading name seems more consistent. So I think making this case consistent 
is ok to me.

I could send a PR to intrinsic-doc, what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99526

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


[PATCH] D99526: [RISCV][Clang] Add RVV Widening Integer Add/Subtract intrinsic functions.

2021-04-01 Thread Zakk Chen via Phabricator via cfe-commits
khchen updated this revision to Diff 334647.
khchen added a comment.

Address Craig's comments. Sorry for a lot of typos.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99526

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vwadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vwsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vwadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vwsub.c

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


[PATCH] D99526: [RISCV][Clang] Add RVV Widening Integer Add/Subtract intrinsic functions.

2021-04-01 Thread Zakk Chen via Phabricator via cfe-commits
khchen updated this revision to Diff 334648.
khchen added a comment.

update missed part for Log2LMUL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99526

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vwadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vwsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vwadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vwsub.c

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


[PATCH] D99689: [OPENMP]Add option -fopenmp-cuda-const-firstprivate to control address space of the corresponding global.

2021-04-01 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D99689#2662875 , @jdoerfert wrote:

> In D99689#2662860 , @ABataev wrote:
>
>> In D99689#2662856 , @jdoerfert 
>> wrote:
>>
>>> In D99689#2662852 , @ABataev wrote:
>>>
 In D99689#2662848 , @jdoerfert 
 wrote:

> Can you please show me a test case or explain to me when/how this global 
> is actually used.

 It is passed as an argument to the target region. When libomptarget 
 requests the memory for the firstprivate, it returns the pointer to this 
 const global, which then passed as argument to the kernel.
>>>
>>> So if we use it, why would we disable it?
>>
>> With this  new option you can control how to handle it. You can either 
>> dynamically allocate memory using libomptarget memmanager (default for this 
>> option) or use preallocated constant memory, if you're not going to remove 
>> the var constantness.
>
> I get what the new option does, what I want to know is why we would ever want 
> to disable the constant memory usage. Is it potentially slower or otherwise 
> problematic?

Because you mentioned that it would not work if we would like to use 
`const_cast` to remove constness. It should be faster than the global memory, 
that's why I introduced this feature.

> Also, I am not sure but I imagine the generated code would be better if we 
> would use the constant global directly, or, add the address space to the 
> corresponding kernel argument.

Ye, but it will require some rework in the libomptarget and the compiler 
itself. Instead, I just reused the existing mechanism to allocate the memory in 
the constant pool and use it instead of the global.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99689

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


[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thank you for picking this back up!




Comment at: clang/include/clang/Parse/Parser.h:2708
+
   bool MaybeParseGNUAttributes(ParsedAttributes &attrs,
SourceLocation *endLoc = nullptr,

aaron.ballman wrote:
> We might as well take the opportunity to fix the style issues.
Can we add a comment above here saying that use of this interface is 
discouraged and callers should use the interface that takes the attributes with 
range information. Eventually, we should be getting rid of the range-less 
parsing functionality.



Comment at: clang/include/clang/Parse/Parser.h:2708-2714
   bool MaybeParseGNUAttributes(ParsedAttributes &attrs,
SourceLocation *endLoc = nullptr,
LateParsedAttrList *LateAttrs = nullptr) {
+if (Tok.is(tok::kw___attribute)) {
+  ParsedAttributesWithRange attrsWithRange(AttrFactory);
+  ParseGNUAttributes(attrs, endLoc, LateAttrs);
+  attrs.takeAllFrom(attrsWithRange);

We might as well take the opportunity to fix the style issues.



Comment at: clang/include/clang/Parse/Parser.h:2720-2721
+
+  bool MaybeParseGNUAttributes(ParsedAttributesWithRange &attrs,
+   SourceLocation *endLoc = nullptr,
+   LateParsedAttrList *LateAttrs = nullptr) {





Comment at: clang/include/clang/Parse/Parser.h:2730
+
   void ParseGNUAttributes(ParsedAttributes &attrs,
+  SourceLocation *endLoc = nullptr,

aaron.ballman wrote:
> Same
A similar comment here would be handy.



Comment at: clang/include/clang/Parse/Parser.h:2730-2734
   void ParseGNUAttributes(ParsedAttributes &attrs,
+  SourceLocation *endLoc = nullptr,
+  LateParsedAttrList *LateAttrs = nullptr,
+  Declarator *D = nullptr) {
+ParsedAttributesWithRange attrsWithRange(AttrFactory);

Same



Comment at: clang/include/clang/Parse/Parser.h:2739-2740
+
+  void ParseGNUAttributes(ParsedAttributesWithRange &attrs,
   SourceLocation *endLoc = nullptr,
   LateParsedAttrList *LateAttrs = nullptr,





Comment at: clang/lib/Parse/ParseDecl.cpp:165-167
+void Parser::ParseGNUAttributes(ParsedAttributesWithRange &attrs,
 SourceLocation *endLoc,
+LateParsedAttrList *LateAttrs, Declarator *D) {

Don't feel obligated to fix the naming style issues here -- that's quite a bit 
more churn. But if you did fix it, I wouldn't complain either.



Comment at: clang/test/AST/sourceranges.cpp:112
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_case
+namespace attributed_case {
+void f(int n) {

This test looks to be failing the CI pipeline currently.


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

https://reviews.llvm.org/D75844

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


[PATCH] D99715: [CMake] Respect LLVM_MINIMUM_PYTHON_VERSION in Tooling/CMakeLists.txt

2021-04-01 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment.

Did you try just removing that `find_package`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99715

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


[PATCH] D99669: [RISCV][Clang] Add more RVV Floating-Point intrinsic functions.

2021-04-01 Thread Zakk Chen via Phabricator via cfe-commits
khchen updated this revision to Diff 334659.
khchen added a comment.

rebase and refine multiclass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99669

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmul.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfeq.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfgt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfle.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmflt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfne.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwmul.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmfeq.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmfge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmfgt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmfle.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmflt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmfne.c

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


[PATCH] D93095: Introduce -Wreserved-identifier

2021-04-01 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 334660.
serge-sans-paille added a comment.

Do not use lexical parent, as suggested by @rsmith 
Add test case for extern function worward-declared in function body, as 
suggested by @rsmith


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

https://reviews.llvm.org/D93095

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Sema/reserved-identifier.c
  clang/test/Sema/reserved-identifier.cpp

Index: clang/test/Sema/reserved-identifier.cpp
===
--- /dev/null
+++ clang/test/Sema/reserved-identifier.cpp
@@ -0,0 +1,87 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wreserved-identifier %s
+
+int foo__bar() { return 0; }// expected-warning {{identifier 'foo__bar' is reserved because it contains '__'}}
+static int _bar() { return 0; } // expected-warning {{identifier '_bar' is reserved because it starts with '_' at global scope}}
+static int _Bar() { return 0; } // expected-warning {{identifier '_Bar' is reserved because it starts with '_' followed by a capital letter}}
+int _barbouille() { return 0; } // expected-warning {{identifier '_barbouille' is reserved because it starts with '_' at global scope}}
+
+void foo(unsigned int _Reserved) { // expected-warning {{identifier '_Reserved' is reserved because it starts with '_' followed by a capital letter}}
+  unsigned int __1 =   // expected-warning {{identifier '__1' is reserved because it starts with '__'}}
+  _Reserved;   // no-warning
+}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+template  constexpr bool __toucan = true; // expected-warning {{identifier '__toucan' is reserved because it starts with '__'}}
+
+template 
+concept _Barbotine = __toucan; // expected-warning {{identifier '_Barbotine' is reserved because it starts with '_' followed by a capital letter}}
+
+template  // expected-warning {{'__' is reserved because it starts with '__'}}
+struct BarbeNoire {};
+
+template  // no-warning
+struct BarbeJaune {};
+
+template  // expected-warning {{'__' is reserved because it starts with '__'}}
+void BarbeRousse() {}
+
+namespace _Barbidur { // expected-warning {{identifier '_Barbidur' is reserved because it starts with '_' followed by a capital letter}}
+
+struct __barbidou {}; // expected-warning {{identifier '__barbidou' is reserved because it starts with '__'}}
+struct _barbidou {};  // no-warning
+
+int __barbouille; // expected-warning {{identifier '__barbouille' is reserved because it starts with '__'}}
+int _barbouille;  // no-warning
+
+int __babar() { return 0; } // expected-warning {{identifier '__babar' is reserved because it starts with '__'}}
+int _babar() { return 0; }  // no-warning
+
+} // namespace _Barbidur
+
+class __barbapapa { // expected-warning {{identifier '__barbapapa' is reserved because it starts with '__'}}
+  void _barbabelle() {} // no-warning
+  int _Barbalala;   // expected-warning {{identifier '_Barbalala' is reserved because it starts with '_' followed by a capital letter}}
+};
+
+enum class __menu { // expected-warning {{identifier '__menu' is reserved because it starts with '__'}}
+  __some,   // expected-warning {{identifier '__some' is reserved because it starts with '__'}}
+  _Other,   // expected-warning {{identifier '_Other' is reserved because it starts with '_' followed by a capital letter}}
+  _other// no-warning
+};
+
+enum _Menu { // expected-warning {{identifier '_Menu' is reserved because it starts with '_' followed by a capital letter}}
+  _OtheR_,   // expected-warning {{identifier '_OtheR_' is reserved because it starts with '_' followed by a capital letter}}
+  _other_// expected-warning {{identifier '_other_' is reserved because it starts with '_' at global scope}}
+};
+
+enum {
+  __some, // expected-warning {{identifier '__some' is reserved because it starts with '__'}}
+  _Other, // expected-warning {{identifier '_Other' is reserved because it starts with '_' followed by a capital letter}}
+  _other  // expected-warning {{identifier '_other' is reserved because it starts with '_' at global scope}}
+};
+
+static union {
+  int _barbeFleurie; // no-warning
+};
+
+using _Barbamama = __barbapapa; // expected-warning {{identifier '_Barbamama' is reserved because it starts with '_' followed by a capital letter}}
+
+int foobar() {
+  return foo__bar(); // no-warning
+}
+
+namespace {
+int _barbatruc; // no-warning
+}
+
+

[PATCH] D99715: [CMake] Respect LLVM_MINIMUM_PYTHON_VERSION in Tooling/CMakeLists.txt

2021-04-01 Thread Dominik Montada via Phabricator via cfe-commits
gargaroff added a comment.

In D99715#2663588 , @serge-sans-paille 
wrote:

> Did you try just removing that `find_package`?

Simply removing it also works. I just wasn't sure whether it might be required 
by something else, but if not I can go ahead and change this diff to remove it 
instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99715

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


[PATCH] D93095: Introduce -Wreserved-identifier

2021-04-01 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 334662.
serge-sans-paille added a comment.

Warn on friend functions. I failed to support friend classes, but they are only 
declared and not defined, so that should be fine, right?


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

https://reviews.llvm.org/D93095

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Sema/reserved-identifier.c
  clang/test/Sema/reserved-identifier.cpp

Index: clang/test/Sema/reserved-identifier.cpp
===
--- /dev/null
+++ clang/test/Sema/reserved-identifier.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wreserved-identifier %s
+
+int foo__bar() { return 0; }// expected-warning {{identifier 'foo__bar' is reserved because it contains '__'}}
+static int _bar() { return 0; } // expected-warning {{identifier '_bar' is reserved because it starts with '_' at global scope}}
+static int _Bar() { return 0; } // expected-warning {{identifier '_Bar' is reserved because it starts with '_' followed by a capital letter}}
+int _barbouille() { return 0; } // expected-warning {{identifier '_barbouille' is reserved because it starts with '_' at global scope}}
+
+void foo(unsigned int _Reserved) { // expected-warning {{identifier '_Reserved' is reserved because it starts with '_' followed by a capital letter}}
+  unsigned int __1 =   // expected-warning {{identifier '__1' is reserved because it starts with '__'}}
+  _Reserved;   // no-warning
+}
+
+// This one is explicitly skipped by -Wreserved-identifier
+void *_; // no-warning
+
+template  constexpr bool __toucan = true; // expected-warning {{identifier '__toucan' is reserved because it starts with '__'}}
+
+template 
+concept _Barbotine = __toucan; // expected-warning {{identifier '_Barbotine' is reserved because it starts with '_' followed by a capital letter}}
+
+template  // expected-warning {{'__' is reserved because it starts with '__'}}
+struct BarbeNoire {};
+
+template  // no-warning
+struct BarbeJaune {};
+
+template  // expected-warning {{'__' is reserved because it starts with '__'}}
+void BarbeRousse() {}
+
+namespace _Barbidur { // expected-warning {{identifier '_Barbidur' is reserved because it starts with '_' followed by a capital letter}}
+
+struct __barbidou {}; // expected-warning {{identifier '__barbidou' is reserved because it starts with '__'}}
+struct _barbidou {};  // no-warning
+
+int __barbouille; // expected-warning {{identifier '__barbouille' is reserved because it starts with '__'}}
+int _barbouille;  // no-warning
+
+int __babar() { return 0; } // expected-warning {{identifier '__babar' is reserved because it starts with '__'}}
+int _babar() { return 0; }  // no-warning
+
+} // namespace _Barbidur
+
+class __barbapapa { // expected-warning {{identifier '__barbapapa' is reserved because it starts with '__'}}
+  void _barbabelle() {} // no-warning
+  int _Barbalala;   // expected-warning {{identifier '_Barbalala' is reserved because it starts with '_' followed by a capital letter}}
+};
+
+enum class __menu { // expected-warning {{identifier '__menu' is reserved because it starts with '__'}}
+  __some,   // expected-warning {{identifier '__some' is reserved because it starts with '__'}}
+  _Other,   // expected-warning {{identifier '_Other' is reserved because it starts with '_' followed by a capital letter}}
+  _other// no-warning
+};
+
+enum _Menu { // expected-warning {{identifier '_Menu' is reserved because it starts with '_' followed by a capital letter}}
+  _OtheR_,   // expected-warning {{identifier '_OtheR_' is reserved because it starts with '_' followed by a capital letter}}
+  _other_// expected-warning {{identifier '_other_' is reserved because it starts with '_' at global scope}}
+};
+
+enum {
+  __some, // expected-warning {{identifier '__some' is reserved because it starts with '__'}}
+  _Other, // expected-warning {{identifier '_Other' is reserved because it starts with '_' followed by a capital letter}}
+  _other  // expected-warning {{identifier '_other' is reserved because it starts with '_' at global scope}}
+};
+
+static union {
+  int _barbeFleurie; // no-warning
+};
+
+using _Barbamama = __barbapapa; // expected-warning {{identifier '_Barbamama' is reserved because it starts with '_' followed by a capital letter}}
+
+int foobar() {
+  return foo__bar(); // no-warning
+}
+
+namespace {
+int _barbatruc; // no-warning
+}
+
+long dou

[clang] 7c541a1 - [OpenCL][Docs] Added a label for C++ libs section and example link

2021-04-01 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2021-04-01T13:55:23+01:00
New Revision: 7c541a195f651aa8d6aa270db83932a6ac7fac78

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

LOG: [OpenCL][Docs] Added a label for C++ libs section and example link

Added: 


Modified: 
clang/docs/OpenCLSupport.rst

Removed: 




diff  --git a/clang/docs/OpenCLSupport.rst b/clang/docs/OpenCLSupport.rst
index 8af579a082a9..7ecc4f2bed0a 100644
--- a/clang/docs/OpenCLSupport.rst
+++ b/clang/docs/OpenCLSupport.rst
@@ -398,6 +398,8 @@ Feel free to contact us on `cfe-dev
 `_ or via `Bugzilla
 `__.
 
+.. _opencl_experimenal_cxxlibs:
+
 C++ libraries for OpenCL
 
 
@@ -441,4 +443,5 @@ The possible clang invocation to compile the example is as 
follows:
  $ clang -cl-std=clc++  -I/include test.cl
 
 Note that `type_traits` is a header only library and therefore no extra
-linking step against the standard libraries is required.
+linking step against the standard libraries is required. See full example
+in `Compiler Explorer `_.



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


[PATCH] D99714: [clang][Analyzer] Handle flexible arrays better in ArrayBoundV2 checker.

2021-04-01 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

It works not reliable for all data types. If `char` is used instead of `int` 
(in the test), the allocated size may be larger than the intended size of the 
array, probably because memory alignment adjustments. In the following case it 
is possible to index "past the end" of the array for some first indices (until 
12?).

  struct S {
int n;
char x;
char s[];
  };
  struct S *s = (struct S *)malloc(sizeof(struct S) + 10);
  s.s[12] = 12;




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:117
+namespace {
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const MemRegion *MRegion) 
{
+  SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();

martong wrote:
> Do we need an overload perhaps in `DynamicSize.h` of 
> ```
> SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV)
> ``` 
> that takes a MemRegion?
If the change is accepted then yes, or have only the `MemRegion` version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99714

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


[PATCH] D99292: [flang][driver] Add support for `-cpp/-nocpp`

2021-04-01 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski updated this revision to Diff 334667.
awarzynski marked 3 inline comments as done.
awarzynski added a comment.

Update comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99292

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Frontend/PreprocessorOptions.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendAction.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/test/Driver/cpp-nocpp-command-line-macro.f90
  flang/test/Driver/cpp-nocpp-predefined-macro.F90
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/input-from-stdin.f90
  flang/test/Driver/macro-def-undef.F90
  flang/test/Driver/macro-def-undef.f90
  flang/test/Driver/macro-multiline.F90
  flang/test/Driver/macro-multiline.f90
  flang/test/Driver/predefined-macros-compiler-version.F90
  flang/test/Driver/predefined-macros-compiler-version.f90

Index: flang/test/Driver/predefined-macros-compiler-version.f90
===
--- /dev/null
+++ flang/test/Driver/predefined-macros-compiler-version.f90
@@ -1,26 +0,0 @@
-! Check that the driver correctly defines macros with the compiler version
-
-! REQUIRES: new-flang-driver
-
-!--
-! FLANG DRIVER (flang-new)
-!--
-! RUN: %flang-new -E %s  2>&1 | FileCheck %s --ignore-case
-
-!-
-! FRONTEND FLANG DRIVER (flang-new -fc1)
-!-
-! RUN: %flang-new -fc1 -E %s  2>&1 | FileCheck %s --ignore-case
-
-!-
-! EXPECTED OUTPUT
-!-
-! CHECK: flang = 1
-! CHECK: flang_major = {{[1-9][0-9]*$}}
-! CHECK: flang_minor = {{[0-9]+$}}
-! CHECK: flang_patchlevel = {{[0-9]+$}}
-
-integer, parameter :: flang = __flang__
-integer, parameter :: flang_major = __flang_major__
-integer, parameter :: flang_minor = __flang_minor__
-integer, parameter :: flang_patchlevel = __flang_patchlevel__
Index: flang/test/Driver/macro-multiline.f90
===
--- /dev/null
+++ flang/test/Driver/macro-multiline.f90
@@ -1,22 +0,0 @@
-! Ensure the end-of-line character and anything that follows after in a macro definition (-D) is ignored.
-
-! REQUIRES: new-flang-driver
-
-!--
-! FLANG DRIVER (flang-new)
-!--
-! RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs %flang-new -E %s  2>&1 | FileCheck --strict-whitespace --match-full-lines %s
-
-!-
-! FRONTEND FLANG DRIVER (flang-new -fc1)
-!-
-! RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs %flang-new -fc1 -E %s  2>&1 | FileCheck --strict-whitespace --match-full-lines %s
-
-!---
-! EXPECTED OUTPUT FOR MACRO 'X'
-!---
-! CHECK:start a end
-! CHECK-NOT:THIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT
-! CHECK-NOT:this_should_not_exist_in_the_output
-
-START X END
\ No newline at end of file
Index: flang/test/Driver/macro-def-undef.F90
===
--- flang/test/Driver/macro-def-undef.F90
+++ flang/test/Driver/macro-def-undef.F90
@@ -1,20 +1,18 @@
 ! Ensure arguments -D and -U work as expected.
 
-! REQUIRES: new-flang-driver
-
 !--
 ! FLANG DRIVER (flang-new)
 !--
-! RUN: %flang-new -E %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
-! RUN: %flang-new -E -DX=A %s  2>&1 | FileCheck %s --check-prefix=DEFINED
-! RUN: %flang-new -E -DX=A -UX %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
+! RUN: %flang -E %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
+! RUN: %flang -E -DX=A %s  2>&1 | FileCheck %s --check-prefix=DEFINED
+! RUN: %flang -E -DX=A -UX %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
 
 !-
 ! FRONTEND FLANG DRIVER (flang-new -fc1)
 !-
-! RUN: %flang-new -fc1 -E %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
-! RUN: %flang-new -fc1 -E -DX=A %s  2>&1 | FileCheck %s --check-prefix=DEFINED
-! RUN: %flang-new -fc1 -E -DX -UX %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
+! RUN: %flang_fc1 -E %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
+! RUN: %flang_fc1 -E -DX=A %s  2>&1 | FileCheck %s --check-prefix=DEFINED
+! RUN: %flang_fc1 -E -DX -UX %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
 
 !
 ! EXPECTED OUTPUT FOR AN UNDEFINED MACRO
@@ -35,4 +33,4 @@
 #else
 program B
 #endif

[PATCH] D99292: [flang][driver] Add support for `-cpp/-nocpp`

2021-04-01 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added inline comments.



Comment at: flang/include/flang/Frontend/CompilerInvocation.h:142
+  ///
+  /// \param [in] ppOpts The preprocessor options
+  void collectMacroDefinitions();

kiranchandramohan wrote:
> Nit: Misplaced? Don't see any params here.
Yes, needs updating.



Comment at: flang/include/flang/Frontend/CompilerInvocation.h:153
+  /// Updates this instance based on the extension of the input file (e.g. f90
+  /// s F90)
+  void updateBasedOnExtension(const FrontendInputFile &inputFile);

kiranchandramohan wrote:
> Nit: Should this have a param in?
Should be deleted, sorry!



Comment at: flang/lib/Frontend/FrontendActions.cpp:68
 
 bool PrescanAndSemaAction::BeginSourceFileAction(CompilerInstance &c1) {
   CompilerInstance &ci = this->instance();

kiranchandramohan wrote:
> Nit: Looks similar to function above.
Yes, they are identical up to L85 (in which Parsing starts):
```
  // Parse. In case of failure, report and return.
  ci.parsing().Parse(llvm::outs());
```

I agree that this is not great and should be refactored at some point. But it 
will be much easier once we have more actions and a better understanding of 
what is needed. I think that https://reviews.llvm.org/D99645 is an important 
step in this direction. It adds a new abstract class for frontend actions. Once 
it's merged, we will have 3 classes of actions:
* prescan
* prescan + parse
* prescan + parse + sema
So there will be a very clear split into 3 groups.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99292

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


[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 334668.

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

https://reviews.llvm.org/D75844

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Parse/ParseDecl.cpp
  clang/test/AST/sourceranges.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -185,9 +185,12 @@
   return 1;
   [[clang::fallthrough]];  // expected-warning{{fallthrough annotation in unreachable code}}
 case 222:
+  return 2;
+  __attribute__((fallthrough)); // expected-warning{{fallthrough annotation in unreachable code}}
+case 223:
   n += 400;
-case 223:  // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
-  ;
+case 224: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+;
   }
 
   long p = static_cast(n) * n;
Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -108,6 +108,24 @@
   }
 }
 
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_case
+namespace attributed_case {
+void f(int n) {
+  switch (n) {
+  case 0:
+n--;
+// CHECK: AttributedStmt {{.*}} 
+// CHECK: FallThroughAttr {{.*}} 
+__attribute__((fallthrough))
+// CHECK: FallThroughAttr {{.*}} 
+  __attribute__((fallthrough));
+  case 1:
+n++;
+break;
+  }
+}
+} // namespace attributed_case
+
 // CHECK: NamespaceDecl {{.*}} attributed_stmt
 namespace attributed_stmt {
   // In DO_PRAGMA and _Pragma cases, `LoopHintAttr` comes from 
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -162,15 +162,19 @@
 ///',' or ')' are ignored, otherwise they produce a parse error.
 ///
 /// We follow the C++ model, but don't allow junk after the identifier.
-void Parser::ParseGNUAttributes(ParsedAttributes &attrs,
-SourceLocation *endLoc,
-LateParsedAttrList *LateAttrs,
-Declarator *D) {
+void Parser::ParseGNUAttributes(ParsedAttributesWithRange &Attrs,
+SourceLocation *EndLoc,
+LateParsedAttrList *LateAttrs, Declarator *D) {
   assert(Tok.is(tok::kw___attribute) && "Not a GNU attribute list!");
 
+  SourceLocation StartLoc = Tok.getLocation(), Loc;
+
+  if (!EndLoc)
+EndLoc = &Loc;
+
   while (Tok.is(tok::kw___attribute)) {
 SourceLocation AttrTokLoc = ConsumeToken();
-unsigned OldNumAttrs = attrs.size();
+unsigned OldNumAttrs = Attrs.size();
 unsigned OldNumLateAttrs = LateAttrs ? LateAttrs->size() : 0;
 
 if (ExpectAndConsume(tok::l_paren, diag::err_expected_lparen_after,
@@ -198,14 +202,14 @@
   SourceLocation AttrNameLoc = ConsumeToken();
 
   if (Tok.isNot(tok::l_paren)) {
-attrs.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0,
+Attrs.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0,
  ParsedAttr::AS_GNU);
 continue;
   }
 
   // Handle "parameterized" attributes
   if (!LateAttrs || !isAttributeLateParsed(*AttrName)) {
-ParseGNUAttributeArgs(AttrName, AttrNameLoc, attrs, endLoc, nullptr,
+ParseGNUAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc, nullptr,
   SourceLocation(), ParsedAttr::AS_GNU, D);
 continue;
   }
@@ -238,8 +242,8 @@
 SourceLocation Loc = Tok.getLocation();
 if (ExpectAndConsume(tok::r_paren))
   SkipUntil(tok::r_paren, StopAtSemi);
-if (endLoc)
-  *endLoc = Loc;
+if (EndLoc)
+  *EndLoc = Loc;
 
 // If this was declared in a macro, attach the macro IdentifierInfo to the
 // parsed attribute.
@@ -251,8 +255,8 @@
   Lexer::getSourceText(ExpansionRange, SM, PP.getLangOpts());
   IdentifierInfo *MacroII = PP.getIdentifierInfo(FoundName);
 
-  for (unsigned i = OldNumAttrs; i < attrs.size(); ++i)
-attrs[i].setMacroIdentifier(MacroII, ExpansionRange.getBegin());
+  for (unsigned i = OldNumAttrs; i < Attrs.size(); ++i)
+Attrs[i].setMacroIdentifier(MacroII, ExpansionRange.getBegin());
 
   if (LateAttrs) {
 for (unsigned i = OldNumLateAttrs; i < LateAttrs->size(); ++i)
@@ -260,6 +264,8 @@
   }
 }

[PATCH] D99714: [clang][Analyzer] Handle flexible arrays better in ArrayBoundV2 checker.

2021-04-01 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

In D99714#2663677 , @balazske wrote:

> It works not reliable for all data types. If `char` is used instead of `int` 
> (in the test), the allocated size may be larger than the intended size of the 
> array, probably because memory alignment adjustments. In the following case 
> it is possible to index "past the end" of the array for some first indices 
> (until 12?).
>
>   struct S {
> int n;
> char x;
> char s[];
>   };



> struct S *s = (struct S *)malloc(sizeof(struct S) + 10);
> s.s[12] = 12;
>
>   

Then I suppose we have to consider the alignment info as well. Perhaps you 
could reuse some parts of the PlacementNewChecker's alignment checking 
implementation? (see 
https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp#L176
 ) I'd do that only in second follow-up patch, because that is going to 
complicate things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99714

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


[PATCH] D99714: [clang][Analyzer] Handle flexible arrays better in ArrayBoundV2 checker.

2021-04-01 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:117
+namespace {
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const MemRegion *MRegion) 
{
+  SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();

balazske wrote:
> martong wrote:
> > Do we need an overload perhaps in `DynamicSize.h` of 
> > ```
> > SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV)
> > ``` 
> > that takes a MemRegion?
> If the change is accepted then yes, or have only the `MemRegion` version.
I think this is the other way around, let's create the overload then we accept.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99714

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


[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

2021-04-01 Thread Marco Gartmann via Phabricator via cfe-commits
mgartmann added a comment.

In D99646#2661651 , @njames93 wrote:

> Is it not wise to also check the c standard library.
> So check for function refs to these names in the global or std namespace.
> `printf`, `vprintf`, `puts`, `putchar`, `scanf`, `scanf`, `getchar` and `gets`
> It may be a bit of a pain checking for usages of `stdin` and  `stdout` due to 
> them being defined as macros.

Hi @njames93,
I can see your point, I am going to add this functionality.

However, I do not completely understand what you mean with //check for function 
refs to these names in the global or std namespace//.  
Could you explain this a bit further?

E.g., should all calls to these functions be flagged if they happen inside the 
`std` namespace or in the `global` namespace?
And if they happen inside `my_namespace` e.g., they should not be flagged. Am I 
understanding this correctly?
How should the check behave if the functions are called inside `main()`?

Would it make more sense to put this functionality into a separat check?

Thanks for your effort in advance.
Looking forward to hearing from you soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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


[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D75844

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


[PATCH] D99190: WIP: [SYCL] Add design document for SYCL mode

2021-04-01 Thread Alexey Bader via Phabricator via cfe-commits
bader added inline comments.



Comment at: clang/docs/SYCLSupport.md:123
+traverse all symbols accessible from kernel functions and add them to the
+"device part" of the code marking them with the new SYCL device attribute.
+

Naghasan wrote:
> ABataev wrote:
> > bader wrote:
> > > Naghasan wrote:
> > > > OpenMP offload uses a similar approach isn't it? Might be worth to 
> > > > describe how the 2 relates to each other and where they diverge. 
> > > Do you mean the approach OpenMP compiler uses to outline single-source 
> > > code parts to offload?
> > > To be honest, I'm not sure... @ABataev, is there any description how 
> > > OpenMP compiler outlines device code?
> > > https://clang.llvm.org/docs/OpenMPSupport.html doesn't provide much 
> > > details unfortunately.
> > I don't think we have anything like this. Moreover, currently, there are 2 
> > different models, one with outlining by the frontend and another one with 
> > the outlining by the LLVM.
> I mentioned that as I know there is some support for CUDA and the clang 
> driver openmp offload works with multiple frontend passes.
> If the model(s) is too different then there is no point going further here. 
> 
> > Moreover, currently, there are 2 different models, one with outlining by 
> > the frontend and another one with the outlining by the LLVM.
> 
> I do recall some brief conversations about that. Are they meant to work in 
> pair or one aims to replace the other ?
What do say if add a TODO here (or in a separate TODO document) to study more 
about differences between SYCL/OpenMP-offload/CUDA implementation designs?
It seems to be useful to understanding if we want to re-use other programming 
model experience with implementing common tasks like device code outlining.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99190

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


[PATCH] D99646: [clang-tidy] misc-std-stream-objects-outside-main: a new check

2021-04-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D99646#2663733 , @mgartmann wrote:

> Hi @njames93,
> I can see your point, I am going to add this functionality.
>
> However, I do not completely understand what you mean with //check for 
> function refs to these names in the global or std namespace//.  
> Could you explain this a bit further?
>
> E.g., should all calls to these functions be flagged if they happen inside 
> the `std` namespace or in the `global` namespace?
> And if they happen inside `my_namespace` e.g., they should not be flagged. Am 
> I understanding this correctly?
> How should the check behave if the functions are called inside `main()`?

I mean any `DeclRefExpr` that reference those functions.
We don't actually want to only match on call expressions to those functions, we 
also want the other ways they can be called.

  auto Print = &puts;
  Print("This is using stdio");

I could imagine this is the kind of matcher expression you need.

  declRefExpr(
   hasDeclaration(functionDecl(
   anyOf(hasDeclContext(translationUnitDecl()), isInStdNamespace()),
   hasAnyName("printf", "vprintf", ...))),
   unless(forFunction(isMain(

As for flagging the same rules should apply for references to `cin`, `cout` etc.

> Would it make more sense to put this functionality into a separat check?
>
> Thanks for your effort in advance.
> Looking forward to hearing from you soon.

You could include it in this check, then maybe rename this check to a more 
general name like `misc-avoid-stdio-outside-main`.




Comment at: 
clang-tools-extra/clang-tidy/misc/StdStreamObjectsOutsideMainCheck.cpp:22
+  Finder->addMatcher(
+  declRefExpr(to(namedDecl(hasAnyName("cin", "wcin", "cout", "wcout",
+  "cerr", "wcerr"),

should probably use varDecl here instead of namedDecl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

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


[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

2021-04-01 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added a comment.

In D88978#2660274 , @arsenm wrote:

> Is this still needed?

Yes, I just got a little bogged down in the OMP code and haven't gotten back to 
it to finish it up. I anticipate needing to do this to soon, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88978

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


[PATCH] D99488: [SYCL][Doc] Add address space handling section to SYCL documentation

2021-04-01 Thread Alexey Bader via Phabricator via cfe-commits
bader updated this revision to Diff 334689.
bader marked 3 inline comments as done.
bader added a comment.

Applied code review suggestions from @Naghasan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99488

Files:
  clang/docs/SYCLSupport.rst

Index: clang/docs/SYCLSupport.rst
===
--- clang/docs/SYCLSupport.rst
+++ clang/docs/SYCLSupport.rst
@@ -219,3 +219,136 @@
 
 Additional details of kernel parameter passing may be found in the document
 `SYCL Kernel Parameter Handling and Array Support `_.
+
+Address space handling
+^^
+
+The SYCL specification represents pointers to disjoint memory regions using C++
+wrapper classes on an accelerator to enable compilation with a standard C++
+toolchain and a SYCL compiler toolchain. Section 3.8.2 of SYCL 2020
+specification defines
+`memory model `_\ ,
+section 4.7.7 - `address space classes `_
+and section 5.9 covers `address space deduction `_.
+The SYCL specification allows two modes of address space deduction: "generic as
+default address space" (see section 5.9.3) and "inferred address space" (see
+section 5.9.4). Current implementation supports only "generic as default address
+space" mode.
+
+SYCL borrows its memory model from OpenCL however SYCL doesn't perform
+the address space qualifier inference as detailed in
+`OpenCL C v3.0 6.7.8 `_.
+
+Similar to other single-source C++-based GPU programming modes like
+OpenMP/CUDA/HIP, SYCL uses clang's "default" address space for types with no
+explicit address space attribute. This design has two important features: it
+keeps the type system consistent with C++ and enable tools for emitting device
+code aligned with SPIR memory model (and other GPU targets).
+
+So inside a function, this variable declaration:
+
+.. code-block:: C++
+
+   int var;
+
+The SYCL device compiler turns into:
+
+.. code-block:: C++
+
+   VarDecl  var 'int'
+
+while the OpenCL compiler turns it into:
+
+.. code-block:: C++
+
+   VarDecl  var '__private int'
+
+Changing the type of a variable can have observable effects in C++. For example,
+this does not compile in C++ for OpenCL mode:
+
+.. code-block:: C++
+
+   template
+   struct is_same {
+ static constexpr int value = 0;
+   };
+
+   template
+   struct is_same {
+ static constexpr int value = 1;
+   };
+
+   void foo(int p) {
+ static_assert(is_same::value, "int is not an int?"); // Fails: p is '__private int' != 'int'
+ static_assert(is_same::value, "int* is not an int*?");  // Fails: p is '__private int*' != '__generic int*'
+   }
+
+``multi_ptr`` class implementation example:
+
+.. code-block:: C++
+
+   // check that SYCL mode is ON and we can use non-standard decorations
+   #if defined(__SYCL_DEVICE_ONLY__)
+   // GPU/accelerator implementation
+   template  class multi_ptr {
+ // DecoratedType applies corresponding address space attribute to the type T
+ // DecoratedType::type == "__attribute__((opencl_global)) T"
+ // See sycl/include/CL/sycl/access/access.hpp for more details
+ using pointer_t = typename DecoratedType::type *;
+
+ pointer_t m_Pointer;
+ public:
+ pointer_t get() { return m_Pointer; }
+ T& operator* () { return *reinterpret_cast(m_Pointer); }
+   }
+   #else
+   // CPU/host implementation
+   template  class multi_ptr {
+ T *m_Pointer; // regular undecorated pointer
+ public:
+ T *get() { return m_Pointer; }
+ T& operator* () { return *m_Pointer; }
+   }
+   #endif
+
+Depending on the compiler mode, ``multi_ptr`` will either decorate its internal
+data with the address space attribute or not.
+
+To utilize clang's existing functionality, we reuse the following OpenCL address
+space attributes for pointers:
+
+.. list-table::
+   :header-rows: 1
+
+   * - Address space attribute
+ - SYCL address_space enumeration
+   * - ``__attribute__((opencl_global))``
+ - global_space, constant_space
+   * - ``__attribute__((opencl_local))``
+ - local_space
+   * - ``__attribute__((opencl_private))``
+ - private_space
+
+
+.. code-block::
+
+   TODO: add support for `__attribute__((opencl_global_host))` and
+   `__attribute__((opencl_global_device))`.
+
+
+The default address space is "generic-memory", which is a virtual address space
+that overlaps the global, local, and private address spaces. SYCL mode enables
+conversion to/from the default address space 

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

2021-04-01 Thread Benson Chu via Phabricator via cfe-commits
pestctrl created this revision.
pestctrl added reviewers: akyrtzi, rsmith.
pestctrl added a project: clang.
pestctrl requested review of this revision.
Herald added a subscriber: cfe-commits.

I noticed this bug because attributes were being dropped from tentative
definitions after the second tentative definition. If you have the following C
code:

char chararr[13];
char chararr[13] __attribute__ ((section("data")));
char chararr[13] __attribute__ ((aligned(16)));

If you emit the LLVM IR, the chararr will have the section attribute, but not
the aligned attribute. If you have more tentative definitions following the
last line, those attributes will be dropped as well.

This is not a problem with merging of attributes, as that works completely
fine. Clang will store a merged list of attributes in the most recent tentative
definition encountered. The problem is that clang will choose one of the
tentative definitions to emit into LLVM IR, and it's NOT the most recent
tentative definition.

VarDecl::getActingDefinition is called to choose the which tentative definition
should be emitted. VarDecl::getActingDefinition loops through VarDecl::redecls,
which will return an iterator of all redeclarations of a variable, which will
include all tentative definitions. VarDecl::getActingDefinition assumes that
the order of this iterator is the order in which the defs/decls appear in the
file, but this is not the case. The first element of the iterator is in fact
the first def/decl, but the rest of the elements are reversed. So, if we had 4
def/decls, the order they appear in the iterator would be [1, 4, 3, 2].

So, when VarDecl::getActingDefinition loops through VarDecl::redecls, picking
the last element in the iterator, it's actually picking the second
decl/def. Because merged attributes appear in the most recent def/decl, any
attributes that appear after the second decl are dropped.

This changeset modifies VarDecl::getActingDefinition to use some helper
functions - namely getMostRecentDecl and getPreviousDecl - to instead loop
through the declaration chain in reverse order of how they appear in the file,
returning the first instance that is a tentative definition. This means
attributes will no longer be dropped.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99732

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/attr-tentative-definition.c


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section 
"datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2189,19 +2189,20 @@
 
 VarDecl *VarDecl::getActingDefinition() {
   DefinitionKind Kind = isThisDeclarationADefinition();
-  if (Kind != TentativeDefinition)
+  if (Kind != TentativeDefinition || hasDefinition())
 return nullptr;
 
-  VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
-if (Kind == Definition)
-  return nullptr;
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
+
+// Return the first TentativeDefinition that is encountered.
 if (Kind == TentativeDefinition)
-  LastTentative = I;
+  return Decl;
   }
-  return LastTentative;
+
+  return nullptr;
 }
 
 VarDecl *VarDecl::getDefinition(ASTContext &C) {


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2189,19 +2189,20 @@
 
 VarDecl *VarDecl::getActingDefinition() {
   DefinitionKind Kind = isThisDeclarationADefinition();
-  if (Kind != TentativeDefinition)
+  if (Kind != TentativeDefinition || hasDefinition())
 return nullptr;
 
-  VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
-if (Kind == Definition)
-  return nullptr;
+  

[PATCH] D93095: Introduce -Wreserved-identifier

2021-04-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:383
+  "identifier %0 is reserved because %select{"
+  "|" // passing 0 for %1 is not valid but we need that | to make the enum 
order match the diagnostic
+  "it starts with '_' at global scope|"

rsmith wrote:
> This is hopefully more useful to future readers and also terser. (I like 
> including  in the diagnostic in the bad case because if it ever 
> happens we're more likely to get a bug report that way.)
Ooh, I like that suggestion, thanks!



Comment at: clang/lib/AST/Decl.cpp:1096
+
+// Walk up the lexical parents to determine if we're at TU level or not.
+if (isa(this) || isTemplateParameter())

This comment should be updated -- we no longer walk the lexical parents.



Comment at: clang/lib/AST/Decl.cpp:1084
+  const IdentifierInfo *II = nullptr;
+  if (const auto *FD = dyn_cast(this))
+II = FD->getLiteralIdentifier();

rsmith wrote:
> Please reorder the literal operator case after the plain `getIdentifier` 
> case. I think this'd be more readable if the common and expected case is 
> first, and it doesn't make any functional difference since a literal operator 
> doesn't have a "regular" identifier name.
It looks like this code still needs to move.



Comment at: clang/lib/Basic/IdentifierTable.cpp:297
+
+return ReservedIdentifierStatus::StartsWithUnderscoreAtGlobalScope;
+  }

I think comments here may help -- we don't know that the identifier is at 
global scope within this call, so the return value looks a bit strange. This is 
a case where the enumerator name is a bit awkward in this context but makes a 
lot of sense in the context of the check.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4051-4052
   if (const auto *Id = CalleeDecl->getIdentifier())
-if (Id->isReservedName())
+if (Id->isReserved(CGM.getLangOpts()) !=
+ReservedIdentifierStatus::NotReserved)
   return;

There's a slight functionality change here in that the code used to allow 
identifiers that start with a single underscore followed by a lowercase letter 
and that now early returns. Is that expected (and tested)?



Comment at: clang/lib/Sema/SemaDecl.cpp:5564-5565
+  ReservedIdentifierStatus Status = D->isReserved(getLangOpts());
+  if (Status != ReservedIdentifierStatus::NotReserved)
+if (Context.getSourceManager().isInMainFile(D->getLocation()))
+  Diag(D->getLocation(), diag::warn_reserved_extern_symbol) << D << Status;





Comment at: clang/lib/Sema/SemaDeclCXX.cpp:10998
   PushDeclContext(NamespcScope, Namespc);
+
   return Namespc;

Spurious whitespace change.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:12673
   ActOnDocumentableDecl(NewND);
+
   return NewND;

Spurious whitespace change.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16743
 
+  warnOnReservedIdentifier(ND);
+

Doesn't `PushOnScopeChains()` do this already?



Comment at: clang/lib/Sema/SemaStmt.cpp:545
+  if (!Context.getSourceManager().isInSystemHeader(IdentLoc)) {
+ReservedIdentifierStatus Status = TheDecl->isReserved(getLangOpts());
+if (Status != ReservedIdentifierStatus::NotReserved)

aaron.ballman wrote:
> This check should be reversed as well.
This still needs to be reversed so we check for system headers after checking 
the reserved status.



Comment at: clang/lib/Sema/SemaTemplate.cpp:1680-1681
 
+  for (NamedDecl *P : Params)
+warnOnReservedIdentifier(P);
+

rsmith wrote:
> Again, it'd be more consistent to do this when we finish creating the 
> declaration and push it into scope, for all kinds of declaration.
Is this handled by `PushOnScopeChains()`?


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

https://reviews.llvm.org/D93095

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


[PATCH] D99190: [SYCL] Add design document for SYCL mode

2021-04-01 Thread Victor Lomuller via Phabricator via cfe-commits
Naghasan added inline comments.



Comment at: clang/docs/SYCLSupport.md:123
+traverse all symbols accessible from kernel functions and add them to the
+"device part" of the code marking them with the new SYCL device attribute.
+

bader wrote:
> Naghasan wrote:
> > ABataev wrote:
> > > bader wrote:
> > > > Naghasan wrote:
> > > > > OpenMP offload uses a similar approach isn't it? Might be worth to 
> > > > > describe how the 2 relates to each other and where they diverge. 
> > > > Do you mean the approach OpenMP compiler uses to outline single-source 
> > > > code parts to offload?
> > > > To be honest, I'm not sure... @ABataev, is there any description how 
> > > > OpenMP compiler outlines device code?
> > > > https://clang.llvm.org/docs/OpenMPSupport.html doesn't provide much 
> > > > details unfortunately.
> > > I don't think we have anything like this. Moreover, currently, there are 
> > > 2 different models, one with outlining by the frontend and another one 
> > > with the outlining by the LLVM.
> > I mentioned that as I know there is some support for CUDA and the clang 
> > driver openmp offload works with multiple frontend passes.
> > If the model(s) is too different then there is no point going further here. 
> > 
> > > Moreover, currently, there are 2 different models, one with outlining by 
> > > the frontend and another one with the outlining by the LLVM.
> > 
> > I do recall some brief conversations about that. Are they meant to work in 
> > pair or one aims to replace the other ?
> What do say if add a TODO here (or in a separate TODO document) to study more 
> about differences between SYCL/OpenMP-offload/CUDA implementation designs?
> It seems to be useful to understanding if we want to re-use other programming 
> model experience with implementing common tasks like device code outlining.
fine by me :)



Comment at: clang/docs/SYCLSupport.rst:198
+   class MyObj {
+ accessor _ptr; // accessor contains a pointer to the global 
address space.
+   public:

Oups sorry, I just noticed  I made a mistake in my suggestion, that's more in 
line with the statement `KernelFuncObj.A.__init(a);` in the pseudo  opencl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99190

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


[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

2021-04-01 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

A user interrupt is different than a regular interrupt right? It doesn't make 
sense that we would change the behavior of the interrupt calling convention 
just because the the user interrupt instructions are enabled. That would occur 
just from passing a -march for a newer CPU wouldn't it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99708

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


[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

2021-04-01 Thread LuoYuanke via Phabricator via cfe-commits
LuoYuanke added a comment.

In D99708#2663989 , @craig.topper 
wrote:

> A user interrupt is different than a regular interrupt right? It doesn't make 
> sense that we would change the behavior of the interrupt calling convention 
> just because the the user interrupt instructions are enabled. That would 
> occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "__attribute__ ((user_interrupt))" for 
functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99708

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


[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-01 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 marked an inline comment as not done.
Ericson2314 added inline comments.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
 get_compiler_rt_target(${arch} target)
-set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+set(${install_dir} 
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
   else()

phosek wrote:
> Ericson2314 wrote:
> > ldionne wrote:
> > > lebedev.ri wrote:
> > > > This looks suspect
> > > Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like 
> > > elsewhere.
> > See the non-line comment I responded to @lebidev.ri with. In sort if
> > 
> > ```
> > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> > ```
> > 
> > is a relative path, then we end up with
> > 
> > ```
> > ${CMAKE_INSTALL_PREFIX}/${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> > ```
> > 
> > instead of 
> > 
> > ```
> > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${target}
> > ```
> > 
> > as we do with the other per-package prefixes. Also if 
> > `CMAKE_INSTALL_LIBDIR` is already an absolute path, then
> > ```
> > ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target}
> > ```
> > is the same thing, and closer to the second than the first.
> I'm not sure if that's desirable. I'm not sure if we still need 
> `COMPILER_RT_INSTALL_PATH`. That variable is only used by 
> `clang/runtime/CMakeLists.txt` which predates `runtimes/CMakeLists.txt` and 
> was AFAIK only ever used by Apple. I think we should consider removing 
> `COMPILER_RT_INSTALL_PATH`. We'll need to check if 
> `clang/runtime/CMakeLists.txt` is still being used or not.
With D99697 now all approved (thanks again!) it looks like this is the next 
step? Would be very nice if this could be removed!



Comment at: polly/cmake/CMakeLists.txt:82-96
+set(POLLY_INSTALL_PREFIX "")
 set(POLLY_CONFIG_LLVM_CMAKE_DIR 
"${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_CMAKE_DIR 
"${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_LIBRARY_DIRS 
"${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+set(POLLY_CONFIG_CMAKE_DIR 
"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
+set(POLLY_CONFIG_LIBRARY_DIRS 
"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}${LLVM_LIBDIR_SUFFIX}")
 if (POLLY_BUNDLED_ISL)
   set(POLLY_CONFIG_INCLUDE_DIRS
+"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}"

`POLLY_INSTALL_PREFIX`, like `COMPILER_RT_INSTALL_PATH`, I changed to be empty 
by default. If the `COMPILER_RT_INSTALL_PATH` can be removed, maybe 
`POLLY_INSTALL_PREFIX` can also be removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[clang] 1ea9fa8 - [clang][parser] Set source ranges for GNU-style attributes

2021-04-01 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2021-04-01T17:25:23+02:00
New Revision: 1ea9fa8c507ec360cf43faf46d13b149e37c950d

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

LOG: [clang][parser] Set source ranges for GNU-style attributes

Set the source ranges for parsed GNU-style attributes in
ParseGNUAttributes(), the same way that ParseCXX11Attributes() does it.

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

Added: 


Modified: 
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Parse/ParseDecl.cpp
clang/test/AST/sourceranges.cpp
clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Removed: 




diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index b67e541836d59..6f6d4697e6d09 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1572,27 +1572,6 @@ class Parser : public CodeCompletionHandler {
 
   
//======//
   // C99 6.9: External Definitions.
-  struct ParsedAttributesWithRange : ParsedAttributes {
-ParsedAttributesWithRange(AttributeFactory &factory)
-  : ParsedAttributes(factory) {}
-
-void clear() {
-  ParsedAttributes::clear();
-  Range = SourceRange();
-}
-
-SourceRange Range;
-  };
-  struct ParsedAttributesViewWithRange : ParsedAttributesView {
-ParsedAttributesViewWithRange() : ParsedAttributesView() {}
-void clearListOnly() {
-  ParsedAttributesView::clearListOnly();
-  Range = SourceRange();
-}
-
-SourceRange Range;
-  };
-
   DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributesWithRange &attrs,
   ParsingDeclSpec *DS = nullptr);
   bool isDeclarationAfterDeclarator();
@@ -2725,17 +2704,50 @@ class Parser : public CodeCompletionHandler {
   D.takeAttributes(attrs, endLoc);
 }
   }
-  bool MaybeParseGNUAttributes(ParsedAttributes &attrs,
-   SourceLocation *endLoc = nullptr,
+
+  /// Parses GNU-style attributes and returns them without source range
+  /// information.
+  ///
+  /// This API is discouraged. Use the version that takes a
+  /// ParsedAttributesWithRange instead.
+  bool MaybeParseGNUAttributes(ParsedAttributes &Attrs,
+   SourceLocation *EndLoc = nullptr,
LateParsedAttrList *LateAttrs = nullptr) {
 if (Tok.is(tok::kw___attribute)) {
-  ParseGNUAttributes(attrs, endLoc, LateAttrs);
+  ParsedAttributesWithRange AttrsWithRange(AttrFactory);
+  ParseGNUAttributes(Attrs, EndLoc, LateAttrs);
+  Attrs.takeAllFrom(AttrsWithRange);
   return true;
 }
 return false;
   }
-  void ParseGNUAttributes(ParsedAttributes &attrs,
-  SourceLocation *endLoc = nullptr,
+
+  bool MaybeParseGNUAttributes(ParsedAttributesWithRange &Attrs,
+   SourceLocation *EndLoc = nullptr,
+   LateParsedAttrList *LateAttrs = nullptr) {
+if (Tok.is(tok::kw___attribute)) {
+  ParseGNUAttributes(Attrs, EndLoc, LateAttrs);
+  return true;
+}
+return false;
+  }
+
+  /// Parses GNU-style attributes and returns them without source range
+  /// information.
+  ///
+  /// This API is discouraged. Use the version that takes a
+  /// ParsedAttributesWithRange instead.
+  void ParseGNUAttributes(ParsedAttributes &Attrs,
+  SourceLocation *EndLoc = nullptr,
+  LateParsedAttrList *LateAttrs = nullptr,
+  Declarator *D = nullptr) {
+ParsedAttributesWithRange AttrsWithRange(AttrFactory);
+ParseGNUAttributes(AttrsWithRange, EndLoc, LateAttrs, D);
+Attrs.takeAllFrom(AttrsWithRange);
+  }
+
+  void ParseGNUAttributes(ParsedAttributesWithRange &Attrs,
+  SourceLocation *EndLoc = nullptr,
   LateParsedAttrList *LateAttrs = nullptr,
   Declarator *D = nullptr);
   void ParseGNUAttributeArgs(IdentifierInfo *AttrName,

diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index a3d82fcd84f7a..fb086e3c1e50f 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -1034,6 +1034,27 @@ class ParsedAttributes : public ParsedAttributesView {
   mutable AttributePool pool;
 };
 
+struct ParsedAttributesWithRange : ParsedAttributes {
+  ParsedAttributesWithRange(AttributeFactory &factory)
+  : ParsedAttributes(factory) {}
+
+  void clear() {
+ParsedAttributes::clear();
+Range = SourceRange();
+  }
+
+  SourceRange Range;
+};
+struct ParsedAttributesViewWithRange : ParsedAtt

[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Timm Bäder 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 rG1ea9fa8c507e: [clang][parser] Set source ranges for 
GNU-style attributes (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75844

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Parse/ParseDecl.cpp
  clang/test/AST/sourceranges.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -185,9 +185,12 @@
   return 1;
   [[clang::fallthrough]];  // expected-warning{{fallthrough annotation in unreachable code}}
 case 222:
+  return 2;
+  __attribute__((fallthrough)); // expected-warning{{fallthrough annotation in unreachable code}}
+case 223:
   n += 400;
-case 223:  // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
-  ;
+case 224: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+;
   }
 
   long p = static_cast(n) * n;
Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -108,6 +108,24 @@
   }
 }
 
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_case
+namespace attributed_case {
+void f(int n) {
+  switch (n) {
+  case 0:
+n--;
+// CHECK: AttributedStmt {{.*}} 
+// CHECK: FallThroughAttr {{.*}} 
+__attribute__((fallthrough))
+// CHECK: FallThroughAttr {{.*}} 
+  __attribute__((fallthrough));
+  case 1:
+n++;
+break;
+  }
+}
+} // namespace attributed_case
+
 // CHECK: NamespaceDecl {{.*}} attributed_stmt
 namespace attributed_stmt {
   // In DO_PRAGMA and _Pragma cases, `LoopHintAttr` comes from 
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -162,15 +162,19 @@
 ///',' or ')' are ignored, otherwise they produce a parse error.
 ///
 /// We follow the C++ model, but don't allow junk after the identifier.
-void Parser::ParseGNUAttributes(ParsedAttributes &attrs,
-SourceLocation *endLoc,
-LateParsedAttrList *LateAttrs,
-Declarator *D) {
+void Parser::ParseGNUAttributes(ParsedAttributesWithRange &Attrs,
+SourceLocation *EndLoc,
+LateParsedAttrList *LateAttrs, Declarator *D) {
   assert(Tok.is(tok::kw___attribute) && "Not a GNU attribute list!");
 
+  SourceLocation StartLoc = Tok.getLocation(), Loc;
+
+  if (!EndLoc)
+EndLoc = &Loc;
+
   while (Tok.is(tok::kw___attribute)) {
 SourceLocation AttrTokLoc = ConsumeToken();
-unsigned OldNumAttrs = attrs.size();
+unsigned OldNumAttrs = Attrs.size();
 unsigned OldNumLateAttrs = LateAttrs ? LateAttrs->size() : 0;
 
 if (ExpectAndConsume(tok::l_paren, diag::err_expected_lparen_after,
@@ -198,14 +202,14 @@
   SourceLocation AttrNameLoc = ConsumeToken();
 
   if (Tok.isNot(tok::l_paren)) {
-attrs.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0,
+Attrs.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0,
  ParsedAttr::AS_GNU);
 continue;
   }
 
   // Handle "parameterized" attributes
   if (!LateAttrs || !isAttributeLateParsed(*AttrName)) {
-ParseGNUAttributeArgs(AttrName, AttrNameLoc, attrs, endLoc, nullptr,
+ParseGNUAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc, nullptr,
   SourceLocation(), ParsedAttr::AS_GNU, D);
 continue;
   }
@@ -238,8 +242,8 @@
 SourceLocation Loc = Tok.getLocation();
 if (ExpectAndConsume(tok::r_paren))
   SkipUntil(tok::r_paren, StopAtSemi);
-if (endLoc)
-  *endLoc = Loc;
+if (EndLoc)
+  *EndLoc = Loc;
 
 // If this was declared in a macro, attach the macro IdentifierInfo to the
 // parsed attribute.
@@ -251,8 +255,8 @@
   Lexer::getSourceText(ExpansionRange, SM, PP.getLangOpts());
   IdentifierInfo *MacroII = PP.getIdentifierInfo(FoundName);
 
-  for (unsigned i = OldNumAttrs; i < attrs.size(); ++i)
-attrs[i].setMacroIdentifier(MacroII, ExpansionRange.getBegin());
+  for (unsigned i = OldNu

[clang] 908a267 - Revert "[clang][parser] Set source ranges for GNU-style attributes"

2021-04-01 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2021-04-01T17:32:40+02:00
New Revision: 908a267b5a3b8dff1cd2af0f5971c05c30aaccb0

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

LOG: Revert "[clang][parser] Set source ranges for GNU-style attributes"

This reverts commit 1ea9fa8c507ec360cf43faf46d13b149e37c950d.

Added: 


Modified: 
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Parse/ParseDecl.cpp
clang/test/AST/sourceranges.cpp
clang/test/SemaCXX/switch-implicit-fallthrough.cpp

Removed: 




diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 6f6d4697e6d09..b67e541836d59 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1572,6 +1572,27 @@ class Parser : public CodeCompletionHandler {
 
   
//======//
   // C99 6.9: External Definitions.
+  struct ParsedAttributesWithRange : ParsedAttributes {
+ParsedAttributesWithRange(AttributeFactory &factory)
+  : ParsedAttributes(factory) {}
+
+void clear() {
+  ParsedAttributes::clear();
+  Range = SourceRange();
+}
+
+SourceRange Range;
+  };
+  struct ParsedAttributesViewWithRange : ParsedAttributesView {
+ParsedAttributesViewWithRange() : ParsedAttributesView() {}
+void clearListOnly() {
+  ParsedAttributesView::clearListOnly();
+  Range = SourceRange();
+}
+
+SourceRange Range;
+  };
+
   DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributesWithRange &attrs,
   ParsingDeclSpec *DS = nullptr);
   bool isDeclarationAfterDeclarator();
@@ -2704,50 +2725,17 @@ class Parser : public CodeCompletionHandler {
   D.takeAttributes(attrs, endLoc);
 }
   }
-
-  /// Parses GNU-style attributes and returns them without source range
-  /// information.
-  ///
-  /// This API is discouraged. Use the version that takes a
-  /// ParsedAttributesWithRange instead.
-  bool MaybeParseGNUAttributes(ParsedAttributes &Attrs,
-   SourceLocation *EndLoc = nullptr,
+  bool MaybeParseGNUAttributes(ParsedAttributes &attrs,
+   SourceLocation *endLoc = nullptr,
LateParsedAttrList *LateAttrs = nullptr) {
 if (Tok.is(tok::kw___attribute)) {
-  ParsedAttributesWithRange AttrsWithRange(AttrFactory);
-  ParseGNUAttributes(Attrs, EndLoc, LateAttrs);
-  Attrs.takeAllFrom(AttrsWithRange);
+  ParseGNUAttributes(attrs, endLoc, LateAttrs);
   return true;
 }
 return false;
   }
-
-  bool MaybeParseGNUAttributes(ParsedAttributesWithRange &Attrs,
-   SourceLocation *EndLoc = nullptr,
-   LateParsedAttrList *LateAttrs = nullptr) {
-if (Tok.is(tok::kw___attribute)) {
-  ParseGNUAttributes(Attrs, EndLoc, LateAttrs);
-  return true;
-}
-return false;
-  }
-
-  /// Parses GNU-style attributes and returns them without source range
-  /// information.
-  ///
-  /// This API is discouraged. Use the version that takes a
-  /// ParsedAttributesWithRange instead.
-  void ParseGNUAttributes(ParsedAttributes &Attrs,
-  SourceLocation *EndLoc = nullptr,
-  LateParsedAttrList *LateAttrs = nullptr,
-  Declarator *D = nullptr) {
-ParsedAttributesWithRange AttrsWithRange(AttrFactory);
-ParseGNUAttributes(AttrsWithRange, EndLoc, LateAttrs, D);
-Attrs.takeAllFrom(AttrsWithRange);
-  }
-
-  void ParseGNUAttributes(ParsedAttributesWithRange &Attrs,
-  SourceLocation *EndLoc = nullptr,
+  void ParseGNUAttributes(ParsedAttributes &attrs,
+  SourceLocation *endLoc = nullptr,
   LateParsedAttrList *LateAttrs = nullptr,
   Declarator *D = nullptr);
   void ParseGNUAttributeArgs(IdentifierInfo *AttrName,

diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index fb086e3c1e50f..a3d82fcd84f7a 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -1034,27 +1034,6 @@ class ParsedAttributes : public ParsedAttributesView {
   mutable AttributePool pool;
 };
 
-struct ParsedAttributesWithRange : ParsedAttributes {
-  ParsedAttributesWithRange(AttributeFactory &factory)
-  : ParsedAttributes(factory) {}
-
-  void clear() {
-ParsedAttributes::clear();
-Range = SourceRange();
-  }
-
-  SourceRange Range;
-};
-struct ParsedAttributesViewWithRange : ParsedAttributesView {
-  ParsedAttributesViewWithRange() : ParsedAttributesView() {}
-  void clearListOnly() {
-Parsed

[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Damn. Reverted again for the time being. The libc++ build seems to fail and I 
won't have time to look into that this week. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75844

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


[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D75844#2664119 , @tbaeder wrote:

> Damn. Reverted again for the time being. The libc++ build seems to fail and I 
> won't have time to look into that this week. :(

Thank you for the quick revert! From looking at the test failure, it looks like 
we may be relying on accepting attributes in more places than we expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75844

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


[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2021-04-01 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Yep. I'll try to come up with a test and a fix next week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75844

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


[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

2021-04-01 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added inline comments.



Comment at: clang/cmake/modules/AddClang.cmake:127
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+  RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})

Ericson2314 wrote:
> compnerd wrote:
> > Ericson2314 wrote:
> > > compnerd wrote:
> > > > For the initial change, Id leave this off.  `CMAKE_INSTALL_LIBDIR` 
> > > > sometimes already deals with the bit suffix, so you can end up with two 
> > > > instances of `32` or `64`.  I think that cleaning that up separately, 
> > > > while focusing on the details of cleaning up how to handle 
> > > > `LLVM_LIBDIR_SUFFIX` is the right thing to do.  The same applies to the 
> > > > rest of the patch.
> > > https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/GNUInstallDirs.cmake#L257
> > >  Hmm I see what you mean. So you are saying `s/${ 
> > > CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}/${ CMAKE_INSTALL_LIBDIR}/` 
> > > everywhere?
> > Yes, that is what I was referring to.  I'm suggesting that you do *not* 
> > make that change instead.  That needs a much more involved change to clean 
> > up the use of `${LLVM_LIBDIR_SUFFIX}`.  I think that this change is already 
> > extremely large as is, and folding more into it is not going to help.
> So you are saying II should back all of these out into 
> `lib${LLVM_LIBDIR_SUFFIX}` as they were before, for now?
> 
> Yes I don't want to make this bigger either, and would rather be on the hook 
> for follow-up work than have this one be too massive to get over the finish 
> line.
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[PATCH] D99190: [SYCL] Add design document for SYCL mode

2021-04-01 Thread Alexey Bader via Phabricator via cfe-commits
bader marked an inline comment as done.
bader added inline comments.



Comment at: clang/docs/SYCLSupport.md:123
+traverse all symbols accessible from kernel functions and add them to the
+"device part" of the code marking them with the new SYCL device attribute.
+

Naghasan wrote:
> bader wrote:
> > Naghasan wrote:
> > > ABataev wrote:
> > > > bader wrote:
> > > > > Naghasan wrote:
> > > > > > OpenMP offload uses a similar approach isn't it? Might be worth to 
> > > > > > describe how the 2 relates to each other and where they diverge. 
> > > > > Do you mean the approach OpenMP compiler uses to outline 
> > > > > single-source code parts to offload?
> > > > > To be honest, I'm not sure... @ABataev, is there any description how 
> > > > > OpenMP compiler outlines device code?
> > > > > https://clang.llvm.org/docs/OpenMPSupport.html doesn't provide much 
> > > > > details unfortunately.
> > > > I don't think we have anything like this. Moreover, currently, there 
> > > > are 2 different models, one with outlining by the frontend and another 
> > > > one with the outlining by the LLVM.
> > > I mentioned that as I know there is some support for CUDA and the clang 
> > > driver openmp offload works with multiple frontend passes.
> > > If the model(s) is too different then there is no point going further 
> > > here. 
> > > 
> > > > Moreover, currently, there are 2 different models, one with outlining 
> > > > by the frontend and another one with the outlining by the LLVM.
> > > 
> > > I do recall some brief conversations about that. Are they meant to work 
> > > in pair or one aims to replace the other ?
> > What do say if add a TODO here (or in a separate TODO document) to study 
> > more about differences between SYCL/OpenMP-offload/CUDA implementation 
> > designs?
> > It seems to be useful to understanding if we want to re-use other 
> > programming model experience with implementing common tasks like device 
> > code outlining.
> fine by me :)
I've added a TODO section to the document.



Comment at: clang/docs/SYCLSupport.rst:198
+   class MyObj {
+ accessor _ptr; // accessor contains a pointer to the global 
address space.
+   public:

Naghasan wrote:
> Oups sorry, I just noticed  I made a mistake in my suggestion, that's more in 
> line with the statement `KernelFuncObj.A.__init(a);` in the pseudo  opencl
Thanks. Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99190

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


[PATCH] D93822: [clang-tidy] Add check for implicit widening of multiplication result

2021-04-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

The CI is showing build failures and there are some clang-tidy nits to be 
addressed as well.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:29
+  // Is this:  long r = int(x) * int(y);  ?
+  // FIXME: shall we skip brackets/casts/etc?
+  auto *BO = dyn_cast(E);

I think we should skip parens at the very least. `x * y` should check the same 
as `(x) * (y)`.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:32
+  if (!BO || BO->getOpcode() != BO_Mul)
+// FIXME: what about:  long r = int(x) + (int(y) * int(z));  ?
+return nullptr;

I think that could be handled in a follow-up if we find the need, WDYT?



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:75
+  else if (Ty->isSignedIntegerType()) {
+assert(ETy->isUnsignedIntegerType() &&
+   "Expected source type to be signed.");

Might be worth it to have tests around `signed char`, `unsigned char`, and 
`char` explicitly, as that gets awkward.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:126
+  QualType SizeTy = IndexExprType->isSignedIntegerType() ? SSizeTy : USizeTy;
+  // FIXME: is there a way to actually get the QualType for size_t/ssize_t?
+  StringRef TyAsString =

You already are using the way?



Comment at: clang/lib/AST/ASTContext.cpp:10158
+  if (const auto *ETy = T->getAs())
+T = ETy->getDecl()->getIntegerType();
+

This looks to be getting the same value as in the 
`getCorrespondingUnsignedType()` call?



Comment at: clang/lib/AST/ASTContext.cpp:10204
+return SatLongFractTy;
+  default:
+llvm_unreachable("Unexpected unsigned integer or fixed point type");

It looks like `_ExtInt` is missing from both this function and the 
corresponding unsigned one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93822

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


[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

2021-04-01 Thread H.J Lu via Phabricator via cfe-commits
hjl.tools added a comment.

In D99708#2664076 , @LuoYuanke wrote:

> In D99708#2663989 , @craig.topper 
> wrote:
>
>> A user interrupt is different than a regular interrupt right? It doesn't 
>> make sense that we would change the behavior of the interrupt calling 
>> convention just because the the user interrupt instructions are enabled. 
>> That would occur just from passing a -march for a newer CPU wouldn't it?
>
> Maybe need support another attribute "__attribute__ ((user_interrupt))" for 
> functions? However this is what gcc does 
> (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler 
in the source, there is no need for another
attribute.   We discussed that kernel might need to use UINTR instructions.  We 
decided that kernel could use inline asm
statements if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99708

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


[PATCH] D99622: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang

2021-04-01 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen updated this revision to Diff 334720.
cchen added a comment.

Update codegen test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99622

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/parallel_ast_print.cpp
  clang/test/OpenMP/parallel_proc_bind_messages.cpp
  clang/test/OpenMP/parallel_proc_bind_primary_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -967,6 +967,7 @@
 __OMP_PROC_BIND_KIND(master, 2)
 __OMP_PROC_BIND_KIND(close, 3)
 __OMP_PROC_BIND_KIND(spread, 4)
+__OMP_PROC_BIND_KIND(primary, 5)
 __OMP_PROC_BIND_KIND(default, 6)
 __OMP_PROC_BIND_KIND(unknown, 7)
 
Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -103,13 +103,15 @@
 def OMP_PROC_BIND_master : ClauseVal<"master",2,1> {}
 def OMP_PROC_BIND_close : ClauseVal<"close",3,1> {}
 def OMP_PROC_BIND_spread : ClauseVal<"spread",4,1> {}
-def OMP_PROC_BIND_default : ClauseVal<"default",5,0> {}
-def OMP_PROC_BIND_unknown : ClauseVal<"unknown",6,0> { let isDefault = true; }
+def OMP_PROC_BIND_primary : ClauseVal<"primary",5,1> {}
+def OMP_PROC_BIND_default : ClauseVal<"default",6,0> {}
+def OMP_PROC_BIND_unknown : ClauseVal<"unknown",7,0> { let isDefault = true; }
 def OMPC_ProcBind : Clause<"proc_bind"> {
   let clangClass = "OMPProcBindClause";
   let flangClass = "OmpProcBindClause";
   let enumClauseValue = "ProcBindKind";
   let allowedClauseValues = [
+OMP_PROC_BIND_primary,
 OMP_PROC_BIND_master,
 OMP_PROC_BIND_close,
 OMP_PROC_BIND_spread,
Index: clang/test/OpenMP/parallel_proc_bind_primary_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/parallel_proc_bind_primary_codegen.cpp
@@ -0,0 +1,48 @@
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 -x c++ -triple %itanium_abi_triple -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -x c++ -std=c++11 -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=51 -x c++ -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=51 -x c++ -triple %itanium_abi_triple -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=51 -x c++ -std=c++11 -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=51 -x c++ -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
+#ifndef HEADER
+#define HEADER
+
+typedef __INTPTR_TYPE__ intptr_t;
+
+// CHECK-DAG: [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
+// CHECK-DAG: [[STR:@.+]] = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00"
+// CHECK-DAG: [[DEF_LOC_2:@.+]] = private unnamed_addr constant [[IDENT_T_TY]] { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[STR]], i32 0, i32 0) }
+
+void foo();
+
+template 
+T tmain() {
+#pragma omp parallel proc_bind(primary)
+  foo();
+  return T();
+}
+
+int main() {
+#pragma omp parallel proc_bind(primary)
+  foo();
+  return tmain();
+}
+
+// CHECK-LABEL: @main
+// CHECK:   [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEF_LOC_2]])
+// CHECK:   call {{.*}}void @__kmpc_push_proc_bind([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 [[GTID]], i32 5)
+// CHECK:   call {{.*}}void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(
+
+// CHECK-LABEL: @{{.+}}tmain
+// CHECK:   [[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEF_LOC_2]])
+// CHECK:   call {{.*}}void @__kmpc_push_proc_bind([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 [[GTID]], i32 5)
+// CHECK:   call {{.*}}void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(
+// CHECK:   ret i32 0
+// CHECK-NEXT:  }
+
+#endif
+
Index: clang/test/OpenMP/parallel_proc_bind_messages.cpp
===
--- clang/test/OpenMP/parallel_proc_bind_messages.cpp
+++ clang/test/OpenMP/parallel_proc_bind_messages.cpp
@@ -1,16 +1,20 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -o - %s

[PATCH] D99738: [HIP] remove overloaded abs in header

2021-04-01 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: tra, JonChesterfield, ashi1.
yaxunl requested review of this revision.

This function seems to be introduced by accident by 
https://github.com/llvm/llvm-project/commit/aa2b593f1495a972a4a592952760ec9d5f7c01f1

Such overloaded abs function did not exist before the refactoring, and does not 
exist in 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/__clang_cuda_cmath.h

Conceptually it also does not make sense, since it adds something like

double abs(int x) {

  return ::abs((double)x);

}

It caused regressions in CuPy.


https://reviews.llvm.org/D99738

Files:
  clang/lib/Headers/__clang_hip_cmath.h


Index: clang/lib/Headers/__clang_hip_cmath.h
===
--- clang/lib/Headers/__clang_hip_cmath.h
+++ clang/lib/Headers/__clang_hip_cmath.h
@@ -323,7 +323,6 @@
   }
 #endif
 
-__HIP_OVERLOAD1(double, abs)
 __HIP_OVERLOAD1(double, acos)
 __HIP_OVERLOAD1(double, acosh)
 __HIP_OVERLOAD1(double, asin)


Index: clang/lib/Headers/__clang_hip_cmath.h
===
--- clang/lib/Headers/__clang_hip_cmath.h
+++ clang/lib/Headers/__clang_hip_cmath.h
@@ -323,7 +323,6 @@
   }
 #endif
 
-__HIP_OVERLOAD1(double, abs)
 __HIP_OVERLOAD1(double, acos)
 __HIP_OVERLOAD1(double, acosh)
 __HIP_OVERLOAD1(double, asin)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99738: [HIP] remove overloaded abs in header

2021-04-01 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

I have tested removing this did not cause regressions in our CI.


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

https://reviews.llvm.org/D99738

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


[PATCH] D99622: [OpenMP51] Accept `primary` as proc bind affinity policy in Clang

2021-04-01 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99622

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


[PATCH] D99738: [HIP] remove overloaded abs in header

2021-04-01 Thread Aaron Enye Shi via Phabricator via cfe-commits
ashi1 accepted this revision.
ashi1 added a comment.
This revision is now accepted and ready to land.

LGTM, thank you.


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

https://reviews.llvm.org/D99738

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


[PATCH] D99580: [CLANG] [DebugInfo] Convert File name to native format

2021-04-01 Thread Adrian McCarthy via Phabricator via cfe-commits
amccarth added a comment.

In D99580#2660040 , @kamleshbhalui 
wrote:

> In D99580#2659858 , @amccarth wrote:
>
>> It looks like the code change is for everyone, but the new test is specific 
>> to mingw.
>
> For Linux like platform it does not create problem because input file  path 
> is already in POSIX style, so having a test case will not test the change 
> because even without the change it will pass.

The fix is on the code path for all platforms, so running the test on all 
platforms could help catch future regressions.  There could also be unusual 
cases, such as cross compiling for Linux on Windows, and thus you could have 
Windows-style paths even though the target is Posix.  Or someone might further 
refine the fix to make more cases work correctly for mingw but inadvertently 
cause problems for other platforms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99580

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


[clang] 56b39af - [OpenMP51][DOCS] Mark "add present modifier in defaultmap clause" as

2021-04-01 Thread via cfe-commits

Author: cchen
Date: 2021-04-01T11:02:23-05:00
New Revision: 56b39afb58627507ffbc7eaa749781a30b750c03

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

LOG: [OpenMP51][DOCS] Mark "add present modifier in defaultmap clause" as
done, NFC.

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index e66d9a64a93f..6e15716f3bb4 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -272,7 +272,7 @@ want to help with the implementation.
 
+--+--+--+---+
 | device extension | 'present' motion modifier 
   | :good:`done` | D84711, D84712  
  |
 
+--+--+--+---+
-| device extension | 'present' in defaultmap clause
   | :part:`worked on`| D92427  
  |
+| device extension | 'present' in defaultmap clause
   | :good:`done` | D92427  
  |
 
+--+--+--+---+
 | device extension | map clause reordering reordering based on 
'present' modifier | :none:`unclaimed`| 
  |
 
+--+--+--+---+



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


[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

2021-04-01 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D99708#2664164 , @hjl.tools wrote:

> In D99708#2664076 , @LuoYuanke wrote:
>
>> In D99708#2663989 , @craig.topper 
>> wrote:
>>
>>> A user interrupt is different than a regular interrupt right? It doesn't 
>>> make sense that we would change the behavior of the interrupt calling 
>>> convention just because the the user interrupt instructions are enabled. 
>>> That would occur just from passing a -march for a newer CPU wouldn't it?
>>
>> Maybe need support another attribute "__attribute__ ((user_interrupt))" for 
>> functions? However this is what gcc does 
>> (https://gcc.godbolt.org/z/8ojTMG6bT).
>
> Since there won't be both user interrupt handler and kernel interrupt handler 
> in the source, there is no need for another
> attribute.   We discussed that kernel might need to use UINTR instructions.  
> We decided that kernel could use inline asm
> statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If 
tomorrow I change my command line to -march=sapphirerapids, now my kernel 
interrupt code generates user interrupt instructions. That seems surprising.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99708

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


[PATCH] D99668: [RISCV][Clang] Add some RVV Floating-Point intrinsic functions.

2021-04-01 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper 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/D99668/new/

https://reviews.llvm.org/D99668

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


[clang] 85ff35a - [HIP] remove overloaded abs in header

2021-04-01 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2021-04-01T12:23:29-04:00
New Revision: 85ff35a9529a1ea9ed7ab8cda10761d66705d518

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

LOG: [HIP] remove overloaded abs in header

This function seems to be introduced by accident by
https://github.com/llvm/llvm-project/commit/aa2b593f1495a972a4a592952760ec9d5f7c01f1

Such overloaded abs function did not exist before
the refactoring, and does not exist in
https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/__clang_cuda_cmath.h

Conceptually it also does not make sense, since it adds something like

double abs(int x) {
  return ::abs((double)x);
}

It caused regressions in CuPy.

Reviewed by: Aaron Enye Shi, Artem Belevich

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

Added: 


Modified: 
clang/lib/Headers/__clang_hip_cmath.h

Removed: 




diff  --git a/clang/lib/Headers/__clang_hip_cmath.h 
b/clang/lib/Headers/__clang_hip_cmath.h
index cd22a2df954bc..18871e63bfa0f 100644
--- a/clang/lib/Headers/__clang_hip_cmath.h
+++ b/clang/lib/Headers/__clang_hip_cmath.h
@@ -312,7 +312,6 @@ class __promote : public __promote_imp<_A1, _A2, _A3> {};
   }
 #endif
 
-__HIP_OVERLOAD1(double, abs)
 __HIP_OVERLOAD1(double, acos)
 __HIP_OVERLOAD1(double, acosh)
 __HIP_OVERLOAD1(double, asin)



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


[PATCH] D99738: [HIP] remove overloaded abs in header

2021-04-01 Thread Yaxun Liu 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 rG85ff35a9529a: [HIP] remove overloaded abs in header 
(authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99738

Files:
  clang/lib/Headers/__clang_hip_cmath.h


Index: clang/lib/Headers/__clang_hip_cmath.h
===
--- clang/lib/Headers/__clang_hip_cmath.h
+++ clang/lib/Headers/__clang_hip_cmath.h
@@ -312,7 +312,6 @@
   }
 #endif
 
-__HIP_OVERLOAD1(double, abs)
 __HIP_OVERLOAD1(double, acos)
 __HIP_OVERLOAD1(double, acosh)
 __HIP_OVERLOAD1(double, asin)


Index: clang/lib/Headers/__clang_hip_cmath.h
===
--- clang/lib/Headers/__clang_hip_cmath.h
+++ clang/lib/Headers/__clang_hip_cmath.h
@@ -312,7 +312,6 @@
   }
 #endif
 
-__HIP_OVERLOAD1(double, abs)
 __HIP_OVERLOAD1(double, acos)
 __HIP_OVERLOAD1(double, acosh)
 __HIP_OVERLOAD1(double, asin)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99688: [CUDA][HIP] rename -fcuda-flush-denormals-to-zero

2021-04-01 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Driver/cuda-flush-denormals-to-zero.cu:2
 // Checks that cuda compilation does the right thing when passed
-// -fcuda-flush-denormals-to-zero. This should be translated to
+// -fgpu-flush-denormals-to-zero. This should be translated to
 // -fdenormal-fp-math-f32=preserve-sign

It would be good to add a couple of tests to verify that 
`-fcuda-flush-denormals-to-zero` still work as we still need them for existing 
users.


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

https://reviews.llvm.org/D99688

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


[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

2021-04-01 Thread H.J Lu via Phabricator via cfe-commits
hjl.tools added a comment.

In D99708#2664218 , @craig.topper 
wrote:

> In D99708#2664164 , @hjl.tools wrote:
>
>> In D99708#2664076 , @LuoYuanke 
>> wrote:
>>
>>> In D99708#2663989 , @craig.topper 
>>> wrote:
>>>
 A user interrupt is different than a regular interrupt right? It doesn't 
 make sense that we would change the behavior of the interrupt calling 
 convention just because the the user interrupt instructions are enabled. 
 That would occur just from passing a -march for a newer CPU wouldn't it?
>>>
>>> Maybe need support another attribute "__attribute__ ((user_interrupt))" for 
>>> functions? However this is what gcc does 
>>> (https://gcc.godbolt.org/z/8ojTMG6bT).
>>
>> Since there won't be both user interrupt handler and kernel interrupt 
>> handler in the source, there is no need for another
>> attribute.   We discussed that kernel might need to use UINTR instructions.  
>> We decided that kernel could use inline asm
>> statements if needed.
>
> So if write kernel code and compile with -march=haswell today, I get IRET. If 
> tomorrow I change my command line to -march=sapphirerapids, now my kernel 
> interrupt code generates user interrupt instructions. That seems surprising.

-mcmodel=kernel should disable uiret.:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99708

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


[PATCH] D99708: [X86] Enable compilation of user interrupt handlers.

2021-04-01 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D99708#2664351 , @hjl.tools wrote:

> In D99708#2664218 , @craig.topper 
> wrote:
>
>> In D99708#2664164 , @hjl.tools 
>> wrote:
>>
>>> In D99708#2664076 , @LuoYuanke 
>>> wrote:
>>>
 In D99708#2663989 , @craig.topper 
 wrote:

> A user interrupt is different than a regular interrupt right? It doesn't 
> make sense that we would change the behavior of the interrupt calling 
> convention just because the the user interrupt instructions are enabled. 
> That would occur just from passing a -march for a newer CPU wouldn't it?

 Maybe need support another attribute "__attribute__ ((user_interrupt))" 
 for functions? However this is what gcc does 
 (https://gcc.godbolt.org/z/8ojTMG6bT).
>>>
>>> Since there won't be both user interrupt handler and kernel interrupt 
>>> handler in the source, there is no need for another
>>> attribute.   We discussed that kernel might need to use UINTR instructions. 
>>>  We decided that kernel could use inline asm
>>> statements if needed.
>>
>> So if write kernel code and compile with -march=haswell today, I get IRET. 
>> If tomorrow I change my command line to -march=sapphirerapids, now my kernel 
>> interrupt code generates user interrupt instructions. That seems surprising.
>
> -mcmodel=kernel should disable uiret.:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870

That makes sense. Can we put that in this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99708

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


  1   2   >