[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

2020-10-22 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

Does this bug only effect the printing of inline assembly to a .s file? The 
encoder should work correctly even without this I think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90009

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


[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

2020-10-22 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp:358
+  else if (Flags & X86::Force_EVEXEncoding)
+O << "\t{evex}";
 }

We also need to print {disp8} and {disp32} here to fix the same bug with those 
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90009

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


[PATCH] D89913: SourceManager: Encapsulate line number mapping into SrcMgr::LineOffsetMapping

2020-10-22 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere accepted this revision.
JDevlieghere added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Basic/SourceManager.h:118
+  private:
+unsigned *Storage = nullptr;
+  };

I guess it's implicit in the implementation, but maybe it's worth adding a 
comment describing the layout (first element is the size, elements at index i 
are at index i+1. 


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

https://reviews.llvm.org/D89913

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


[PATCH] D89554: SourceManager: Clarify that FileInfo always has a ContentCache, NFC

2020-10-22 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik accepted this revision.
shafik added a comment.

LGTM


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

https://reviews.llvm.org/D89554

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


[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

2020-10-22 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp:354
+  else if (Flags & X86::Force_VEX2Encoding)
+O << "\t{vex2}";
+  else if (Flags & X86::Force_VEX3Encoding)

craig.topper wrote:
> Is it important that we use {vex2} instead of just treating it as {vex}?
I guess we'd actually have to print {vex2} instead of {vex} for compatibility 
with older versions of GNU assember that don't support {vex}. So I guess maybe 
they should be different as much as I hate wasting flags


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90009

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


[PATCH] D89580: SourceManager: Fix an SLocEntry memory regression introduced with FileEntryRef

2020-10-22 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere added inline comments.



Comment at: clang/include/clang/Basic/SourceManager.h:293
   X.ContentAndKind.setInt(FileCharacter);
-  X.Filename = Filename;
+  const_cast(Con).Filename = Filename;
   return X;

Would it possibly make more sense to drop the `const` qualifier from the 
argument? 


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

https://reviews.llvm.org/D89580

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


[PATCH] D89554: SourceManager: Clarify that FileInfo always has a ContentCache, NFC

2020-10-22 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM if Shafik is happy


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

https://reviews.llvm.org/D89554

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


[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

2020-10-22 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:3896
+// encoder.
 Prefixes |= X86::IP_USE_VEX3;
+Prefixes |= X86::Force_VEX3Encoding;

Why do we need Force_VEX3Encoding and IP_USE_VEX3?



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:67
+IP_USE_DISP32 = 1U << 8,
+Force_VEXEncoding = 1U << 9,
+Force_VEX2Encoding = 1U << 10,

Why don't these start with IP_?



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp:354
+  else if (Flags & X86::Force_VEX2Encoding)
+O << "\t{vex2}";
+  else if (Flags & X86::Force_VEX3Encoding)

Is it important that we use {vex2} instead of just treating it as {vex}?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90009

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


[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

2020-10-22 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 added inline comments.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2824
 ForcedVEXEncoding = VEXEncoding_VEX;
+  else if (Prefix == "vex2")
+ForcedVEXEncoding = VEXEncoding_VEX2;

pengfei wrote:
> I think it's reasonable if we generate "{vex}" for input "{vex2}"
GCC will out put {vex2} if the input is {vex2}.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90009

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


[PATCH] D90010: clang-tidy: Reduce number of stderr write calls

2020-10-22 Thread Hiral via Phabricator via cfe-commits
Hiralo created this revision.
Hiralo added reviewers: alexfh, njames93, hokein, DmitryPolukhin, djasper, 
ilya-biryukov.
Hiralo added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Hiralo requested review of this revision.

commit c3fa7da502d6cc77e5c0710b0e9308e6fbdd137c
Author: Hiral Oza 
Date:   Thu Oct 22 22:06:25 2020 -0700

  clang-tidy: Reduce number of stderr write calls
  
  Making single write call in place of 7 small string write calls.

Thank you in advance for your kind review.
-Hiral


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90010

Files:
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -241,31 +241,36 @@
 namespace tidy {
 
 static void printStats(const ClangTidyStats ) {
+  std::string Msg;
+  llvm::raw_string_ostream ErrStream(Msg);
+
   if (Stats.errorsIgnored()) {
-llvm::errs() << "Suppressed " << Stats.errorsIgnored() << " warnings (";
+ErrStream << "Suppressed " << Stats.errorsIgnored() << " warnings (";
+
 StringRef Separator = "";
 if (Stats.ErrorsIgnoredNonUserCode) {
-  llvm::errs() << Stats.ErrorsIgnoredNonUserCode << " in non-user code";
+  ErrStream << Stats.ErrorsIgnoredNonUserCode << " in non-user code";
   Separator = ", ";
 }
 if (Stats.ErrorsIgnoredLineFilter) {
-  llvm::errs() << Separator << Stats.ErrorsIgnoredLineFilter
-   << " due to line filter";
+  ErrStream << Separator << Stats.ErrorsIgnoredLineFilter
+<< " due to line filter";
   Separator = ", ";
 }
 if (Stats.ErrorsIgnoredNOLINT) {
-  llvm::errs() << Separator << Stats.ErrorsIgnoredNOLINT << " NOLINT";
+  ErrStream << Separator << Stats.ErrorsIgnoredNOLINT << " NOLINT";
   Separator = ", ";
 }
 if (Stats.ErrorsIgnoredCheckFilter)
-  llvm::errs() << Separator << Stats.ErrorsIgnoredCheckFilter
-   << " with check filters";
-llvm::errs() << ").\n";
+  ErrStream << Separator << Stats.ErrorsIgnoredCheckFilter
+<< " with check filters";
+ErrStream << ").\n";
 if (Stats.ErrorsIgnoredNonUserCode)
-  llvm::errs() << "Use -header-filter=.* to display errors from all "
-  "non-system headers. Use -system-headers to display "
-  "errors from system headers as well.\n";
+  ErrStream << "Use -header-filter=.* to display errors from all "
+   "non-system headers. Use -system-headers to display "
+   "errors from system headers as well.\n";
   }
+  llvm::errs() << ErrStream.str();
 }
 
 static std::unique_ptr createOptionsProvider(


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -241,31 +241,36 @@
 namespace tidy {
 
 static void printStats(const ClangTidyStats ) {
+  std::string Msg;
+  llvm::raw_string_ostream ErrStream(Msg);
+
   if (Stats.errorsIgnored()) {
-llvm::errs() << "Suppressed " << Stats.errorsIgnored() << " warnings (";
+ErrStream << "Suppressed " << Stats.errorsIgnored() << " warnings (";
+
 StringRef Separator = "";
 if (Stats.ErrorsIgnoredNonUserCode) {
-  llvm::errs() << Stats.ErrorsIgnoredNonUserCode << " in non-user code";
+  ErrStream << Stats.ErrorsIgnoredNonUserCode << " in non-user code";
   Separator = ", ";
 }
 if (Stats.ErrorsIgnoredLineFilter) {
-  llvm::errs() << Separator << Stats.ErrorsIgnoredLineFilter
-   << " due to line filter";
+  ErrStream << Separator << Stats.ErrorsIgnoredLineFilter
+<< " due to line filter";
   Separator = ", ";
 }
 if (Stats.ErrorsIgnoredNOLINT) {
-  llvm::errs() << Separator << Stats.ErrorsIgnoredNOLINT << " NOLINT";
+  ErrStream << Separator << Stats.ErrorsIgnoredNOLINT << " NOLINT";
   Separator = ", ";
 }
 if (Stats.ErrorsIgnoredCheckFilter)
-  llvm::errs() << Separator << Stats.ErrorsIgnoredCheckFilter
-   << " with check filters";
-llvm::errs() << ").\n";
+  ErrStream << Separator << Stats.ErrorsIgnoredCheckFilter
+<< " with check filters";
+ErrStream << ").\n";
 if (Stats.ErrorsIgnoredNonUserCode)
-  llvm::errs() << "Use -header-filter=.* to display errors from all "
-  "non-system headers. Use -system-headers to display "
-  "errors from system headers as well.\n";
+  ErrStream << "Use -header-filter=.* to display errors from all "
+   "non-system headers. Use -system-headers to display "
+ 

[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

2020-10-22 Thread Pengfei Wang via Phabricator via cfe-commits
pengfei added inline comments.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2824
 ForcedVEXEncoding = VEXEncoding_VEX;
+  else if (Prefix == "vex2")
+ForcedVEXEncoding = VEXEncoding_VEX2;

I think it's reasonable if we generate "{vex}" for input "{vex2}"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90009

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


[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

2020-10-22 Thread Pengfei Wang via Phabricator via cfe-commits
pengfei added inline comments.



Comment at: clang/test/CodeGen/X86/att-inline-asm-prefix.c:12
+// CHECK: {evex} vcvtps2pd %xmm0, %xmm1
+"{vex} vcvtps2pd %xmm0, %xmm1\n\t"
+"{vex2} vcvtps2pd %xmm0, %xmm1\n\t"

Better adding a no prefix one.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:57-70
 IP_NO_PREFIX = 0,
-IP_HAS_OP_SIZE = 1,
-IP_HAS_AD_SIZE = 2,
-IP_HAS_REPEAT_NE = 4,
-IP_HAS_REPEAT = 8,
-IP_HAS_LOCK = 16,
-IP_HAS_NOTRACK = 32,
-IP_USE_VEX3 = 64,
-IP_USE_DISP8 = 128,
-IP_USE_DISP32 = 256,
+IP_HAS_OP_SIZE = 1U << 0,
+IP_HAS_AD_SIZE = 1U << 1,
+IP_HAS_REPEAT_NE = 1U << 2,
+IP_HAS_REPEAT = 1U << 3,
+IP_HAS_LOCK = 1U << 4,
+IP_HAS_NOTRACK = 1U << 5,

Can we make these `= 1U <<` aligned?



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp:352
+  if (Flags & X86::Force_VEXEncoding)
+O << "\t{vex}";
+  else if (Flags & X86::Force_VEX2Encoding)

`"\t{vex}\t"` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90009

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


[PATCH] D90009: [X86] VEX/EVEX prefix doesn't work for inline assembly.

2020-10-22 Thread LiuChen via Phabricator via cfe-commits
LiuChen3 created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
LiuChen3 requested review of this revision.

For now, we lost the encoding information if we using inline assembly.
The encoding for the inline assembly will keep default even if we add
the vex/evex prefix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90009

Files:
  clang/test/CodeGen/X86/att-inline-asm-prefix.c
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
  llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp

Index: llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
===
--- llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
+++ llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
@@ -346,6 +346,16 @@
 O << "\trepne\t";
   else if (Flags & X86::IP_HAS_REPEAT)
 O << "\trep\t";
+
+  // These all require a pseudo prefix
+  if (Flags & X86::Force_VEXEncoding)
+O << "\t{vex}";
+  else if (Flags & X86::Force_VEX2Encoding)
+O << "\t{vex2}";
+  else if (Flags & X86::Force_VEX3Encoding)
+O << "\t{vex3}";
+  else if (Flags & X86::Force_EVEXEncoding)
+O << "\t{evex}";
 }
 
 void X86InstPrinterCommon::printVKPair(const MCInst *MI, unsigned OpNo,
Index: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
===
--- llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
+++ llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
@@ -55,15 +55,19 @@
   /// The constants to describe instr prefixes if there are
   enum IPREFIXES {
 IP_NO_PREFIX = 0,
-IP_HAS_OP_SIZE = 1,
-IP_HAS_AD_SIZE = 2,
-IP_HAS_REPEAT_NE = 4,
-IP_HAS_REPEAT = 8,
-IP_HAS_LOCK = 16,
-IP_HAS_NOTRACK = 32,
-IP_USE_VEX3 = 64,
-IP_USE_DISP8 = 128,
-IP_USE_DISP32 = 256,
+IP_HAS_OP_SIZE = 1U << 0,
+IP_HAS_AD_SIZE = 1U << 1,
+IP_HAS_REPEAT_NE = 1U << 2,
+IP_HAS_REPEAT = 1U << 3,
+IP_HAS_LOCK = 1U << 4,
+IP_HAS_NOTRACK = 1U << 5,
+IP_USE_VEX3 = 1U << 6,
+IP_USE_DISP8 = 1U << 7,
+IP_USE_DISP32 = 1U << 8,
+Force_VEXEncoding = 1U << 9,
+Force_VEX2Encoding = 1U << 10,
+Force_VEX3Encoding = 1U << 11,
+Force_EVEXEncoding = 1U << 12,
   };
 
   enum OperandType : unsigned {
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -83,6 +83,7 @@
   enum VEXEncoding {
 VEXEncoding_Default,
 VEXEncoding_VEX,
+VEXEncoding_VEX2,
 VEXEncoding_VEX3,
 VEXEncoding_EVEX,
   };
@@ -2818,8 +2819,10 @@
 return Error(Parser.getTok().getLoc(), "Expected '}'");
   Parser.Lex(); // Eat curly.
 
-  if (Prefix == "vex" || Prefix == "vex2")
+  if (Prefix == "vex")
 ForcedVEXEncoding = VEXEncoding_VEX;
+  else if (Prefix == "vex2")
+ForcedVEXEncoding = VEXEncoding_VEX2;
   else if (Prefix == "vex3")
 ForcedVEXEncoding = VEXEncoding_VEX3;
   else if (Prefix == "evex")
@@ -3837,6 +3840,7 @@
 return Match_Unsupported;
 
   if ((ForcedVEXEncoding == VEXEncoding_VEX ||
+   ForcedVEXEncoding == VEXEncoding_VEX2 ||
ForcedVEXEncoding == VEXEncoding_VEX3) &&
   (MCID.TSFlags & X86II::EncodingMask) != X86II::VEX)
 return Match_Unsupported;
@@ -3879,10 +3883,19 @@
 
   MCInst Inst;
 
-  // If VEX3 encoding is forced, we need to pass the USE_VEX3 flag to the
-  // encoder.
-  if (ForcedVEXEncoding == VEXEncoding_VEX3)
+  // Passing the prefix info to Printer if VEX or EVEX encoding is forced.
+  if (ForcedVEXEncoding == VEXEncoding_VEX)
+Prefixes |= X86::Force_VEXEncoding;
+  else if (ForcedVEXEncoding == VEXEncoding_VEX2)
+Prefixes |= X86::Force_VEX2Encoding;
+  else if (ForcedVEXEncoding == VEXEncoding_EVEX)
+Prefixes |= X86::Force_EVEXEncoding;
+  else if (ForcedVEXEncoding == VEXEncoding_VEX3) {
+// If VEX3 encoding is forced, we need to pass the USE_VEX3 flag to the
+// encoder.
 Prefixes |= X86::IP_USE_VEX3;
+Prefixes |= X86::Force_VEX3Encoding;
+  }
 
   // Set encoded flags for {disp8} and {disp32}.
   if (ForcedDispEncoding == DispEncoding_Disp8)
Index: clang/test/CodeGen/X86/att-inline-asm-prefix.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/att-inline-asm-prefix.c
@@ -0,0 +1,17 @@
+// RUN:%clang_cc1 %s -ferror-limit 0 -triple=x86_64-pc -target-feature +avx512f -target-feature +avx2 -target-feature +avx512vl -S -o -  | FileCheck %s -check-prefix CHECK
+
+// This test is to check if the prefix in inline assembly is correctly
+// preserved.
+
+void check_inline_prefix(void) {
+  __asm__ (
+// CHECK: {vex} vcvtps2pd %xmm0, %xmm1
+// CHECK: {vex2} vcvtps2pd %xmm0, %xmm1
+// CHECK: {vex3} 

[PATCH] D90007: [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

2020-10-22 Thread Tom Stellard via Phabricator via cfe-commits
tstellar created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
tstellar requested review of this revision.

Fix premature decision in the presence of type-dependent expression
operands on whether AltiVec vector initializations from single
expressions are "splat" operations.

Verify that the instantiation is able to determine the correct cast
semantics for both the scalar type and the vector type case.

Note that, because the change only affects the single-expression
case (and the target type is an AltiVec-style vector type), the
replacement of a parenthesized list with a parenthesized expression
does not change the semantics of the program in a program-observable
manner.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90007

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/pr47676.cpp


Index: clang/test/SemaTemplate/pr47676.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly differentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7401,7 +7401,7 @@
 }
 if (PE || PLE->getNumExprs() == 1) {
   Expr *E = (PE ? PE->getSubExpr() : PLE->getExpr(0));
-  if (!E->getType()->isVectorType())
+  if (!E->isTypeDependent() && !E->getType()->isVectorType())
 isVectorLiteral = true;
 }
 else


Index: clang/test/SemaTemplate/pr47676.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly differentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 
+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 
+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}
Index: clang/lib/Sema/SemaExpr.cpp

[PATCH] D89987: [analyzer] [NFC] Rename SymbolRef to SymExprRef

2020-10-22 Thread Ella Ma via Phabricator via cfe-commits
OikawaKirie added a comment.

Since `SymbolRef` is just a `const SymExpr *` in the current codebase, I'd 
prefer using `const SymExpr *` directly, just like how `MemRegion` is used, 
which would be clearer than both `SymbolRef` and `SymExprRef` as far as I am 
thinking.

Different from `ProgramStateRef` which is an alias to `IntrusiveRefCntPtr`, or 
`StoreRef` which is a wrapper object, an alias to a `const SymExpr *` makes no 
sense to me.

And this is also where I have been confused for a long while.

Or on the opposite, do we also need a similar alias for a `const MemRegion *`, 
maybe say `MemRegionRef`? After all, it is shorter. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89987

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


[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

2020-10-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/test/Parser/pragma-fenv_access.c:28
+#if defined(CPP) & defined(STRICT)
+//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a 
constant expression}}
+//not-expected-note@+2 {{compile time floating point arithmetic suppressed in 
strict evaluation modes}}

mibintc wrote:
> rsmith wrote:
> > mibintc wrote:
> > > @rsmith no diagnostic, is this OK?
> > Under the new approach from D89360, I would expect no diagnostic with 
> > `CONST` defined as `constexpr`, because the initializer is then manifestly 
> > constant-evaluated, so should be evaluated in the constant rounding mode.
> > 
> > With `CONST` defined as merely `const`, I'd expect that we emit a 
> > constrained floating-point operation using the runtime rounding mode here. 
> > You can test that we don't constant-evaluate the value of a 
> > (non-`constexpr`) `const float` variable by using this hack (with `CONST` 
> > defined as `const`):
> > 
> > ```
> > enum {
> >   e1 = (int)one, e3 = (int)three, e4 = (int)four, e_four_quarters = 
> > (int)(frac_ok * 4)
> > };
> > static_assert(e1 == 1  && e3 == 3 && e4 == 4 && e_four_quarters == 1, "");
> > enum {
> >   e_three_thirds = (int)(frac * 3) // expected-error {{not an integral 
> > constant expression}}
> > };
> > ```
> > 
> > (The hack here is that we permit constant folding in the values of 
> > enumerators, and we allow constant folding to look at the evaluated values 
> > of `const float` variables. This should not be allowed for `frac`, because 
> > attempting to evaluate its initializer should fail.)
> I'm not seeing the expected-error for e_three_thirds = (int)(frac * 3)
Are you sync'd past commit rG08c8d5bc51c512e605840b8003fcf38c86d0fc96? We had a 
bug that would incorrectly treat the initializer of `frac` as being in a 
constant context until pretty recently.

This testcase:

```
int main() {
  const float f = 1.0 / 3.0;
  enum { e = (int)(f * 3) };
}
```

... produces an error under `-frounding-math` but works without 
`-frounding-math` with recent trunk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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


[clang] 3b8d895 - SourceManager: Remove a redundant nullptr check in getNonBuiltinFilenameForID, NFC

2020-10-22 Thread Duncan P . N . Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2020-10-22T22:20:50-04:00
New Revision: 3b8d8954bf2c192502d757019b9fe434864068e9

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

LOG: SourceManager: Remove a redundant nullptr check in 
getNonBuiltinFilenameForID, NFC

Added: 


Modified: 
clang/lib/Basic/SourceManager.cpp

Removed: 




diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index 79eeeb29d6b9..e3b88f9127d5 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -729,7 +729,7 @@ Optional
 SourceManager::getNonBuiltinFilenameForID(FileID FID) const {
   if (const SrcMgr::SLocEntry *Entry = getSLocEntryForFile(FID))
 if (auto *Content = Entry->getFile().getContentCache())
-  if (Content && Content->OrigEntry)
+  if (Content->OrigEntry)
 return Entry->getFile().getName();
   return None;
 }



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


[PATCH] D89508: SourceManager: getFileEntryRefForID => getNonBuiltinFilenameForID, NFC

2020-10-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf593d224c9c: SourceManager: getFileEntryRefForID = 
getNonBuiltinFilenameForID, NFC (authored by dexonsmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89508

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -77,15 +77,10 @@
   // Dependency generation really does want to go all the way to the
   // file entry for a source location to find out what is depended on.
   // We do not want #line markers to affect dependency generation!
-  Optional File =
-  SM.getFileEntryRefForID(SM.getFileID(SM.getExpansionLoc(Loc)));
-  if (!File)
-return;
-
-  StringRef FileName =
-  llvm::sys::path::remove_leading_dotslash(File->getName());
-
-  MDC.MainDeps.push_back(std::string(FileName));
+  if (Optional Filename =
+  SM.getNonBuiltinFilenameForID(SM.getFileID(SM.getExpansionLoc(Loc
+MDC.MainDeps.push_back(
+std::string(llvm::sys::path::remove_leading_dotslash(*Filename)));
 }
 
 void ModuleDepCollectorPP::InclusionDirective(
Index: clang/lib/Frontend/DependencyFile.cpp
===
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -46,17 +46,12 @@
 // Dependency generation really does want to go all the way to the
 // file entry for a source location to find out what is depended on.
 // We do not want #line markers to affect dependency generation!
-Optional File =
-SM.getFileEntryRefForID(SM.getFileID(SM.getExpansionLoc(Loc)));
-if (!File)
-  return;
-
-StringRef Filename =
-llvm::sys::path::remove_leading_dotslash(File->getName());
-
-DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
-isSystem(FileType),
-/*IsModuleFile*/false, /*IsMissing*/false);
+if (Optional Filename = SM.getNonBuiltinFilenameForID(
+SM.getFileID(SM.getExpansionLoc(Loc
+  DepCollector.maybeAddDependency(
+  llvm::sys::path::remove_leading_dotslash(*Filename),
+  /*FromModule*/ false, isSystem(FileType), /*IsModuleFile*/ false,
+  /*IsMissing*/ false);
   }
 
   void FileSkipped(const FileEntryRef , const Token ,
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -725,11 +725,12 @@
   const_cast(CC)->IsTransient = true;
 }
 
-Optional SourceManager::getFileEntryRefForID(FileID FID) const {
+Optional
+SourceManager::getNonBuiltinFilenameForID(FileID FID) const {
   if (const SrcMgr::SLocEntry *Entry = getSLocEntryForFile(FID))
 if (auto *Content = Entry->getFile().getContentCache())
   if (Content && Content->OrigEntry)
-return FileEntryRef(Entry->getFile().getName(), *Content->OrigEntry);
+return Entry->getFile().getName();
   return None;
 }
 
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -997,8 +997,11 @@
 return nullptr;
   }
 
-  /// Returns the FileEntryRef for the provided FileID.
-  Optional getFileEntryRefForID(FileID FID) const;
+  /// Returns the filename for the provided FileID, unless it's a built-in
+  /// buffer that's not represented by a filename.
+  ///
+  /// Returns None for non-files and built-in files.
+  Optional getNonBuiltinFilenameForID(FileID FID) const;
 
   /// Returns the FileEntry record for the provided SLocEntry.
   const FileEntry *getFileEntryForSLocEntry(const SrcMgr::SLocEntry ) 
const


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -77,15 +77,10 @@
   // Dependency generation really does want to go all the way to the
   // file entry for a source location to find out what is depended on.
   // We do not want #line markers to affect dependency generation!
-  Optional File =
-  SM.getFileEntryRefForID(SM.getFileID(SM.getExpansionLoc(Loc)));
-  if (!File)
-return;
-
-  StringRef FileName =
-  

[clang] cf593d2 - SourceManager: getFileEntryRefForID => getNonBuiltinFilenameForID, NFC

2020-10-22 Thread Duncan P . N . Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2020-10-22T22:20:50-04:00
New Revision: cf593d224c9c3fc2cf93827cb66fd9e550a6a486

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

LOG: SourceManager: getFileEntryRefForID => getNonBuiltinFilenameForID, NFC

`SourceManager::getFileEntryRefForID`'s remaining callers just want the
filename component, which is coming from the `FileInfo`. Replace the API
with `getNonBuiltinFilenameForID`, which also removes another use of
`FileEntryRef::FileEntryRef` outside of `FileManager`.

Both callers are collecting file dependencies, and one of them relied on
this API to filter out built-ins (as exposed by
clang/test/ClangScanDeps/modules-full.cpp). It seems nice to continue
providing that service.

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

Added: 


Modified: 
clang/include/clang/Basic/SourceManager.h
clang/lib/Basic/SourceManager.cpp
clang/lib/Frontend/DependencyFile.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/SourceManager.h 
b/clang/include/clang/Basic/SourceManager.h
index 93eee503a8e5..5035326297f7 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -997,8 +997,11 @@ class SourceManager : public RefCountedBase 
{
 return nullptr;
   }
 
-  /// Returns the FileEntryRef for the provided FileID.
-  Optional getFileEntryRefForID(FileID FID) const;
+  /// Returns the filename for the provided FileID, unless it's a built-in
+  /// buffer that's not represented by a filename.
+  ///
+  /// Returns None for non-files and built-in files.
+  Optional getNonBuiltinFilenameForID(FileID FID) const;
 
   /// Returns the FileEntry record for the provided SLocEntry.
   const FileEntry *getFileEntryForSLocEntry(const SrcMgr::SLocEntry ) 
const

diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index c91ef41979bc..79eeeb29d6b9 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -725,11 +725,12 @@ void SourceManager::setFileIsTransient(const FileEntry 
*File) {
   const_cast(CC)->IsTransient = true;
 }
 
-Optional SourceManager::getFileEntryRefForID(FileID FID) const {
+Optional
+SourceManager::getNonBuiltinFilenameForID(FileID FID) const {
   if (const SrcMgr::SLocEntry *Entry = getSLocEntryForFile(FID))
 if (auto *Content = Entry->getFile().getContentCache())
   if (Content && Content->OrigEntry)
-return FileEntryRef(Entry->getFile().getName(), *Content->OrigEntry);
+return Entry->getFile().getName();
   return None;
 }
 

diff  --git a/clang/lib/Frontend/DependencyFile.cpp 
b/clang/lib/Frontend/DependencyFile.cpp
index c9240f4122a7..fe8ab7197400 100644
--- a/clang/lib/Frontend/DependencyFile.cpp
+++ b/clang/lib/Frontend/DependencyFile.cpp
@@ -46,17 +46,12 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
 // Dependency generation really does want to go all the way to the
 // file entry for a source location to find out what is depended on.
 // We do not want #line markers to affect dependency generation!
-Optional File =
-SM.getFileEntryRefForID(SM.getFileID(SM.getExpansionLoc(Loc)));
-if (!File)
-  return;
-
-StringRef Filename =
-llvm::sys::path::remove_leading_dotslash(File->getName());
-
-DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
-isSystem(FileType),
-/*IsModuleFile*/false, /*IsMissing*/false);
+if (Optional Filename = SM.getNonBuiltinFilenameForID(
+SM.getFileID(SM.getExpansionLoc(Loc
+  DepCollector.maybeAddDependency(
+  llvm::sys::path::remove_leading_dotslash(*Filename),
+  /*FromModule*/ false, isSystem(FileType), /*IsModuleFile*/ false,
+  /*IsMissing*/ false);
   }
 
   void FileSkipped(const FileEntryRef , const Token ,

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 4f6eff799f22..f74ce7304df5 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -77,15 +77,10 @@ void ModuleDepCollectorPP::FileChanged(SourceLocation Loc,
   // Dependency generation really does want to go all the way to the
   // file entry for a source location to find out what is depended on.
   // We do not want #line markers to affect dependency generation!
-  Optional File =
-  SM.getFileEntryRefForID(SM.getFileID(SM.getExpansionLoc(Loc)));
-  if (!File)
-return;
-
-  StringRef FileName =
-  

[PATCH] D89507: SourceManager: Change SourceManager::isMainFile to take a FileEntry, NFC

2020-10-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG168db92465c5: SourceManager: Change 
SourceManager::isMainFile to take a FileEntry, NFC (authored by dexonsmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89507

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/unittests/Basic/SourceManagerTest.cpp


Index: clang/unittests/Basic/SourceManagerTest.cpp
===
--- clang/unittests/Basic/SourceManagerTest.cpp
+++ clang/unittests/Basic/SourceManagerTest.cpp
@@ -509,10 +509,9 @@
   FileID MainFileID = SourceMgr.getOrCreateFileID(SourceFile, SrcMgr::C_User);
   SourceMgr.setMainFileID(MainFileID);
 
-  EXPECT_TRUE(SourceMgr.isMainFile(FileEntryRef("mainFile.cpp", *SourceFile)));
-  EXPECT_TRUE(
-  SourceMgr.isMainFile(FileEntryRef("anotherName.cpp", *SourceFile)));
-  EXPECT_FALSE(SourceMgr.isMainFile(FileEntryRef("mainFile.cpp", 
*SecondFile)));
+  EXPECT_TRUE(SourceMgr.isMainFile(*SourceFile));
+  EXPECT_TRUE(SourceMgr.isMainFile(*SourceFile));
+  EXPECT_FALSE(SourceMgr.isMainFile(*SecondFile));
 }
 
 #endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2061,7 +2061,7 @@
   // some directives (e.g. #endif of a header guard) will never be seen.
   // Since this will lead to confusing errors, avoid the inclusion.
   if (Action == Enter && File && PreambleConditionalStack.isRecording() &&
-  SourceMgr.isMainFile(*File)) {
+  SourceMgr.isMainFile(File->getFileEntry())) {
 Diag(FilenameTok.getLocation(),
  diag::err_pp_including_mainfile_in_preamble);
 return {ImportAction::None};
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -345,12 +345,11 @@
   createExpansionLoc(SourceLocation(), SourceLocation(), SourceLocation(), 1);
 }
 
-bool SourceManager::isMainFile(FileEntryRef SourceFile) {
+bool SourceManager::isMainFile(const FileEntry ) {
   assert(MainFileID.isValid() && "expected initialized SourceManager");
-  auto FE = getFileEntryRefForID(MainFileID);
-  if (!FE)
-return false;
-  return FE->getUID() == SourceFile.getUID();
+  if (auto *FE = getFileEntryForID(MainFileID))
+return FE->getUID() == SourceFile.getUID();
+  return false;
 }
 
 void SourceManager::initializeForReplay(const SourceManager ) {
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -814,7 +814,7 @@
   /// Returns true when the given FileEntry corresponds to the main file.
   ///
   /// The main file should be set prior to calling this function.
-  bool isMainFile(FileEntryRef SourceFile);
+  bool isMainFile(const FileEntry );
 
   /// Set the file ID for the precompiled preamble.
   void setPreambleFileID(FileID Preamble) {


Index: clang/unittests/Basic/SourceManagerTest.cpp
===
--- clang/unittests/Basic/SourceManagerTest.cpp
+++ clang/unittests/Basic/SourceManagerTest.cpp
@@ -509,10 +509,9 @@
   FileID MainFileID = SourceMgr.getOrCreateFileID(SourceFile, SrcMgr::C_User);
   SourceMgr.setMainFileID(MainFileID);
 
-  EXPECT_TRUE(SourceMgr.isMainFile(FileEntryRef("mainFile.cpp", *SourceFile)));
-  EXPECT_TRUE(
-  SourceMgr.isMainFile(FileEntryRef("anotherName.cpp", *SourceFile)));
-  EXPECT_FALSE(SourceMgr.isMainFile(FileEntryRef("mainFile.cpp", *SecondFile)));
+  EXPECT_TRUE(SourceMgr.isMainFile(*SourceFile));
+  EXPECT_TRUE(SourceMgr.isMainFile(*SourceFile));
+  EXPECT_FALSE(SourceMgr.isMainFile(*SecondFile));
 }
 
 #endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2061,7 +2061,7 @@
   // some directives (e.g. #endif of a header guard) will never be seen.
   // Since this will lead to confusing errors, avoid the inclusion.
   if (Action == Enter && File && PreambleConditionalStack.isRecording() &&
-  SourceMgr.isMainFile(*File)) {
+  SourceMgr.isMainFile(File->getFileEntry())) {
 Diag(FilenameTok.getLocation(),
  diag::err_pp_including_mainfile_in_preamble);
 return {ImportAction::None};
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -345,12 +345,11 @@
   createExpansionLoc(SourceLocation(), SourceLocation(), 

[clang] 168db92 - SourceManager: Change SourceManager::isMainFile to take a FileEntry, NFC

2020-10-22 Thread Duncan P . N . Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2020-10-22T21:32:28-04:00
New Revision: 168db92465c504974274302a6a1a5d4a1580ccfe

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

LOG: SourceManager: Change SourceManager::isMainFile to take a FileEntry, NFC

`SourceManager::isMainFile` does not use the filename, so it doesn't
need the full `FileEntryRef`; in fact, it's misleading to take the name
because that makes it look relevant. Simplify the API, and in the
process remove some calls to `FileEntryRef::FileEntryRef` in the unit
tests (which were blocking making that private to `SourceManager`).

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

Added: 


Modified: 
clang/include/clang/Basic/SourceManager.h
clang/lib/Basic/SourceManager.cpp
clang/lib/Lex/PPDirectives.cpp
clang/unittests/Basic/SourceManagerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/SourceManager.h 
b/clang/include/clang/Basic/SourceManager.h
index 57f8e9d365ad..93eee503a8e5 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -814,7 +814,7 @@ class SourceManager : public RefCountedBase {
   /// Returns true when the given FileEntry corresponds to the main file.
   ///
   /// The main file should be set prior to calling this function.
-  bool isMainFile(FileEntryRef SourceFile);
+  bool isMainFile(const FileEntry );
 
   /// Set the file ID for the precompiled preamble.
   void setPreambleFileID(FileID Preamble) {

diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index a3e4e02cbb5f..c91ef41979bc 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -345,12 +345,11 @@ void SourceManager::clearIDTables() {
   createExpansionLoc(SourceLocation(), SourceLocation(), SourceLocation(), 1);
 }
 
-bool SourceManager::isMainFile(FileEntryRef SourceFile) {
+bool SourceManager::isMainFile(const FileEntry ) {
   assert(MainFileID.isValid() && "expected initialized SourceManager");
-  auto FE = getFileEntryRefForID(MainFileID);
-  if (!FE)
-return false;
-  return FE->getUID() == SourceFile.getUID();
+  if (auto *FE = getFileEntryForID(MainFileID))
+return FE->getUID() == SourceFile.getUID();
+  return false;
 }
 
 void SourceManager::initializeForReplay(const SourceManager ) {

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 62724d467eac..d6b03d85913d 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2061,7 +2061,7 @@ Preprocessor::ImportAction 
Preprocessor::HandleHeaderIncludeOrImport(
   // some directives (e.g. #endif of a header guard) will never be seen.
   // Since this will lead to confusing errors, avoid the inclusion.
   if (Action == Enter && File && PreambleConditionalStack.isRecording() &&
-  SourceMgr.isMainFile(*File)) {
+  SourceMgr.isMainFile(File->getFileEntry())) {
 Diag(FilenameTok.getLocation(),
  diag::err_pp_including_mainfile_in_preamble);
 return {ImportAction::None};

diff  --git a/clang/unittests/Basic/SourceManagerTest.cpp 
b/clang/unittests/Basic/SourceManagerTest.cpp
index 5fd7607f76d7..dadfcc94ac0d 100644
--- a/clang/unittests/Basic/SourceManagerTest.cpp
+++ b/clang/unittests/Basic/SourceManagerTest.cpp
@@ -509,10 +509,9 @@ TEST_F(SourceManagerTest, isMainFile) {
   FileID MainFileID = SourceMgr.getOrCreateFileID(SourceFile, SrcMgr::C_User);
   SourceMgr.setMainFileID(MainFileID);
 
-  EXPECT_TRUE(SourceMgr.isMainFile(FileEntryRef("mainFile.cpp", *SourceFile)));
-  EXPECT_TRUE(
-  SourceMgr.isMainFile(FileEntryRef("anotherName.cpp", *SourceFile)));
-  EXPECT_FALSE(SourceMgr.isMainFile(FileEntryRef("mainFile.cpp", 
*SecondFile)));
+  EXPECT_TRUE(SourceMgr.isMainFile(*SourceFile));
+  EXPECT_TRUE(SourceMgr.isMainFile(*SourceFile));
+  EXPECT_FALSE(SourceMgr.isMainFile(*SecondFile));
 }
 
 #endif



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


[clang] b6c6daa - SourceManager: Factor out helpers for common SLocEntry lookup pattern, NFC

2020-10-22 Thread Duncan P . N . Exon Smith via cfe-commits

Author: Duncan P. N. Exon Smith
Date: 2020-10-22T21:30:31-04:00
New Revision: b6c6daa95d3aa2206d5a42b46793226f181c3e44

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

LOG: SourceManager: Factor out helpers for common SLocEntry lookup pattern, NFC

Add helpers `getSLocEntryOrNull`, which handles the `Invalid` logic
around `getSLocEntry`, and `getSLocEntryForFile`, which also checks for
`SLocEntry::isFile`, and use them to reduce repeated code.

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

Added: 


Modified: 
clang/include/clang/Basic/SourceManager.h
clang/lib/Basic/SourceManager.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/SourceManager.h 
b/clang/include/clang/Basic/SourceManager.h
index 4b84ae132c3e..57f8e9d365ad 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -972,13 +972,10 @@ class SourceManager : public 
RefCountedBase {
   /// If there is an error opening this buffer the first time, return None.
   llvm::Optional
   getBufferOrNone(FileID FID, SourceLocation Loc = SourceLocation()) const {
-bool MyInvalid = false;
-const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-if (MyInvalid || !Entry.isFile())
-  return None;
-
-return Entry.getFile().getContentCache()->getBufferOrNone(
-Diag, getFileManager(), Loc);
+if (auto *Entry = getSLocEntryForFile(FID))
+  return Entry->getFile().getContentCache()->getBufferOrNone(
+  Diag, getFileManager(), Loc);
+return None;
   }
 
   /// Return the buffer for the specified FileID.
@@ -994,15 +991,10 @@ class SourceManager : public 
RefCountedBase {
 
   /// Returns the FileEntry record for the provided FileID.
   const FileEntry *getFileEntryForID(FileID FID) const {
-bool MyInvalid = false;
-const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-if (MyInvalid || !Entry.isFile())
-  return nullptr;
-
-const SrcMgr::ContentCache *Content = Entry.getFile().getContentCache();
-if (!Content)
-  return nullptr;
-return Content->OrigEntry;
+if (auto *Entry = getSLocEntryForFile(FID))
+  if (auto *Content = Entry->getFile().getContentCache())
+return Content->OrigEntry;
+return nullptr;
   }
 
   /// Returns the FileEntryRef for the provided FileID.
@@ -1039,25 +1031,20 @@ class SourceManager : public 
RefCountedBase {
   /// Get the number of FileIDs (files and macros) that were created
   /// during preprocessing of \p FID, including it.
   unsigned getNumCreatedFIDsForFileID(FileID FID) const {
-bool Invalid = false;
-const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-if (Invalid || !Entry.isFile())
-  return 0;
-
-return Entry.getFile().NumCreatedFIDs;
+if (auto *Entry = getSLocEntryForFile(FID))
+  return Entry->getFile().NumCreatedFIDs;
+return 0;
   }
 
   /// Set the number of FileIDs (files and macros) that were created
   /// during preprocessing of \p FID, including it.
   void setNumCreatedFIDsForFileID(FileID FID, unsigned NumFIDs,
   bool Force = false) const {
-bool Invalid = false;
-const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-if (Invalid || !Entry.isFile())
+auto *Entry = getSLocEntryForFile(FID);
+if (!Entry)
   return;
-
-assert((Force || Entry.getFile().NumCreatedFIDs == 0) && "Already set!");
-const_cast(Entry.getFile()).NumCreatedFIDs = NumFIDs;
+assert((Force || Entry->getFile().NumCreatedFIDs == 0) && "Already set!");
+const_cast(Entry->getFile()).NumCreatedFIDs = NumFIDs;
   }
 
   
//======//
@@ -1086,36 +1073,26 @@ class SourceManager : public 
RefCountedBase {
   /// Return the source location corresponding to the first byte of
   /// the specified file.
   SourceLocation getLocForStartOfFile(FileID FID) const {
-bool Invalid = false;
-const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-if (Invalid || !Entry.isFile())
-  return SourceLocation();
-
-unsigned FileOffset = Entry.getOffset();
-return SourceLocation::getFileLoc(FileOffset);
+if (auto *Entry = getSLocEntryForFile(FID))
+  return SourceLocation::getFileLoc(Entry->getOffset());
+return SourceLocation();
   }
 
   /// Return the source location corresponding to the last byte of the
   /// specified file.
   SourceLocation getLocForEndOfFile(FileID FID) const {
-bool Invalid = false;
-const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-if (Invalid || !Entry.isFile())
-  return SourceLocation();
-
-unsigned FileOffset = Entry.getOffset();
-return SourceLocation::getFileLoc(FileOffset + getFileIDSize(FID));
+if (auto *Entry = getSLocEntryForFile(FID))
+

[PATCH] D89503: SourceManager: Factor out helpers for common SLocEntry lookup pattern, NFC

2020-10-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6c6daa95d3a: SourceManager: Factor out helpers for common 
SLocEntry lookup pattern, NFC (authored by dexonsmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89503

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp

Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -727,15 +727,11 @@
 }
 
 Optional SourceManager::getFileEntryRefForID(FileID FID) const {
-  bool Invalid = false;
-  const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-  if (Invalid || !Entry.isFile())
-return None;
-
-  const SrcMgr::ContentCache *Content = Entry.getFile().getContentCache();
-  if (!Content || !Content->OrigEntry)
-return None;
-  return FileEntryRef(Entry.getFile().getName(), *Content->OrigEntry);
+  if (const SrcMgr::SLocEntry *Entry = getSLocEntryForFile(FID))
+if (auto *Content = Entry->getFile().getContentCache())
+  if (Content && Content->OrigEntry)
+return FileEntryRef(Entry->getFile().getName(), *Content->OrigEntry);
+  return None;
 }
 
 StringRef SourceManager::getBufferData(FileID FID, bool *Invalid) const {
@@ -747,23 +743,16 @@
 
 llvm::Optional
 SourceManager::getBufferDataIfLoaded(FileID FID) const {
-  bool MyInvalid = false;
-  const SLocEntry  = getSLocEntry(FID, );
-  if (!SLoc.isFile() || MyInvalid)
-return None;
-
-  return SLoc.getFile().getContentCache()->getBufferDataIfLoaded();
+  if (const SrcMgr::SLocEntry *Entry = getSLocEntryForFile(FID))
+return Entry->getFile().getContentCache()->getBufferDataIfLoaded();
+  return None;
 }
 
 llvm::Optional SourceManager::getBufferDataOrNone(FileID FID) const {
-  bool MyInvalid = false;
-  const SLocEntry  = getSLocEntry(FID, );
-  if (!SLoc.isFile() || MyInvalid)
-return None;
-
-  if (auto B = SLoc.getFile().getContentCache()->getBufferOrNone(
-  Diag, getFileManager(), SourceLocation()))
-return B->getBuffer();
+  if (const SrcMgr::SLocEntry *Entry = getSLocEntryForFile(FID))
+if (auto B = Entry->getFile().getContentCache()->getBufferOrNone(
+Diag, getFileManager(), SourceLocation()))
+  return B->getBuffer();
   return None;
 }
 
@@ -1443,12 +1432,11 @@
 SourceManager::getFileCharacteristic(SourceLocation Loc) const {
   assert(Loc.isValid() && "Can't get file characteristic of invalid loc!");
   std::pair LocInfo = getDecomposedExpansionLoc(Loc);
-  bool Invalid = false;
-  const SLocEntry  = getSLocEntry(LocInfo.first, );
-  if (Invalid || !SEntry.isFile())
+  const SLocEntry *SEntry = getSLocEntryForFile(LocInfo.first);
+  if (!SEntry)
 return C_User;
 
-  const SrcMgr::FileInfo  = SEntry.getFile();
+  const SrcMgr::FileInfo  = SEntry->getFile();
 
   // If there are no #line directives in this file, just return the whole-file
   // state.
@@ -1569,12 +1557,11 @@
   // Presumed locations are always for expansion points.
   std::pair LocInfo = getDecomposedExpansionLoc(Loc);
 
-  bool Invalid = false;
-  const SLocEntry  = getSLocEntry(LocInfo.first, );
-  if (Invalid || !Entry.isFile())
+  const SLocEntry *Entry = getSLocEntryForFile(LocInfo.first);
+  if (!Entry)
 return false;
 
-  const SrcMgr::FileInfo  = Entry.getFile();
+  const SrcMgr::FileInfo  = Entry->getFile();
 
   // Check if there is a line directive for this location.
   if (FI.hasLineDirectives())
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -972,13 +972,10 @@
   /// If there is an error opening this buffer the first time, return None.
   llvm::Optional
   getBufferOrNone(FileID FID, SourceLocation Loc = SourceLocation()) const {
-bool MyInvalid = false;
-const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-if (MyInvalid || !Entry.isFile())
-  return None;
-
-return Entry.getFile().getContentCache()->getBufferOrNone(
-Diag, getFileManager(), Loc);
+if (auto *Entry = getSLocEntryForFile(FID))
+  return Entry->getFile().getContentCache()->getBufferOrNone(
+  Diag, getFileManager(), Loc);
+return None;
   }
 
   /// Return the buffer for the specified FileID.
@@ -994,15 +991,10 @@
 
   /// Returns the FileEntry record for the provided FileID.
   const FileEntry *getFileEntryForID(FileID FID) const {
-bool MyInvalid = false;
-const SrcMgr::SLocEntry  = getSLocEntry(FID, );
-if (MyInvalid || !Entry.isFile())
-  return nullptr;
-
-const SrcMgr::ContentCache *Content = Entry.getFile().getContentCache();
-if (!Content)
-  return nullptr;
-return Content->OrigEntry;
+if (auto *Entry = 

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling support for non-type template parameters of class type and template parameter objects.

2020-10-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added a project: clang.
rsmith requested review of this revision.

This follows the approach I proposed in
https://github.com/itanium-cxx-abi/cxx-abi/issues/47 on 2020-09-06.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89998

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Linkage.h
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/template-param-objects.cpp

Index: clang/test/CodeGenCXX/template-param-objects.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/template-param-objects.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++20 %s -emit-llvm -o - | FileCheck %s
+
+struct S { char buf[32]; };
+template constexpr const char *begin() { return s.buf; }
+template constexpr const char *end() { return s.buf + __builtin_strlen(s.buf); }
+
+// CHECK: [[HELLO:@_ZTAXtl1StlA32_cLc104ELc101ELc108ELc108ELc111ELc32ELc119ELc111ELc114ELc108ELc100]] = linkonce_odr constant { <{ [11 x i8], [21 x i8] }> } { <{ [11 x i8], [21 x i8] }> <{ [11 x i8] c"hello world", [21 x i8] zeroinitializer }> }, comdat
+
+// CHECK: @p = global i8* getelementptr inbounds ({{.*}}* [[HELLO]], i32 0, i32 0, i32 0, i32 0)
+const char *p = begin();
+// CHECK: @q = global i8* getelementptr (i8, i8* getelementptr inbounds ({{.*}}* [[HELLO]], i32 0, i32 0, i32 0, i32 0), i64 11)
+const char *q = end();
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -0,0 +1,165 @@
+// RUN: %clang_cc1 -std=c++20 %s -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+struct A { int a, b; };
+template void f() {}
+
+// CHECK: define weak_odr void @_Z1fIXtl1ALi1ELi2vv(
+template void f();
+
+struct B { int *p; int k; };
+template void f() {}
+
+int n = 0;
+// CHECK: define weak_odr void @_Z1fIXtl1BadL_Z1nvv(
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1BLPi0ELi1vv(
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1BEEEvv(
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1BLPi32vv(
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1BrcPiLi0Evv(
+template void f(0))}>();
+
+// Pointers to subobjects.
+struct Nested { union { int k; int arr[2]; }; } nested[2];
+struct Derived : A, Nested {} derived;
+// CHECK: define weak_odr void @_Z1fIXtl1BadsoiL_Z6nestedE_vv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1BadsoiL_Z6nestedE16_0pvv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1BadsoiL_Z7derivedE8pvv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1BcvPiplcvPcadL_Z7derivedELl16vv
+template void f();
+
+// References to subobjects.
+struct BR { int  };
+template void f() {}
+// CHECK: define weak_odr void @_Z1fIXtl2BRsoiL_Z6nestedE_vv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl2BRsoiL_Z6nestedE12_0vv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl2BRsoiL_Z7derivedE4vv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl2BRdecvPiplcvPcadL_Z7derivedELl16vv
+template void f();
+
+// Qualification conversions.
+struct C { const int *p; };
+template void f() {}
+// CHECK: define weak_odr void @_Z1fIXtl1CadsoKiL_Z7derivedE4vv
+template void f();
+
+// Pointers to members.
+struct D { const int Derived::*p; int k; };
+template void f() {}
+// CHECK: define weak_odr void @_Z1fIXtl1DLM7DerivedKi0ELi1vv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1DEEEvv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN1A1aEEvv
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN1A1bEEvv
+template void f();
+// FIXME: Is the Ut_1 mangling here correct?
+// CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN6NestedUt_1kEE8vv
+template void f();
+struct MoreDerived : A, Derived { int z; };
+// CHECK: define weak_odr void @_Z1fIXtl1DmcM7DerivedKiadL_ZN11MoreDerived1zEEn8vv
+template void f();
+
+union E {
+  int n;
+  float f;
+  constexpr E() {}
+  constexpr E(int n) : n(n) {}
+  constexpr E(float f) : f(f) {}
+};
+template void f() {}
+
+// Union members.
+// CHECK: define weak_odr void @_Z1fIXL1vv(
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1vv(
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1Edi1nLi42vv(
+template void f();
+// CHECK: define weak_odr void @_Z1fIXtl1Edi1fLfvv(

[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:30-34
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop

Generally we strongly prefer for frontend tests to test only the frontend; in 
this case, that means testing only the IR that Clang is producing and not the 
assembly that comes out of LLVM for that IR. This should also remove the need 
to require PPC as a registered target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

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


[PATCH] D89994: [libomptarget][nvptx] Undef, internal shared variables

2020-10-22 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:4794
   CGM.getModule(), LLVMStaticTy,
-  /*isConstant=*/false, llvm::GlobalValue::CommonLinkage,
-  llvm::Constant::getNullValue(LLVMStaticTy),
+  /*isConstant=*/false, llvm::GlobalValue::InternalLinkage,
+  llvm::UndefValue::get(LLVMStaticTy),

Perhaps weak_any + undef?

Could use internal for symbols that may vary in size and weak_any for those 
that don't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89994

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


[PATCH] D89994: [libomptarget][nvptx] Undef, internal shared variables

2020-10-22 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

In D89994#2348656 , @ABataev wrote:

> In D89994#2348655 , @JonChesterfield 
> wrote:
>
>> The nvptx back end accepts common + zero + shared, but not common + undef + 
>> shared. I think weak_odr is conceptually right here, but given the warning 
>> that nvlink doesn't support weak symbols, internal also seems fine. Can 
>> someone see an advantage to weak over internal? It could be arch specific at 
>> the risk of a lot of test duplication.
>
> IIRC, it supports weak symbols, but does not support weak symbols of 
> different sizes.

That seems a reasonable restriction. Linkers sometimes pick the first weak 
symbol they see. Comdat might mean pick the biggest one, but that's probably 
not a good thing to rely on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89994

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


[clang] af189c8 - Fix constant evaluation of zero-initialization of a union whose first

2020-10-22 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-10-22T17:03:59-07:00
New Revision: af189c8ab113668434eae5c71f174613a9e64625

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

LOG: Fix constant evaluation of zero-initialization of a union whose first
FieldDecl is an unamed bitfield.

Unnamed bitfields aren't non-static data member, so such a bitfield
isn't actually the first non-static data member.

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e978a190a195..c6fc02a6fe91 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -9630,6 +9630,8 @@ bool RecordExprEvaluator::ZeroInitialization(const Expr 
*E, QualType T) {
 // C++11 [dcl.init]p5: If T is a (possibly cv-qualified) union type, the
 // object's first non-static named data member is zero-initialized
 RecordDecl::field_iterator I = RD->field_begin();
+while (I != RD->field_end() && (*I)->isUnnamedBitfield())
+  ++I;
 if (I == RD->field_end()) {
   Result = APValue((const FieldDecl*)nullptr);
   return true;

diff  --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp 
b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 0d67b747a0f4..7d51a48eac42 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2021,6 +2021,14 @@ namespace Bitfields {
 const HasUnnamedBitfield oneZero{1, 0};
 int b = 1 / oneZero.b; // expected-warning {{division by zero is 
undefined}}
   }
+
+  union UnionWithUnnamedBitfield {
+int : 3;
+int n;
+  };
+  static_assert(UnionWithUnnamedBitfield().n == 0, "");
+  static_assert(UnionWithUnnamedBitfield{}.n == 0, "");
+  static_assert(UnionWithUnnamedBitfield{1}.n == 1, "");
 }
 
 namespace ZeroSizeTypes {



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


[PATCH] D89882: [clangd] Offset enum values by when marshalling

2020-10-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/remote/Index.proto:15
 // structures corresponding to their clangd::* counterparts.
+// NOTE: Enum values are offset by one to detect missing values.
 service SymbolIndex {

can we just switch to proto2 instead?

apart from reserving the zero value, proto3 also requires you to explicitly 
handle zero in your code (vs being able to set a default in the proto 
definition)

Offsetting enums by one isn't a big deal, but it can be in other cases 
(consider a number like num_refs where 0 is a meaningful value and offsetting 
by 1 is really confusing)



Comment at: 
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:115
   Req.IDs = std::move(*IDs);
-  Req.Filter = static_cast(Message->filter());
+  Req.Filter = static_cast(Message->filter() - 1);
   if (Message->limit())

this -1 and +1 everywhere makes me a bit leery.
Seems a bit easy to reverse or forget it.

What about

```
template  std::underlying_type_t toProtobuf(T Value) {
  return static_cast>(Value) + 1;
}
```

and the inverse?



Comment at: 
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:277
   }
-  Result.set_flags(static_cast(From.Flags));
+  Result.set_flags(static_cast(From.Flags) + 1);
   return Result;

?!

this is a bitfield


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89882

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


[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

As a follow up, I probably want to make `-fno-stack-protector` sprinkle this 
new fn attr `nossp` all over everything; that and this should fix our LTO bug 
with no changes to the source, otherwise we can add 
`__attribute__((no_stack_protector))` to the source (though GCC only landed 
support for that TODAY, so portability would be an issue there)..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

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


[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

In D87956#2348446 , @void wrote:

> Why not just use "expected-error" and the like?

Ah, brilliant! Thank you!  Will wait to submit until tomorrow morning.




Comment at: llvm/docs/BitCodeFormat.rst:1072
+* code 69: ``byref``
+* code 70: ``mustprogress``
+* code 71: ``nossp``

void wrote:
> I'm tempted to say that these two adds should be a separate commit, but it's 
> a nit and I'll leave it up to you.
Done in rG4abaf0ec0a3c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

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


[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 300124.
nickdesaulniers marked 3 inline comments as done.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

- rebase, add alwaysinline tests, use expected-remark, improve remarks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/stack-protector.c
  clang/test/Frontend/optimization-remark-missed-inline-stack-protectors.c
  llvm/bindings/go/llvm/ir_test.go
  llvm/bindings/ocaml/llvm/llvm.ml
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/StackProtector.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/CodeGen/X86/stack-protector-2.ll
  llvm/test/Transforms/CodeExtractor/PartialInlineAttributes.ll
  llvm/test/Transforms/Inline/inline_nossp.ll
  llvm/test/Transforms/Inline/inline_ssp.ll
  llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
  llvm/utils/emacs/llvm-mode.el
  llvm/utils/kate/llvm.xml
  llvm/utils/llvm.grm
  llvm/utils/vim/syntax/llvm.vim
  llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Index: llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
===
--- llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
+++ llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
@@ -245,6 +245,7 @@
 \\bspir_func\\b|\
 \\bspir_kernel\\b|\
 \\bsret\\b|\
+\\bnossp\\b|\
 \\bssp\\b|\
 \\bsspreq\\b|\
 \\bsspstrong\\b|\
Index: llvm/utils/vim/syntax/llvm.vim
===
--- llvm/utils/vim/syntax/llvm.vim
+++ llvm/utils/vim/syntax/llvm.vim
@@ -148,6 +148,7 @@
   \ spir_func
   \ spir_kernel
   \ sret
+  \ nossp
   \ ssp
   \ sspreq
   \ sspstrong
Index: llvm/utils/llvm.grm
===
--- llvm/utils/llvm.grm
+++ llvm/utils/llvm.grm
@@ -168,6 +168,7 @@
  | noinline
  | alwaysinline
  | optsize
+ | nossp
  | ssp
  | sspreq
  | returns_twice
Index: llvm/utils/kate/llvm.xml
===
--- llvm/utils/kate/llvm.xml
+++ llvm/utils/kate/llvm.xml
@@ -93,6 +93,7 @@
optsize 
readnone 
readonly 
+   nossp 
ssp 
sspreq 
sspstrong 
Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -26,7 +26,7 @@
  "inaccessiblemem_or_argmemonly" "inlinehint" "jumptable" "minsize" "mustprogress" "naked" "nobuiltin"
  "noduplicate" "nofree" "noimplicitfloat" "noinline" "nonlazybind" "noredzone" "noreturn"
  "norecurse" "noundef" "nounwind" "optnone" "optsize" "readnone" "readonly" "returns_twice"
- "speculatable" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
+ "speculatable" "nossp" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "uwtable" "willreturn" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
'("%[-a-zA-Z$._][-a-zA-Z$._0-9]*" . font-lock-variable-name-face)
Index: llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
===
--- /dev/null
+++ llvm/test/Verifier/function-attribute-nossp-ssp-sspreq-sspstrong.ll
@@ -0,0 +1,58 @@
+; RUN: not opt -verify -o - %s 2>&1 | FileCheck %s
+define void @test_1 () #1 { ret void }
+define void @test_2 () #2 { ret void }
+define void @test_3 () #3 { ret void }
+define void @test_4 () #4 { ret void }
+define void @test_5 () #5 { ret void }
+define void @test_6 () #6 { ret void }
+define void @test_7 () #7 { ret void }
+define void @test_8 () #8 { ret void }
+define void @test_9 () #9 { ret void }
+define void @test_10 () #10 { ret void }
+define void @test_11 () #10 { ret void }
+define void @test_12 () #10 { ret void }
+define void @test_13 () #10 { ret void }
+define void @test_14 () #10 { ret void }
+
+attributes #0 = { nossp }
+attributes #1 = { ssp }
+attributes #2 = { sspreq }
+attributes #3 = { sspstrong }
+
+attributes #4 = { nossp ssp }
+attributes #5 = { nossp sspreq }
+attributes #6 = { nossp sspstrong }
+

[PATCH] D89072: [CodeView] Emit static data members as S_CONSTANTs.

2020-10-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Looks good with a test for explicit zero initialization.




Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:3157
+
+if (Value.isNullValue())
+  continue;

What about zero constants like:
  struct Foo {
static const int sdm = 0;
  };

I think isNullValue just compares against zero, so that's not quite right. 
Maybe structure it as `if ConstantInt / else if ConstantFP / else continue`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89072

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


[PATCH] D89994: [libomptarget][nvptx] Undef, internal shared variables

2020-10-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D89994#2348655 , @JonChesterfield 
wrote:

> The nvptx back end accepts common + zero + shared, but not common + undef + 
> shared. I think weak_odr is conceptually right here, but given the warning 
> that nvlink doesn't support weak symbols, internal also seems fine. Can 
> someone see an advantage to weak over internal? It could be arch specific at 
> the risk of a lot of test duplication.

IIRC, it supports weak symbols, but does not support weak symbols of different 
sizes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89994

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


[PATCH] D89994: [libomptarget][nvptx] Undef, internal shared variables

2020-10-22 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

The nvptx back end accepts common + zero + shared, but not common + undef + 
shared. I think weak_odr is conceptually right here, but given the warning that 
nvlink doesn't support weak symbols, internal also seems fine. Can someone see 
an advantage to weak over internal? It could be arch specific at the risk of a 
lot of test duplication.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89994

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


[PATCH] D89994: [libomptarget][nvptx] Undef, internal shared variables

2020-10-22 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield created this revision.
JonChesterfield added reviewers: jdoerfert, ABataev, grokos, tianshilei1992, 
ye-luo.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
JonChesterfield requested review of this revision.

[libomptarget][nvptx] Undef, internal shared variables

Shared variables on nvptx, and LDS on amdgcn, are uninitialized at
the start of kernel execution. Therefore create the variables with
undef instead of zeros, motivated in part by the amdgcn back end
rejecting LDS+initializer.

Common is zero initialized, which seems incompatible with shared. Thus
change them to internal, following the direction of
https://reviews.llvm.org/rG7b3eabdcd215

WIP, other tests need to be updated if direction is good


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89994

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp

Index: clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
===
--- clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
+++ clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
@@ -17,14 +17,14 @@
 // CHECK-DAG: [[TEAMS_REDUCE_UNION_TY:%.+]] = type { [[TEAM1_REDUCE_TY]] }
 // SEQ-DAG: [[MAP_TY:%.+]] = type { [128 x i8] }
 
-// SEQ-DAG: [[KERNEL_PTR:@.+]] = internal addrspace(3) global i8* null
+// SEQ-DAG: [[KERNEL_PTR:@.+]] = internal addrspace(3) global i8* undef
 // SEQ-DAG: [[KERNEL_SHARED1:@.+]] = internal unnamed_addr constant i16 1
 // SEQ-DAG: [[KERNEL_SHARED2:@.+]] = internal unnamed_addr constant i16 1
 // SEQ-DAG: [[KERNEL_SIZE1:@.+]] = internal unnamed_addr constant i{{64|32}} {{16|8}}
 // SEQ-DAG: [[KERNEL_SIZE2:@.+]] = internal unnamed_addr constant i{{64|32}} 16
 
 // Check for the data transfer medium in shared memory to transfer the reduction list to the first warp.
-// CHECK-DAG: [[TRANSFER_STORAGE:@.+]] = common addrspace([[SHARED_ADDRSPACE:[0-9]+]]) global [32 x i32]
+// CHECK-DAG: [[TRANSFER_STORAGE:@.+]] = internal addrspace([[SHARED_ADDRSPACE:[0-9]+]]) global [32 x i32]
 
 // Check that the execution mode of 2 target regions is set to Non-SPMD and the 3rd is in SPMD.
 // CHECK-DAG: {{@__omp_offloading_.+l44}}_exec_mode = weak constant i8 1
@@ -220,7 +220,7 @@
   // CHECK: [[BASE_ELT:%.+]] = bitcast i8* [[ELT_VOID]] to i32*
   // CHECK: [[ELT:%.+]] = getelementptr i32, i32* [[BASE_ELT]], i32 [[CNT]]
   //
-  // CHECK: [[MEDIUM_ELT:%.+]] = getelementptr inbounds [32 x i32], [32 x i32] addrspace([[SHARED_ADDRSPACE]])* [[TRANSFER_STORAGE]], i64 0, i32 [[WARPID]]
+  // CHECK: [[MEDIUM_ELT:%.+]] = getelementptr inbounds [32 x i32], [32 x i32] addrspace([[SHARED_ADDRSPACE]])* [[TRANSFER_STORAGE:@.+]], i64 0, i32 [[WARPID]]
   // CHECK: [[ELT_VAL:%.+]] = load i32, i32* [[ELT]],
   // CHECK: store volatile i32 [[ELT_VAL]], i32 addrspace([[SHARED_ADDRSPACE]])* [[MEDIUM_ELT]],
   // CHECK: br label {{%?}}[[COPY_CONT:.+]]
@@ -238,7 +238,7 @@
   // CHECK: br i1 [[IS_W0_ACTIVE_THREAD]], label {{%?}}[[DO_READ:.+]], label {{%?}}[[READ_ELSE:.+]]
   //
   // CHECK: [[DO_READ]]
-  // CHECK: [[MEDIUM_ELT:%.+]] = getelementptr inbounds [32 x i32], [32 x i32] addrspace([[SHARED_ADDRSPACE]])* [[TRANSFER_STORAGE]], i64 0, i32 [[TID]]
+  // CHECK: [[MEDIUM_ELT:%.+]] = getelementptr inbounds [32 x i32], [32 x i32] addrspace([[SHARED_ADDRSPACE]])* [[TRANSFER_STORAGE:@.+]], i64 0, i32 [[TID]]
   // CHECK: [[ELT_REF:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[RED_LIST:%.+]], i{{32|64}} 0, i{{32|64}} 0
   // CHECK: [[ELT_VOID:%.+]] = load i8*, i8** [[ELT_REF]],
   // CHECK: [[ELT_BASE:%.+]] = bitcast i8* [[ELT_VOID]] to i32*
@@ -531,7 +531,7 @@
   // CHECK: [[ELT_REF:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[RED_LIST]], i{{32|64}} 0, i{{32|64}} 0
   // CHECK: [[ELT_VOID:%.+]] = load i8*, i8** [[ELT_REF]],
   //
-  // CHECK: [[MEDIUM_ELT64:%.+]] = getelementptr inbounds [32 x i32], [32 x i32] addrspace([[SHARED_ADDRSPACE]])* [[TRANSFER_STORAGE]], i64 0, i32 [[WARPID]]
+  // CHECK: [[MEDIUM_ELT64:%.+]] = getelementptr inbounds [32 x i32], [32 x i32] addrspace([[SHARED_ADDRSPACE]])* [[TRANSFER_STORAGE:@.+]], i64 0, i32 [[WARPID]]
   // CHECK: [[MEDIUM_ELT:%.+]] = bitcast i32 addrspace([[SHARED_ADDRSPACE]])* [[MEDIUM_ELT64]] to i8 addrspace([[SHARED_ADDRSPACE]])*
   // CHECK: [[ELT_VAL:%.+]] = load i8, i8* [[ELT_VOID]], align
   // CHECK: store volatile i8 [[ELT_VAL]], i8 addrspace([[SHARED_ADDRSPACE]])* [[MEDIUM_ELT]], align
@@ -550,7 +550,7 @@
   // CHECK: br i1 [[IS_W0_ACTIVE_THREAD]], label {{%?}}[[DO_READ:.+]], label {{%?}}[[READ_ELSE:.+]]
   //
   // CHECK: [[DO_READ]]
-  // CHECK: [[MEDIUM_ELT32:%.+]] = getelementptr inbounds [32 x i32], [32 x i32] addrspace([[SHARED_ADDRSPACE]])* [[TRANSFER_STORAGE]], i64 0, i32 [[TID]]
+  // CHECK: [[MEDIUM_ELT32:%.+]] = getelementptr inbounds [32 x i32], [32 x i32] addrspace([[SHARED_ADDRSPACE]])* [[TRANSFER_STORAGE:@.+]], i64 0, i32 [[TID]]
   // CHECK: [[MEDIUM_ELT:%.+]] = 

[PATCH] D83088: Introduce CfgTraits abstraction

2020-10-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment.

In D83088#2347111 , @arsenm wrote:

> In D83088#2346322 , @mehdi_amini 
> wrote:
>
>> In D83088#2345540 , @nhaehnle wrote:
>>
>>> David, I don't think this is appropriate here. Let's take the discussion to 
>>> llvm-dev.
>>
>> Seems like David asked to revert in the meantime?
>
> -1 to reverting, which will just make the history messier with no tangible 
> benefit

This is the usual LLVM policy I believe: someone expressed a concern and ask to 
revert. We revert and discuss first.
So again: please revert.

The messier history is not an argument: we revert so many times for any random 
bot failures already, and our contribution guidelines still tell people to push 
a "fake commit" with a whitespace change to test their access.

I also see tangile benefits:

- we don't start building dependencies on newly introduced API making a revert 
more difficult later.
- the burden of convincing of the approach is on the patch author, reverting is 
forcing the discussion here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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


[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-22 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:20
+// RUN: FileCheck -check-prefixes=COMMON-IR,VISIBILITY-IR %s
+
+int x = 66;

There's no testing for the interaction with `-mignore-xcoff-visibility`.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:29
+}
+
+// COMMON-ASM: mflr 0

The general issue is that the `nop` instruction is being omitted even when the 
visibility information is not generated into the object file. This is also a 
problem when the visibility is applied in the source code:
```
int x = 66;
#pragma GCC visibility push(hidden)
__attribute__((__noinline__)) inline void f() {
  x = 55;
}
#pragma GCC visibility pop
int bar() {
  f();
  return x;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

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


[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

2020-10-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.
Herald added a subscriber: dexonsmith.

Thanks, I like the new direction, hopefully others agree. Sorry for providing 
slow feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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


[PATCH] D89910: [AIX] Let alloca return 16 bytes alignment

2020-10-22 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks!


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

https://reviews.llvm.org/D89910

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


[PATCH] D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file

2020-10-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision.
alexfh added a comment.

In D89886#2347376 , @DmitryPolukhin 
wrote:

> In D89886#2346851 , @alexfh wrote:
>
>> Ah, btw, any chance of adding a test for this?
>
> Oh, I was not able to create small reproducer that without including large 
> Apple Frameworks with modules :( My hypothesis that it is side effect of 
> module cache that triggers module load before it is referenced from sources. 
> I tested it on reproducer from PR47839 + my real internal example.

I see. Let's keep it as is then.

Looks good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89886

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


[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

2020-10-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 300107.
sammccall added a comment.

Fix nullptr condition (probably dead, but...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchFinder.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/AST/ASTTypeTraitsTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1875,6 +1875,26 @@
  nestedNameSpecifier()));
 }
 
+TEST_P(ASTMatchersTest, Attr) {
+  if (GetParam().isCXX11OrLater()) {
+EXPECT_TRUE(matches("struct [[clang::warn_unused_result]] F{};",
+attr(hasAttrName("warn_unused_result";
+EXPECT_TRUE(notMatches("struct [[clang::warn_unused_result]] F{};",
+   attr(hasAttrName("clang::warn_unused_result";
+  }
+  if (GetParam().isCXX17OrLater()) {
+EXPECT_TRUE(
+matches("struct [[nodiscard]] F{};", attr(hasAttrName("nodiscard";
+EXPECT_TRUE(notMatches("struct [[nodiscard]] F{};",
+   attr(hasAttrName("warn_unused_result";
+  }
+  EXPECT_TRUE(matches("int x(int * __attribute__((nonnull)) );",
+  attr(hasAttrName("nonnull";
+  // On windows, some nodes an implicit visibility attribute.
+  EXPECT_TRUE(
+  notMatches("struct F{}; int x(int *);", attr(unless(isImplicit();
+}
+
 TEST_P(ASTMatchersTest, NullStmt) {
   EXPECT_TRUE(matches("void f() {int i;;}", nullStmt()));
   EXPECT_TRUE(notMatches("void f() {int i;}", nullStmt()));
Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -120,6 +120,7 @@
   VERIFY_NAME(CallExpr);
   VERIFY_NAME(Type);
   VERIFY_NAME(ConstantArrayType);
+  VERIFY_NAME(NonNullAttr);
 #undef VERIFY_NAME
 }
 
@@ -148,6 +149,13 @@
  nestedNameSpecifierLoc()));
 }
 
+TEST(DynTypedNode, AttrSourceRange) {
+  RangeVerifier Verifier;
+  Verifier.expectRange(1, 31, 1, 31);
+  EXPECT_TRUE(Verifier.match("void x(char *y __attribute__((nonnull)) );",
+ ast_matchers::attr()));
+}
+
 TEST(DynTypedNode, DeclDump) {
   DumpVerifier Verifier;
   Verifier.expectSubstring("FunctionDecl");
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -141,6 +141,7 @@
   REGISTER_MATCHER(asmStmt);
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
+  REGISTER_MATCHER(attr);
   REGISTER_MATCHER(autoType);
   REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -544,6 +544,15 @@
 
 } // namespace
 
+bool HasNameMatcher::matchesNode(const NamedDecl , StringRef Name) {
+  if (Name.contains("::")) // slow path
+return HasNameMatcher({Name.str()}).matchesNode(Node);
+  // Fast path.
+  llvm::SmallString<128> Scratch;
+  StringRef NodeName = getNodeName(Node, Scratch);
+  return consumeNameSuffix(Name, NodeName) && Name.empty();
+}
+
 bool HasNameMatcher::matchesNodeUnqualified(const NamedDecl ) const {
   assert(UseUnqualifiedMatch);
   llvm::SmallString<128> Scratch;
@@ -973,6 +982,7 @@
 const internal::VariadicAllOfMatcher nestedNameSpecifier;
 const internal::VariadicAllOfMatcher
 nestedNameSpecifierLoc;
+const internal::VariadicAllOfMatcher attr;
 const internal::VariadicDynCastAllOfMatcher
 cudaKernelCallExpr;
 const AstTypeMatcher builtinType;
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -131,6 +131,8 @@
 else if (const TemplateArgumentLoc *TALoc =
  DynNode.get())
   traverse(*TALoc);
+else if (const Attr *A = DynNode.get())
+  traverse(*A);
 // FIXME: Add other base types after adding tests.
 
 // It's OK to always overwrite the 

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

2020-10-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 300106.
sammccall marked 2 inline comments as done.
sammccall added a comment.

Address review comments.
Add hasAttrName() and make isImplicit() support Attr too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchFinder.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/AST/ASTTypeTraitsTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1875,6 +1875,26 @@
  nestedNameSpecifier()));
 }
 
+TEST_P(ASTMatchersTest, Attr) {
+  if (GetParam().isCXX11OrLater()) {
+EXPECT_TRUE(matches("struct [[clang::warn_unused_result]] F{};",
+attr(hasAttrName("warn_unused_result";
+EXPECT_TRUE(notMatches("struct [[clang::warn_unused_result]] F{};",
+   attr(hasAttrName("clang::warn_unused_result";
+  }
+  if (GetParam().isCXX17OrLater()) {
+EXPECT_TRUE(
+matches("struct [[nodiscard]] F{};", attr(hasAttrName("nodiscard";
+EXPECT_TRUE(notMatches("struct [[nodiscard]] F{};",
+   attr(hasAttrName("warn_unused_result";
+  }
+  EXPECT_TRUE(matches("int x(int * __attribute__((nonnull)) );",
+  attr(hasAttrName("nonnull";
+  // On windows, some nodes an implicit visibility attribute.
+  EXPECT_TRUE(
+  notMatches("struct F{}; int x(int *);", attr(unless(isImplicit();
+}
+
 TEST_P(ASTMatchersTest, NullStmt) {
   EXPECT_TRUE(matches("void f() {int i;;}", nullStmt()));
   EXPECT_TRUE(notMatches("void f() {int i;}", nullStmt()));
Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -120,6 +120,7 @@
   VERIFY_NAME(CallExpr);
   VERIFY_NAME(Type);
   VERIFY_NAME(ConstantArrayType);
+  VERIFY_NAME(NonNullAttr);
 #undef VERIFY_NAME
 }
 
@@ -148,6 +149,13 @@
  nestedNameSpecifierLoc()));
 }
 
+TEST(DynTypedNode, AttrSourceRange) {
+  RangeVerifier Verifier;
+  Verifier.expectRange(1, 31, 1, 31);
+  EXPECT_TRUE(Verifier.match("void x(char *y __attribute__((nonnull)) );",
+ ast_matchers::attr()));
+}
+
 TEST(DynTypedNode, DeclDump) {
   DumpVerifier Verifier;
   Verifier.expectSubstring("FunctionDecl");
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -141,6 +141,7 @@
   REGISTER_MATCHER(asmStmt);
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
+  REGISTER_MATCHER(attr);
   REGISTER_MATCHER(autoType);
   REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -544,6 +544,15 @@
 
 } // namespace
 
+bool HasNameMatcher::matchesNode(const NamedDecl , StringRef Name) {
+  if (Name.contains("::")) // slow path
+return HasNameMatcher({Name.str()}).matchesNode(Node);
+  // Fast path.
+  llvm::SmallString<128> Scratch;
+  StringRef NodeName = getNodeName(Node, Scratch);
+  return consumeNameSuffix(Name, NodeName) && Name.empty();
+}
+
 bool HasNameMatcher::matchesNodeUnqualified(const NamedDecl ) const {
   assert(UseUnqualifiedMatch);
   llvm::SmallString<128> Scratch;
@@ -973,6 +982,7 @@
 const internal::VariadicAllOfMatcher nestedNameSpecifier;
 const internal::VariadicAllOfMatcher
 nestedNameSpecifierLoc;
+const internal::VariadicAllOfMatcher attr;
 const internal::VariadicDynCastAllOfMatcher
 cudaKernelCallExpr;
 const AstTypeMatcher builtinType;
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -131,6 +131,8 @@
 else if (const TemplateArgumentLoc *TALoc =
  DynNode.get())
   traverse(*TALoc);
+else if (const Attr *A = DynNode.get())
+  traverse(*A);
 // FIXME: Add 

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

2020-10-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D89743#2342229 , @aaron.ballman 
wrote:

> In D89743#2341900 , @sammccall wrote:
>
>> In D89743#2341779 , @aaron.ballman 
>> wrote:
>>
>>> This is *awesome*, thank you so much for working on it!
>>
>> Thanks for the comments - haven't addressed them yet, but wanted to clarify 
>> scope first.
>>
>>> One question I have is: as it stands, this is not a particularly useful 
>>> matcher because it can only be used to say "yup, there's an attribute"
>>
>> Yes definitely, I should have mentioned this...
>>
>> My main goal here was to support it in DynTypedNode, as we have APIs 
>> (clangd::SelectionTree) that can only handle nodes that fit there.
>> But the common test pattern in ASTTypeTraitsTest used matchers, and I 
>> figured a basic matcher wasn't hard to add.
>
> Okay, that makes sense to me.
>
>>> are you planning to extend this functionality so that you can do something 
>>> like `attr(hasName("foo"))`, or specify the syntax the attribute uses, 
>>> `isImplicit()`, etc?
>>
>> I hadn't planned to - it definitely makes sense though I don't have any 
>> immediate use cases. I can do any of:
>>
>> - leave as-is, waiting for someone to add matchers to make it useful
>> - scope down the patch to exclude the matcher (and write the 
>> ASTTypeTraitsTest another way)
>> - add some simple matcher like hasName (I guess in a followup patch) to make 
>> it minimally useful, with space for more
>>
>> What do you think?
>
> My preference is to add support for `hasName()` as that's going to be the 
> most common need for matching attributes.

Sounds good (though I ran into issues calling this `hasName` specifically, see 
below). There's overlap with hasAttr(Kind) but I think that only handles decls, 
you can't bind the attribute itself, and it's harder to extend to the other 
stuff you mentioned.
So maybe hasAttr gets deprecated, or maybe `has(attr(hasName("foo")))` is a 
mouthful for a common case so the shorthand is nice.

> If you also added support for matching which syntax is used, support 
> attributes for `hasArgument()` (which I suppose could be interesting given 
> that there are the parsed attribute arguments and the semantic attribute 
> arguments, and they may differ), etc, I certainly wouldn't complain, but I 
> think at least name matching support is required to make the matcher 
> marginally useful.

Yeah, in the interests of time I think i'll need to stop there (my 
understanding of the attribute model is pretty superficial).

> I'm totally fine if this happens in a follow-up patch (or patches). WDYT?

For `hasAttrName` and `isImplicit` (which simplifies the tests), I've tried to 
fold it into this patch, it's simple enough.
I went with `hasAttrName` (matching `Attr::getAttrName()`) because I wasn't 
easily able to get `attrName` to work.

The issue is that `hasName` is already the name of a Decl matcher with the same 
signature, so we'd have to make it polymorphic.
(This means they'd appear to be one matcher, with one set of docs etc, despite 
having very different semantics - doesn't really seem right...)
The implementation of the Decl version is the fairly complex HasNameMatcher 
(shared with `hasAnyName`) that copies strings into a vector on construction. 
This doesn't fit into the polymorphic matcher model as far as I can find, so 
we're adding heap allocations to the fastpath of matching hasName("foo") which 
seems... not obviously a good tradeoff.

If you think the `hasName` name is important I'm happy to drop `hasAttrName` 
from this patch, and someone can work out how to add it with the better name, 
but I'm not sure I want to pick this battle with the template gods...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

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


[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

2020-10-22 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added inline comments.



Comment at: clang/test/Parser/pragma-fenv_access.c:28
+#if defined(CPP) & defined(STRICT)
+//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a 
constant expression}}
+//not-expected-note@+2 {{compile time floating point arithmetic suppressed in 
strict evaluation modes}}

rsmith wrote:
> mibintc wrote:
> > @rsmith no diagnostic, is this OK?
> Under the new approach from D89360, I would expect no diagnostic with `CONST` 
> defined as `constexpr`, because the initializer is then manifestly 
> constant-evaluated, so should be evaluated in the constant rounding mode.
> 
> With `CONST` defined as merely `const`, I'd expect that we emit a constrained 
> floating-point operation using the runtime rounding mode here. You can test 
> that we don't constant-evaluate the value of a (non-`constexpr`) `const 
> float` variable by using this hack (with `CONST` defined as `const`):
> 
> ```
> enum {
>   e1 = (int)one, e3 = (int)three, e4 = (int)four, e_four_quarters = 
> (int)(frac_ok * 4)
> };
> static_assert(e1 == 1  && e3 == 3 && e4 == 4 && e_four_quarters == 1, "");
> enum {
>   e_three_thirds = (int)(frac * 3) // expected-error {{not an integral 
> constant expression}}
> };
> ```
> 
> (The hack here is that we permit constant folding in the values of 
> enumerators, and we allow constant folding to look at the evaluated values of 
> `const float` variables. This should not be allowed for `frac`, because 
> attempting to evaluate its initializer should fail.)
I'm not seeing the expected-error for e_three_thirds = (int)(frac * 3)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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


[PATCH] D89961: [libTooling] Add function to Transformer to create a no-op edit.

2020-10-22 Thread Yitzhak Mandelbaum 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 rG6f8f5cb77efd: [libTooling] Add function to Transformer to 
create a no-op edit. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89961

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -426,6 +426,14 @@
   testRule(makeRule(returnStmt().bind("return"), noEdits()), Input, Input);
 }
 
+TEST_F(TransformerTest, NoopEdit) {
+  using transformer::node;
+  using transformer::noopEdit;
+  std::string Input = "int f(int x) { return x; }";
+  testRule(makeRule(returnStmt().bind("return"), noopEdit(node("return"))),
+   Input, Input);
+}
+
 TEST_F(TransformerTest, IfBound2Args) {
   using transformer::ifBound;
   std::string Input = "int f(int x) { return x; }";
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -73,6 +73,24 @@
   };
 }
 
+EditGenerator transformer::noopEdit(RangeSelector Anchor) {
+  return [Anchor = std::move(Anchor)](const MatchResult )
+ -> Expected> {
+Expected Range = Anchor(Result);
+if (!Range)
+  return Range.takeError();
+// In case the range is inside a macro expansion, map the location back to 
a
+// "real" source location.
+SourceLocation Begin =
+Result.SourceManager->getSpellingLoc(Range->getBegin());
+Edit E;
+// Implicitly, leave `E.Replacement` as the empty string.
+E.Kind = EditKind::Range;
+E.Range = CharSourceRange::getCharRange(Begin, Begin);
+return SmallVector{E};
+  };
+}
+
 EditGenerator
 transformer::flattenVector(SmallVector Generators) {
   if (Generators.size() == 1)
Index: clang/include/clang/Tooling/Transformer/RewriteRule.h
===
--- clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -107,7 +107,7 @@
   TextGenerator Replacement;
   TextGenerator Note;
   // Not all transformations will want or need to attach metadata and therefore
-  // should not be requierd to do so.
+  // should not be required to do so.
   AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &)
   -> llvm::Expected {
 return llvm::Expected(llvm::Any());
@@ -131,6 +131,11 @@
 /// Generates no edits.
 inline EditGenerator noEdits() { return editList({}); }
 
+/// Generates a single, no-op edit anchored at the start location of the
+/// specified range. A `noopEdit` may be preferred over `noEdits` to associate 
a
+/// diagnostic `Explanation` with the rule.
+EditGenerator noopEdit(RangeSelector Anchor);
+
 /// Version of `ifBound` specialized to `ASTEdit`.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit,
  ASTEdit FalseEdit) {


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -426,6 +426,14 @@
   testRule(makeRule(returnStmt().bind("return"), noEdits()), Input, Input);
 }
 
+TEST_F(TransformerTest, NoopEdit) {
+  using transformer::node;
+  using transformer::noopEdit;
+  std::string Input = "int f(int x) { return x; }";
+  testRule(makeRule(returnStmt().bind("return"), noopEdit(node("return"))),
+   Input, Input);
+}
+
 TEST_F(TransformerTest, IfBound2Args) {
   using transformer::ifBound;
   std::string Input = "int f(int x) { return x; }";
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -73,6 +73,24 @@
   };
 }
 
+EditGenerator transformer::noopEdit(RangeSelector Anchor) {
+  return [Anchor = std::move(Anchor)](const MatchResult )
+ -> Expected> {
+Expected Range = Anchor(Result);
+if (!Range)
+  return Range.takeError();
+// In case the range is inside a macro expansion, map the location back to a
+// "real" source location.
+SourceLocation Begin =
+Result.SourceManager->getSpellingLoc(Range->getBegin());
+Edit E;
+// Implicitly, leave `E.Replacement` as the empty string.
+E.Kind = EditKind::Range;
+E.Range = CharSourceRange::getCharRange(Begin, Begin);
+return SmallVector{E};
+  

[clang] 6f8f5cb - [libTooling] Add function to Transformer to create a no-op edit.

2020-10-22 Thread Yitzhak Mandelbaum via cfe-commits

Author: Yitzhak Mandelbaum
Date: 2020-10-22T21:29:03Z
New Revision: 6f8f5cb77efd100e5d4916db871b18c88cf49ed0

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

LOG: [libTooling] Add function to Transformer to create a no-op edit.

This functionality is commonly needed in clang tidy checks (based on
transformer) that only print warnings, without suggesting any edits. The no-op
edit allows the user to associate a diagnostic message with a source location.

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/RewriteRule.h
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/unittests/Tooling/TransformerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h 
b/clang/include/clang/Tooling/Transformer/RewriteRule.h
index 4bdcc8d5c329..13d0edaf11b3 100644
--- a/clang/include/clang/Tooling/Transformer/RewriteRule.h
+++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h
@@ -107,7 +107,7 @@ struct ASTEdit {
   TextGenerator Replacement;
   TextGenerator Note;
   // Not all transformations will want or need to attach metadata and therefore
-  // should not be requierd to do so.
+  // should not be required to do so.
   AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &)
   -> llvm::Expected {
 return llvm::Expected(llvm::Any());
@@ -131,6 +131,11 @@ EditGenerator editList(llvm::SmallVector 
Edits);
 /// Generates no edits.
 inline EditGenerator noEdits() { return editList({}); }
 
+/// Generates a single, no-op edit anchored at the start location of the
+/// specified range. A `noopEdit` may be preferred over `noEdits` to associate 
a
+/// diagnostic `Explanation` with the rule.
+EditGenerator noopEdit(RangeSelector Anchor);
+
 /// Version of `ifBound` specialized to `ASTEdit`.
 inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit,
  ASTEdit FalseEdit) {

diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp 
b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index 03921e0ea7de..e0467d2149cc 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -73,6 +73,24 @@ EditGenerator transformer::edit(ASTEdit Edit) {
   };
 }
 
+EditGenerator transformer::noopEdit(RangeSelector Anchor) {
+  return [Anchor = std::move(Anchor)](const MatchResult )
+ -> Expected> {
+Expected Range = Anchor(Result);
+if (!Range)
+  return Range.takeError();
+// In case the range is inside a macro expansion, map the location back to 
a
+// "real" source location.
+SourceLocation Begin =
+Result.SourceManager->getSpellingLoc(Range->getBegin());
+Edit E;
+// Implicitly, leave `E.Replacement` as the empty string.
+E.Kind = EditKind::Range;
+E.Range = CharSourceRange::getCharRange(Begin, Begin);
+return SmallVector{E};
+  };
+}
+
 EditGenerator
 transformer::flattenVector(SmallVector Generators) {
   if (Generators.size() == 1)

diff  --git a/clang/unittests/Tooling/TransformerTest.cpp 
b/clang/unittests/Tooling/TransformerTest.cpp
index a8d6d3dd851d..ff735cd73198 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -426,6 +426,14 @@ TEST_F(TransformerTest, NoEdits) {
   testRule(makeRule(returnStmt().bind("return"), noEdits()), Input, Input);
 }
 
+TEST_F(TransformerTest, NoopEdit) {
+  using transformer::node;
+  using transformer::noopEdit;
+  std::string Input = "int f(int x) { return x; }";
+  testRule(makeRule(returnStmt().bind("return"), noopEdit(node("return"))),
+   Input, Input);
+}
+
 TEST_F(TransformerTest, IfBound2Args) {
   using transformer::ifBound;
   std::string Input = "int f(int x) { return x; }";



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


[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-22 Thread Digger via Phabricator via cfe-commits
DiggerLin created this revision.
DiggerLin added reviewers: jasonliu, hubert.reinterpretcast, sfertile.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
DiggerLin requested review of this revision.

in AIX OS, we will ignore the option -fvisibility-inlines-hidden when there is 
no option -fvisibility=*


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89986

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/aix-visibility-inlines-hidden.cpp


Index: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -o - -x c++ -S  
%s  |\
+// RUN:   FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden  -fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefix=COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -emit-llvm -o - 
-x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden -fvisibility default -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,VISIBILITY-IR %s
+
+int x = 66;
+__attribute__((__noinline__)) inline void f() {
+  x = 55;
+}
+int bar() {
+  f();
+  return x;
+}
+
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop
+
+// VISIBILITY-IR: define linkonce_odr hidden void @_Z1fv()
+// NOVISIBILITY-IR:   define linkonce_odr void @_Z1fv()
+// COMMON-IR-NEXT:  entry:
+// COMMON-IR-NEXT:store i32 55, i32* @x, align 4
+// COMMON-IR-NEXT:ret void
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-fvisibility-inlines-hidden  -fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefix=CHECK %s
+/// CHECK-NOT: nop
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2779,8 +2779,12 @@
 Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
   }
 
-  if (Args.hasArg(OPT_fvisibility_inlines_hidden))
-Opts.InlineVisibilityHidden = 1;
+  if (Args.hasArg(OPT_fvisibility_inlines_hidden)) {
+if (T.isOSAIX() && !Args.hasArg(OPT_fvisibility))
+  Opts.InlineVisibilityHidden = 0;
+else
+  Opts.InlineVisibilityHidden = 1;
+  }
 
   if (Args.hasArg(OPT_fvisibility_inlines_hidden_static_local_var))
 Opts.VisibilityInlinesHiddenStaticLocalVar = 1;


Index: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-visibility-inlines-hidden.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -o - -x c++ -S  %s  |\
+// RUN:   FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefixes=NOP-ASM,COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden  -fvisibility default -o - -x c++ -S %s  | \
+// RUN: FileCheck --check-prefix=COMMON-ASM %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -fvisibility default -emit-llvm -o - -x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,VISIBILITY-IR %s
+
+int x = 66;
+__attribute__((__noinline__)) inline void f() {
+  x = 55;
+}
+int bar() {
+  f();
+  return x;
+}
+
+// COMMON-ASM: mflr 0
+// COMMON-ASM-NEXT:stw 0, 8(1)
+// COMMON-ASM-NEXT:stwu 1, -64(1)
+// COMMON-ASM-NEXT:bl ._Z1fv
+// NOP-ASM-NEXT:   nop
+
+// VISIBILITY-IR: define linkonce_odr hidden void @_Z1fv()
+// NOVISIBILITY-IR:   define linkonce_odr 

[clang] 8277a51 - [SourceManager] Avoid copying SLocEntry in computeMacroArgsCache

2020-10-22 Thread Jan Korous via cfe-commits

Author: Jan Korous
Date: 2020-10-22T14:18:28-07:00
New Revision: 8277a513c74b53e5ad7f482f76b27bc8609c66ae

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

LOG: [SourceManager] Avoid copying SLocEntry in computeMacroArgsCache

Follow-up to e7870223d8b5

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

Added: 


Modified: 
clang/lib/Basic/SourceManager.cpp

Removed: 




diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index 3bffcb548ebb..db87a6e57741 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -1761,7 +1761,7 @@ void SourceManager::computeMacroArgsCache(MacroArgsMap 
,
 if (Invalid)
   return;
 if (Entry.isFile()) {
-  auto File = Entry.getFile();
+  auto& File = Entry.getFile();
   if (File.getFileCharacteristic() == C_User_ModuleMap ||
   File.getFileCharacteristic() == C_System_ModuleMap)
 continue;



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


[clang] fd14a1f - [clang][Frontend] Add missing error handling

2020-10-22 Thread David Blaikie via cfe-commits

Author: LemonBoy
Date: 2020-10-22T14:14:19-07:00
New Revision: fd14a1f6fff37ef62a35ca0f8bc630ac8b23516d

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

LOG: [clang][Frontend] Add missing error handling

Some early errors during the ASTUnit creation were not transferred to the 
`FailedParseDiagnostic` so when the code in `LoadFromCommandLine` swaps its 
content with the content of `StoredDiagnostics` they cannot be retrieved by the 
user in any way.

Reviewed By: andrewrk, dblaikie

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

Added: 


Modified: 
clang/lib/Frontend/ASTUnit.cpp
clang/unittests/Frontend/ASTUnitTest.cpp

Removed: 




diff  --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 7fca1900853b..0728fee49d78 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -69,6 +69,7 @@
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
@@ -1118,6 +1119,19 @@ bool 
ASTUnit::Parse(std::shared_ptr PCHContainerOps,
   std::unique_ptr Clang(
   new CompilerInstance(std::move(PCHContainerOps)));
 
+  // Clean up on error, disengage it if the function returns successfully.
+  auto CleanOnError = llvm::make_scope_exit([&]() {
+// Remove the overridden buffer we used for the preamble.
+SavedMainFileBuffer = nullptr;
+
+// Keep the ownership of the data in the ASTUnit because the client may
+// want to see the diagnostics.
+transferASTDataFromCompilerInstance(*Clang);
+FailedParseDiagnostics.swap(StoredDiagnostics);
+StoredDiagnostics.clear();
+NumStoredDiagnosticsFromDriver = 0;
+  });
+
   // Ensure that Clang has a FileManager with the right VFS, which may have
   // changed above in AddImplicitPreamble.  If VFS is nullptr, rely on
   // createFileManager to create one.
@@ -1200,7 +1214,7 @@ bool 
ASTUnit::Parse(std::shared_ptr PCHContainerOps,
 ActCleanup(Act.get());
 
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
-goto error;
+return true;
 
   if (SavedMainFileBuffer)
 TranslateStoredDiagnostics(getFileManager(), getSourceManager(),
@@ -1210,7 +1224,7 @@ bool 
ASTUnit::Parse(std::shared_ptr PCHContainerOps,
 
   if (llvm::Error Err = Act->Execute()) {
 consumeError(std::move(Err)); // FIXME this drops errors on the floor.
-goto error;
+return true;
   }
 
   transferASTDataFromCompilerInstance(*Clang);
@@ -1219,19 +1233,9 @@ bool 
ASTUnit::Parse(std::shared_ptr PCHContainerOps,
 
   FailedParseDiagnostics.clear();
 
-  return false;
+  CleanOnError.release();
 
-error:
-  // Remove the overridden buffer we used for the preamble.
-  SavedMainFileBuffer = nullptr;
-
-  // Keep the ownership of the data in the ASTUnit because the client may
-  // want to see the diagnostics.
-  transferASTDataFromCompilerInstance(*Clang);
-  FailedParseDiagnostics.swap(StoredDiagnostics);
-  StoredDiagnostics.clear();
-  NumStoredDiagnosticsFromDriver = 0;
-  return true;
+  return false;
 }
 
 static std::pair

diff  --git a/clang/unittests/Frontend/ASTUnitTest.cpp 
b/clang/unittests/Frontend/ASTUnitTest.cpp
index c5b84e74cee9..eb3fdb48a2da 100644
--- a/clang/unittests/Frontend/ASTUnitTest.cpp
+++ b/clang/unittests/Frontend/ASTUnitTest.cpp
@@ -150,4 +150,28 @@ TEST_F(ASTUnitTest, ModuleTextualHeader) {
   >getFileEntry()));
 }
 
+TEST_F(ASTUnitTest, LoadFromCommandLineEarlyError) {
+  EXPECT_FALSE(
+  llvm::sys::fs::createTemporaryFile("ast-unit", "c", FD, InputFileName));
+  input_file = std::make_unique(InputFileName, FD);
+  input_file->os() << "";
+
+  const char *Args[] = {"clang", "-target", "foobar", InputFileName.c_str()};
+
+  auto Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  auto PCHContainerOps = std::make_shared();
+  std::unique_ptr ErrUnit;
+
+  ASTUnit *AST = ASTUnit::LoadFromCommandLine(
+  [0], [4], PCHContainerOps, Diags, "", false,
+  CaptureDiagsKind::All, None, true, 0, TU_Complete, false, false, false,
+  SkipFunctionBodiesScope::None, false, true, false, false, None, ,
+  nullptr);
+
+  ASSERT_EQ(AST, nullptr);
+  ASSERT_NE(ErrUnit, nullptr);
+  ASSERT_TRUE(Diags->hasErrorOccurred());
+  ASSERT_NE(ErrUnit->stored_diag_size(), 0U);
+}
+
 } // anonymous namespace



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


[PATCH] D78658: [clang][Frontend] Add missing error handling

2020-10-22 Thread David Blaikie via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd14a1f6fff3: [clang][Frontend] Add missing error handling 
(authored by LemonBoy, committed by dblaikie).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78658

Files:
  clang/lib/Frontend/ASTUnit.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp

Index: clang/unittests/Frontend/ASTUnitTest.cpp
===
--- clang/unittests/Frontend/ASTUnitTest.cpp
+++ clang/unittests/Frontend/ASTUnitTest.cpp
@@ -150,4 +150,28 @@
   >getFileEntry()));
 }
 
+TEST_F(ASTUnitTest, LoadFromCommandLineEarlyError) {
+  EXPECT_FALSE(
+  llvm::sys::fs::createTemporaryFile("ast-unit", "c", FD, InputFileName));
+  input_file = std::make_unique(InputFileName, FD);
+  input_file->os() << "";
+
+  const char *Args[] = {"clang", "-target", "foobar", InputFileName.c_str()};
+
+  auto Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  auto PCHContainerOps = std::make_shared();
+  std::unique_ptr ErrUnit;
+
+  ASTUnit *AST = ASTUnit::LoadFromCommandLine(
+  [0], [4], PCHContainerOps, Diags, "", false,
+  CaptureDiagsKind::All, None, true, 0, TU_Complete, false, false, false,
+  SkipFunctionBodiesScope::None, false, true, false, false, None, ,
+  nullptr);
+
+  ASSERT_EQ(AST, nullptr);
+  ASSERT_NE(ErrUnit, nullptr);
+  ASSERT_TRUE(Diags->hasErrorOccurred());
+  ASSERT_NE(ErrUnit->stored_diag_size(), 0U);
+}
+
 } // anonymous namespace
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -69,6 +69,7 @@
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
@@ -1118,6 +1119,19 @@
   std::unique_ptr Clang(
   new CompilerInstance(std::move(PCHContainerOps)));
 
+  // Clean up on error, disengage it if the function returns successfully.
+  auto CleanOnError = llvm::make_scope_exit([&]() {
+// Remove the overridden buffer we used for the preamble.
+SavedMainFileBuffer = nullptr;
+
+// Keep the ownership of the data in the ASTUnit because the client may
+// want to see the diagnostics.
+transferASTDataFromCompilerInstance(*Clang);
+FailedParseDiagnostics.swap(StoredDiagnostics);
+StoredDiagnostics.clear();
+NumStoredDiagnosticsFromDriver = 0;
+  });
+
   // Ensure that Clang has a FileManager with the right VFS, which may have
   // changed above in AddImplicitPreamble.  If VFS is nullptr, rely on
   // createFileManager to create one.
@@ -1200,7 +1214,7 @@
 ActCleanup(Act.get());
 
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
-goto error;
+return true;
 
   if (SavedMainFileBuffer)
 TranslateStoredDiagnostics(getFileManager(), getSourceManager(),
@@ -1210,7 +1224,7 @@
 
   if (llvm::Error Err = Act->Execute()) {
 consumeError(std::move(Err)); // FIXME this drops errors on the floor.
-goto error;
+return true;
   }
 
   transferASTDataFromCompilerInstance(*Clang);
@@ -1219,19 +1233,9 @@
 
   FailedParseDiagnostics.clear();
 
-  return false;
+  CleanOnError.release();
 
-error:
-  // Remove the overridden buffer we used for the preamble.
-  SavedMainFileBuffer = nullptr;
-
-  // Keep the ownership of the data in the ASTUnit because the client may
-  // want to see the diagnostics.
-  transferASTDataFromCompilerInstance(*Clang);
-  FailedParseDiagnostics.swap(StoredDiagnostics);
-  StoredDiagnostics.clear();
-  NumStoredDiagnosticsFromDriver = 0;
-  return true;
+  return false;
 }
 
 static std::pair
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment.

In D87956#2348433 , @nickdesaulniers 
wrote:

> In phab here, it looks like my newly added 
> clang/test/Frontend/optimization-remark-missed-inline-stack-protectors.c 
> fails on windows because I redirect stdout to /dev/null.  How does that work 
> for other tests?  I see other tests in that dir write to /dev/null.  Oh, I 
> guess `-o /dev/null` will just create a file called `"/dev/null"` on Windows? 
> So I should do that rather than `1> /dev/null`?

Why not just use "expected-error" and the like?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

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


[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

In phab here, it looks like my newly added 
clang/test/Frontend/optimization-remark-missed-inline-stack-protectors.c fails 
on windows because I redirect stdout to /dev/null.  How does that work for 
other tests?  I see other tests in that dir write to /dev/null.  Oh, I guess 
`-o /dev/null` will just create a file called `"/dev/null"` on Windows? So I 
should do that rather than `1> /dev/null`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

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


[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

2020-10-22 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: llvm/lib/IR/Attributes.cpp:1901-1902
+  // caller was explicitly annotated as nossp.
+  if (Caller.hasFnAttribute(Attribute::NoStackProtect))
+return;
   // If upgrading the SSP attribute, clear out the old SSP Attributes first.

void wrote:
> nickdesaulniers wrote:
> > This should be an anomalous situation due to the added verifier check. 
> > Should I make it an assert instead?
> If it should never happen, then please make it an assert.
Ah this assertion is not quite right; we still would like to permit `nossp` 
callee being inlined into `nossp` caller.  Let me add a test case for that, 
then make the assertion more precise about a **mismatch** between caller and 
callee.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87956

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


[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-10-22 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment.

In D87188#2348326 , @nikic wrote:

> Reopening this so we don't forget...
>
> I believe @spatel is working on the cost modelling. I did not have much luck 
> tracking down the miscompile, at least did not spot anything incriminating in 
> the llvm-diff.

Yes, I'm working through the mess of the TTI cost model with things like:
c963bde0152a 

It's a slog...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87188

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


[PATCH] D89980: [hip] Remove kernel argument coercion.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

Are there any tests to illustrate what this change does to IR or generated code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89980

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


[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D89974#2348310 , @rnk wrote:

> Lastly, if those things don't work, there is the `CCC_OVERRIDE_OPTIONS` 
> environment variable, so the user always has that escape hatch. A separate 
> environment variable would also be fine. It reminds me of `PYTHON_HOME`, with 
> similar drawbacks.

Oh. I didn't know about `CCC_OVERRIDE_OPTIONS`.  It appears that does a bit 
more that just adding extra options. Is it documented somewhere?

Also, clang appears to use SDKROOT environment for specifying XCode path on 
MacOs, presumably for the reasons similar to what we're dealing with here with 
CUDA.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D89897: [AIX] Emit error for -G option on AIX

2020-10-22 Thread Xiangling Liao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ba98433971f: [AIX] Emit error for -G option on AIX 
(authored by Xiangling_L).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89897

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aix-err-options.c
  llvm/CMakeLists.txt


Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -930,7 +930,7 @@
 
   # Modules should be built with -G, so we can use runtime linking with
   # plugins.
-  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -G")
+  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -Wl,-G")
 
   # Also set the correct flags for building shared libraries.
   string(APPEND CMAKE_SHARED_LINKER_FLAGS " -shared")
Index: clang/test/Driver/aix-err-options.c
===
--- /dev/null
+++ clang/test/Driver/aix-err-options.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck --check-prefix=CHECK32 %s
+// RUN: %clang -target powerpc64-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck --check-prefix=CHECK64 %s
+
+// CHECK32: error: unsupported option '-G' for target 'powerpc32-ibm-aix-xcoff'
+// CHECK64: error: unsupported option '-G' for target 'powerpc64-ibm-aix-xcoff'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4958,6 +4958,11 @@
   if (D.CCGenDiagnostics)
 CmdArgs.push_back("-disable-pragma-debug-crash");
 
+  if (RawTriple.isOSAIX())
+if (Arg *A = Args.getLastArg(options::OPT_G))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << A->getSpelling() << RawTriple.str();
+
   bool UseSeparateSections = isUseSeparateSections(Triple);
 
   if (Args.hasFlag(options::OPT_ffunction_sections,


Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -930,7 +930,7 @@
 
   # Modules should be built with -G, so we can use runtime linking with
   # plugins.
-  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -G")
+  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -Wl,-G")
 
   # Also set the correct flags for building shared libraries.
   string(APPEND CMAKE_SHARED_LINKER_FLAGS " -shared")
Index: clang/test/Driver/aix-err-options.c
===
--- /dev/null
+++ clang/test/Driver/aix-err-options.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s | \
+// RUN:   FileCheck --check-prefix=CHECK32 %s
+// RUN: %clang -target powerpc64-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s | \
+// RUN:   FileCheck --check-prefix=CHECK64 %s
+
+// CHECK32: error: unsupported option '-G' for target 'powerpc32-ibm-aix-xcoff'
+// CHECK64: error: unsupported option '-G' for target 'powerpc64-ibm-aix-xcoff'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4958,6 +4958,11 @@
   if (D.CCGenDiagnostics)
 CmdArgs.push_back("-disable-pragma-debug-crash");
 
+  if (RawTriple.isOSAIX())
+if (Arg *A = Args.getLastArg(options::OPT_G))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << A->getSpelling() << RawTriple.str();
+
   bool UseSeparateSections = isUseSeparateSections(Triple);
 
   if (Args.hasFlag(options::OPT_ffunction_sections,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 0ba9843 - [AIX] Emit error for -G option on AIX

2020-10-22 Thread Xiangling Liao via cfe-commits

Author: Xiangling Liao
Date: 2020-10-22T16:16:39-04:00
New Revision: 0ba98433971f6aa7cf4dc2befe7b5446d25d5956

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

LOG: [AIX] Emit error for -G option on AIX

1. Emit error for -G driver option on AIX
2. Adjust cmake file to use -Wl,-G instead of -G

On AIX, legacy XL compiler uses -G to produce a shared object enabled
for use with the run-time linker, which has different meanings from what
it is used for in Clang. And in Clang, other targets do not have -G map
to another functionality in their legacy compiler. So this error is more
important when we are on AIX.

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

Added: 
clang/test/Driver/aix-err-options.c

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
llvm/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 61e367bd835d..61b6af59e8b5 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4958,6 +4958,11 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
   if (D.CCGenDiagnostics)
 CmdArgs.push_back("-disable-pragma-debug-crash");
 
+  if (RawTriple.isOSAIX())
+if (Arg *A = Args.getLastArg(options::OPT_G))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << A->getSpelling() << RawTriple.str();
+
   bool UseSeparateSections = isUseSeparateSections(Triple);
 
   if (Args.hasFlag(options::OPT_ffunction_sections,

diff  --git a/clang/test/Driver/aix-err-options.c 
b/clang/test/Driver/aix-err-options.c
new file mode 100644
index ..6ed8363c161f
--- /dev/null
+++ b/clang/test/Driver/aix-err-options.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck --check-prefix=CHECK32 %s
+// RUN: %clang -target powerpc64-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck --check-prefix=CHECK64 %s
+
+// CHECK32: error: unsupported option '-G' for target 'powerpc32-ibm-aix-xcoff'
+// CHECK64: error: unsupported option '-G' for target 'powerpc64-ibm-aix-xcoff'

diff  --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 344ccb6fda2f..39d78c70c02e 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -930,7 +930,7 @@ if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
 
   # Modules should be built with -G, so we can use runtime linking with
   # plugins.
-  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -G")
+  string(APPEND CMAKE_MODULE_LINKER_FLAGS " -Wl,-G")
 
   # Also set the correct flags for building shared libraries.
   string(APPEND CMAKE_SHARED_LINKER_FLAGS " -shared")



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


[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

2020-10-22 Thread Nikita Popov via Phabricator via cfe-commits
nikic reopened this revision.
nikic added a comment.
This revision is now accepted and ready to land.

Reopening this so we don't forget...

I believe @spatel is working on the cost modelling. I did not have much luck 
tracking down the miscompile, at least did not spot anything incriminating in 
the llvm-diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87188

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


[PATCH] D88659: [NFC] Fix the definition of SuitableAlign

2020-10-22 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L closed this revision.
Xiangling_L added a comment.

Manually close the revision after landing it upstream.


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

https://reviews.llvm.org/D88659

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


[clang] 57cdc52 - Initial support for vectorization using Libmvec (GLIBC vector math library)

2020-10-22 Thread Sanjay Patel via cfe-commits

Author: Venkataramanan Kumar
Date: 2020-10-22T16:01:39-04:00
New Revision: 57cdc52c4df0a8a6835ddeede787b23c0ce9e358

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

LOG: Initial support for vectorization using Libmvec (GLIBC vector math library)

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

Added: 
llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-VF2-VF8.ll
llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll
llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Driver/autocomplete.c
clang/test/Driver/fveclib.c
llvm/include/llvm/Analysis/TargetLibraryInfo.h
llvm/include/llvm/Analysis/VecFuncs.def
llvm/lib/Analysis/TargetLibraryInfo.cpp
llvm/test/Transforms/Util/add-TLI-mappings.ll

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 0ab9054f0bb5..f5222b50fc7b 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -348,7 +348,7 @@ CODEGENOPT(CodeViewGHash, 1, 0)
 ENUM_CODEGENOPT(Inlining, InliningMethod, 2, NormalInlining)
 
 // Vector functions library to use.
-ENUM_CODEGENOPT(VecLib, VectorLibrary, 2, NoLibrary)
+ENUM_CODEGENOPT(VecLib, VectorLibrary, 3, NoLibrary)
 
 /// The default TLS model to use.
 ENUM_CODEGENOPT(DefaultTLSModel, TLSModel, 2, GeneralDynamicTLSModel)

diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index f658c9b8a781..764d0a17cb72 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -54,11 +54,11 @@ class CodeGenOptions : public CodeGenOptionsBase {
   enum VectorLibrary {
 NoLibrary,  // Don't use any vector library.
 Accelerate, // Use the Accelerate framework.
+LIBMVEC,// GLIBC vector math library.
 MASSV,  // IBM MASS vector library.
 SVML// Intel short vector math library.
   };
 
-
   enum ObjCDispatchMethodKind {
 Legacy = 0,
 NonLegacy = 1,

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 1d930dbed9c4..0cab3e8ecc16 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1582,7 +1582,7 @@ def fno_experimental_new_pass_manager : Flag<["-"], 
"fno-experimental-new-pass-m
   Group, Flags<[CC1Option]>,
   HelpText<"Disables an experimental new pass manager in LLVM.">;
 def fveclib : Joined<["-"], "fveclib=">, Group, Flags<[CC1Option]>,
-HelpText<"Use the given vector functions library">, 
Values<"Accelerate,MASSV,SVML,none">;
+HelpText<"Use the given vector functions library">, 
Values<"Accelerate,libmvec,MASSV,SVML,none">;
 def fno_lax_vector_conversions : Flag<["-"], "fno-lax-vector-conversions">, 
Group,
   Alias, AliasArgs<["none"]>;
 def fno_merge_all_constants : Flag<["-"], "fno-merge-all-constants">, 
Group,

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index b9c90eee64ed..0991582005b8 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -371,6 +371,16 @@ static TargetLibraryInfoImpl *createTLII(llvm::Triple 
,
   case CodeGenOptions::Accelerate:
 
TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::Accelerate);
 break;
+  case CodeGenOptions::LIBMVEC:
+switch(TargetTriple.getArch()) {
+  default:
+break;
+  case llvm::Triple::x86_64:
+TLII->addVectorizableFunctionsFromVecLib
+(TargetLibraryInfoImpl::LIBMVEC_X86);
+break;
+}
+break;
   case CodeGenOptions::MASSV:
 TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::MASSV);
 break;

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index ff0df26dc1dd..5175063adb02 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -749,6 +749,8 @@ static bool ParseCodeGenArgs(CodeGenOptions , ArgList 
, InputKind IK,
 StringRef Name = A->getValue();
 if (Name == "Accelerate")
   Opts.setVecLib(CodeGenOptions::Accelerate);
+else if (Name == "libmvec")
+  Opts.setVecLib(CodeGenOptions::LIBMVEC);
 else if (Name == "MASSV")
   Opts.setVecLib(CodeGenOptions::MASSV);
 else if (Name == "SVML")

diff  --git a/clang/test/Driver/autocomplete.c 
b/clang/test/Driver/autocomplete.c
index a6e7be887c8c..c9f66de7cd5d 100644
--- 

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

2020-10-22 Thread Sanjay Patel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG57cdc52c4df0: Initial support for vectorization using 
Libmvec (GLIBC vector math library) (authored by venkataramanan.kumar.llvm, 
committed by spatel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88154

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/autocomplete.c
  clang/test/Driver/fveclib.c
  llvm/include/llvm/Analysis/TargetLibraryInfo.h
  llvm/include/llvm/Analysis/VecFuncs.def
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-VF2-VF8.ll
  llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll
  llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
  llvm/test/Transforms/Util/add-TLI-mappings.ll

Index: llvm/test/Transforms/Util/add-TLI-mappings.ll
===
--- llvm/test/Transforms/Util/add-TLI-mappings.ll
+++ llvm/test/Transforms/Util/add-TLI-mappings.ll
@@ -3,6 +3,8 @@
 ; RUN: opt -vector-library=MASSV  -inject-tli-mappings-S < %s | FileCheck %s  --check-prefixes=COMMON,MASSV
 ; RUN: opt -vector-library=MASSV  -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,MASSV
 ; RUN: opt -vector-library=Accelerate -inject-tli-mappings-S < %s | FileCheck %s  --check-prefixes=COMMON,ACCELERATE
+; RUN: opt -vector-library=LIBMVEC-X86 -inject-tli-mappings-S < %s | FileCheck %s  --check-prefixes=COMMON,LIBMVEC-X86
+; RUN: opt -vector-library=LIBMVEC-X86 -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,LIBMVEC-X86
 ; RUN: opt -vector-library=Accelerate -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,ACCELERATE
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
@@ -21,6 +23,9 @@
 ; MASSV-SAME: i8* bitcast (<4 x float> (<4 x float>)* @__log10f4_massv to i8*)
 ; ACCELERATE-SAME:  [1 x i8*] [
 ; ACCELERATE-SAME:i8* bitcast (<4 x float> (<4 x float>)* @vlog10f to i8*)
+; LIBMVEC-X86-SAME: [2 x i8*] [
+; LIBMVEC-X86-SAME:   i8* bitcast (<2 x double> (<2 x double>)* @_ZGVbN2v_sin to i8*),
+; LIBMVEC-X86-SAME:   i8* bitcast (<4 x double> (<4 x double>)* @_ZGVdN4v_sin to i8*)
 ; COMMON-SAME:  ], section "llvm.metadata"
 
 define double @sin_f64(double %in) {
@@ -28,6 +33,7 @@
 ; SVML: call double @sin(double %{{.*}}) #[[SIN:[0-9]+]]
 ; MASSV:call double @sin(double %{{.*}}) #[[SIN:[0-9]+]]
 ; ACCELERATE:   call double @sin(double %{{.*}})
+; LIBMVEC-X86:  call double @sin(double %{{.*}}) #[[SIN:[0-9]+]]
 ; No mapping of "sin" to a vector function for Accelerate.
 ; ACCELERATE-NOT: _ZGV_LLVM_{{.*}}_sin({{.*}})
   %call = tail call double @sin(double %in)
@@ -39,10 +45,12 @@
 define float @call_llvm.log10.f32(float %in) {
 ; COMMON-LABEL: @call_llvm.log10.f32(
 ; SVML: call float @llvm.log10.f32(float %{{.*}})
+; LIBMVEC-X86:  call float @llvm.log10.f32(float %{{.*}})
 ; MASSV:call float @llvm.log10.f32(float %{{.*}}) #[[LOG10:[0-9]+]]
 ; ACCELERATE:   call float @llvm.log10.f32(float %{{.*}}) #[[LOG10:[0-9]+]]
 ; No mapping of "llvm.log10.f32" to a vector function for SVML.
 ; SVML-NOT: _ZGV_LLVM_{{.*}}_llvm.log10.f32({{.*}})
+; LIBMVEC-X86-NOT: _ZGV_LLVM_{{.*}}_llvm.log10.f32({{.*}})
   %call = tail call float @llvm.log10.f32(float %in)
   ret float %call
 }
@@ -62,3 +70,7 @@
 
 ; ACCELERATE:  attributes #[[LOG10]] = { "vector-function-abi-variant"=
 ; ACCELERATE-SAME:   "_ZGV_LLVM_N4v_llvm.log10.f32(vlog10f)" }
+
+; LIBMVEC-X86:  attributes #[[SIN]] = { "vector-function-abi-variant"=
+; LIBMVEC-X86-SAME:   "_ZGV_LLVM_N2v_sin(_ZGVbN2v_sin),
+; LIBMVEC-X86-SAME:   _ZGV_LLVM_N4v_sin(_ZGVdN4v_sin)" }
Index: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
===
--- /dev/null
+++ llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
@@ -0,0 +1,373 @@
+; RUN: opt -vector-library=LIBMVEC-X86  -inject-tli-mappings -loop-vectorize -S < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @sin_f64(double* nocapture %varray) {
+; CHECK-LABEL: @sin_f64(
+; CHECK-LABEL:vector.body
+; CHECK:[[TMP5:%.*]] = call <4 x double> @_ZGVdN4v_sin(<4 x double> [[TMP4:%.*]])
+;
+entry:
+  br label %for.body
+
+for.body:
+  %iv = phi i64 [ 0, %entry ], [ %iv.next, %for.body ]
+  %tmp = trunc i64 %iv to i32
+  %conv = sitofp i32 %tmp to double
+  %call = tail call double @sin(double %conv)
+  %arrayidx = getelementptr inbounds double, double* %varray, i64 %iv
+  store double %call, double* %arrayidx, 

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

My opinion carries less weight since I don't use CUDA, but I agree with 
everything Art said. Here's some input, if it helps.

I like the `PATH` search for `ptxas` as a way to make things work out of the 
box as often as possible.

I don't like the idea of CMake auto-detecting CUDA for the reasons Art listed. 
It's a great way to implicitly leak more details about the build environment to 
the user.

An explicit CMake option to override the CUDA installation path seems fine. A 
distribution could use this if they vend the CUDA SDK somewhere standard, for 
example.

Lastly, if those things don't work, there is the `CCC_OVERRIDE_OPTIONS` 
environment variable, so the user always has that escape hatch. A separate 
environment variable would also be fine. It reminds me of `PYTHON_HOME`, with 
similar drawbacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D86230: [SourceManager] Skip module maps when searching files for macro arguments

2020-10-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments.



Comment at: clang/lib/Basic/SourceManager.cpp:1764
 if (Entry.isFile()) {
-  SourceLocation IncludeLoc = Entry.getFile().getIncludeLoc();
+  auto File = Entry.getFile();
+  if (File.getFileCharacteristic() == C_User_ModuleMap ||

It seems unintentinoal to copy out the `FileInfo` here. Should this be a 
reference?
```
auto  = Entry.getFile();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86230

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


[PATCH] D89982: [analyzer] [NFC] Simplify SVal::getAsLocSymbol function using existing functions

2020-10-22 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: krememek, vsavchenko, NoQ, martong, steakhal.
ASDenysPetrov added a project: clang.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
ASDenysPetrov requested review of this revision.

Method of obtaining `MemRegion` from `LocAsInteger/MemRegionVal` already exists 
in `SVal::getAsRegion` function. Replace repetitive conditions in 
`SVal::getAsLocSymbol` with `SVal::getAsRegion` function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89982

Files:
  clang/lib/StaticAnalyzer/Core/SVals.cpp


Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -84,16 +84,12 @@
 /// the first symbolic parent region is returned.
 SymbolRef SVal::getAsLocSymbol(bool IncludeBaseRegions) const {
   // FIXME: should we consider SymbolRef wrapped in CodeTextRegion?
-  if (Optional X = getAs())
-return X->getLoc().getAsLocSymbol(IncludeBaseRegions);
-
-  if (Optional X = getAs()) {
-const MemRegion *R = X->getRegion();
-if (const SymbolicRegion *SymR = IncludeBaseRegions ?
-  R->getSymbolicBase() :
-  
dyn_cast(R->StripCasts()))
+  if (const MemRegion *R = getAsRegion())
+if (const SymbolicRegion *SymR =
+IncludeBaseRegions ? R->getSymbolicBase()
+   : dyn_cast(R->StripCasts()))
   return SymR->getSymbol();
-  }
+
   return nullptr;
 }
 


Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -84,16 +84,12 @@
 /// the first symbolic parent region is returned.
 SymbolRef SVal::getAsLocSymbol(bool IncludeBaseRegions) const {
   // FIXME: should we consider SymbolRef wrapped in CodeTextRegion?
-  if (Optional X = getAs())
-return X->getLoc().getAsLocSymbol(IncludeBaseRegions);
-
-  if (Optional X = getAs()) {
-const MemRegion *R = X->getRegion();
-if (const SymbolicRegion *SymR = IncludeBaseRegions ?
-  R->getSymbolicBase() :
-  dyn_cast(R->StripCasts()))
+  if (const MemRegion *R = getAsRegion())
+if (const SymbolicRegion *SymR =
+IncludeBaseRegions ? R->getSymbolicBase()
+   : dyn_cast(R->StripCasts()))
   return SymR->getSymbol();
-  }
+
   return nullptr;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D89980: [hip] Remove kernel argument coercion.

2020-10-22 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added reviewers: arsenm, tra, rjmccall, yaxunl.
Herald added subscribers: cfe-commits, kerbowa, nhaehnle, jvesely.
Herald added a project: clang.
hliao requested review of this revision.
Herald added a subscriber: wdng.

- If an aggregate argument is indirectly accessed within kernels, direct 
passing results in unpromotable `alloca`, which degrade performance 
significantly. In additon, with the enhanced backend and the improved AA, 
kernel argument coercion is no longer required.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89980

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu

Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
+++ /dev/null
@@ -1,113 +0,0 @@
-// REQUIRES: x86-registered-target
-// REQUIRES: amdgpu-registered-target
-
-// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -emit-llvm -x hip %s -o - | FileCheck --check-prefixes=COMMON,CHECK %s
-// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -emit-llvm -x hip %s -disable-O0-optnone -o - | opt -S -O2 | FileCheck %s --check-prefixes=COMMON,OPT
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -x hip %s -o - | FileCheck -check-prefix=HOST %s
-
-#include "Inputs/cuda.h"
-
-// Coerced struct from `struct S` without all generic pointers lowered into
-// global ones.
-// COMMON: %struct.S.coerce = type { i32 addrspace(1)*, float addrspace(1)* }
-// COMMON: %struct.T.coerce = type { [2 x float addrspace(1)*] }
-
-// On the host-side compilation, generic pointer won't be coerced.
-// HOST-NOT: %struct.S.coerce
-// HOST-NOT: %struct.T.coerce
-
-// HOST: define void @_Z22__device_stub__kernel1Pi(i32* %x)
-// COMMON-LABEL: define amdgpu_kernel void @_Z7kernel1Pi(i32 addrspace(1)*{{.*}} %x.coerce)
-// CHECK: = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
-// CHECK-NOT: = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
-// OPT: [[VAL:%.*]] = load i32, i32 addrspace(1)* %x.coerce, align 4
-// OPT: [[INC:%.*]] = add nsw i32 [[VAL]], 1
-// OPT: store i32 [[INC]], i32 addrspace(1)* %x.coerce, align 4
-// OPT: ret void
-__global__ void kernel1(int *x) {
-  x[0]++;
-}
-
-// HOST: define void @_Z22__device_stub__kernel2Ri(i32* nonnull align 4 dereferenceable(4) %x)
-// COMMON-LABEL: define amdgpu_kernel void @_Z7kernel2Ri(i32 addrspace(1)*{{.*}} nonnull align 4 dereferenceable(4) %x.coerce)
-// CHECK: = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
-// CHECK-NOT: = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
-// OPT: [[VAL:%.*]] = load i32, i32 addrspace(1)* %x.coerce, align 4
-// OPT: [[INC:%.*]] = add nsw i32 [[VAL]], 1
-// OPT: store i32 [[INC]], i32 addrspace(1)* %x.coerce, align 4
-// OPT: ret void
-__global__ void kernel2(int ) {
-  x++;
-}
-
-// HOST: define void @_Z22__device_stub__kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
-// CHECK-LABEL: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)*{{.*}} %x, i32 addrspace(1)*{{.*}} %y)
-// CHECK-NOT: = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
-__global__ void kernel3(__attribute__((address_space(2))) int *x,
-__attribute__((address_space(1))) int *y) {
-  y[0] = x[0];
-}
-
-// COMMON-LABEL: define void @_Z4funcPi(i32*{{.*}} %x)
-// CHECK-NOT: = addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to [[TYPE]]*
-__device__ void func(int *x) {
-  x[0]++;
-}
-
-struct S {
-  int *x;
-  float *y;
-};
-// `by-val` struct will be coerced into a similar struct with all generic
-// pointers lowerd into global ones.
-// HOST: define void @_Z22__device_stub__kernel41S(i32* %s.coerce0, float* %s.coerce1)
-// COMMON-LABEL: define amdgpu_kernel void @_Z7kernel41S(%struct.S.coerce %s.coerce)
-// OPT: [[P0:%.*]] = extractvalue %struct.S.coerce %s.coerce, 0
-// OPT: [[P1:%.*]] = extractvalue %struct.S.coerce %s.coerce, 1
-// OPT: [[V0:%.*]] = load i32, i32 addrspace(1)* [[P0]], align 4
-// OPT: [[INC:%.*]] = add nsw i32 [[V0]], 1
-// OPT: store i32 [[INC]], i32 addrspace(1)* [[P0]], align 4
-// OPT: [[V1:%.*]] = load float, float addrspace(1)* [[P1]], align 4
-// OPT: [[ADD:%.*]] = fadd contract float [[V1]], 1.00e+00
-// OPT: store float [[ADD]], float addrspace(1)* [[P1]], align 4
-// OPT: ret void
-__global__ void kernel4(struct S s) {
-  s.x[0]++;
-  s.y[0] += 1.f;
-}
-
-// If a pointer to struct is passed, only the pointer itself is coerced into the global one.
-// HOST: define void @_Z22__device_stub__kernel5P1S(%struct.S* %s)
-// COMMON-LABEL: define amdgpu_kernel void @_Z7kernel5P1S(%struct.S addrspace(1)*{{.*}} %s.coerce)
-__global__ void kernel5(struct S *s) {
-  s->x[0]++;
-  s->y[0] += 1.f;
-}
-
-struct T {
-  float *x[2];
-};
-// `by-val` array is also coerced.
-// HOST: define 

[PATCH] D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC

2020-10-22 Thread Kirill Bobyrev 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 rG68486f9c3a50: [clangd] Get rid of llvm::Optional in Remote- 
and LocalIndexRoot; NFC (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89852

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h

Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -96,8 +96,8 @@
   /// of them can be missing (if the machines are different they don't know each
   /// other's specifics and will only do one-way translation), but both can not
   /// be missing at the same time.
-  llvm::Optional RemoteIndexRoot;
-  llvm::Optional LocalIndexRoot;
+  std::string RemoteIndexRoot;
+  std::string LocalIndexRoot;
   llvm::BumpPtrAllocator Arena;
   llvm::UniqueStringSaver Strings;
 };
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -56,17 +56,19 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 this->RemoteIndexRoot = llvm::sys::path::convert_to_slash(
 RemoteIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->RemoteIndexRoot);
-if (!Path.endswith(PosixSeparator))
-  *this->RemoteIndexRoot += PosixSeparator;
+llvm::StringRef Path(this->RemoteIndexRoot);
+if (!is_separator(this->RemoteIndexRoot.back(),
+  llvm::sys::path::Style::posix))
+  this->RemoteIndexRoot += PosixSeparator;
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 this->LocalIndexRoot = llvm::sys::path::convert_to_slash(
 LocalIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->LocalIndexRoot);
-if (!Path.endswith(PosixSeparator))
-  *this->LocalIndexRoot += PosixSeparator;
+llvm::StringRef Path(this->LocalIndexRoot);
+if (!is_separator(this->LocalIndexRoot.back(),
+  llvm::sys::path::Style::posix))
+  this->LocalIndexRoot += PosixSeparator;
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
@@ -83,7 +85,7 @@
 
 llvm::Expected
 Marshaller::fromProtobuf(const v1::FuzzyFindRequest *Message) {
-  assert(RemoteIndexRoot);
+  assert(!RemoteIndexRoot.empty());
   clangd::FuzzyFindRequest Result;
   Result.Query = Message->query();
   for (const auto  : Message->scopes())
@@ -93,7 +95,7 @@
 Result.Limit = Message->limit();
   Result.RestrictForCodeCompletion = Message->restricted_for_code_completion();
   for (const auto  : Message->proximity_paths()) {
-llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
+llvm::SmallString<256> LocalPath = llvm::StringRef(RemoteIndexRoot);
 llvm::sys::path::append(LocalPath, Path);
 // FuzzyFindRequest requires proximity paths to have platform-native format
 // in order for SymbolIndex to process the query correctly.
@@ -204,7 +206,7 @@
 
 v1::FuzzyFindRequest
 Marshaller::toProtobuf(const clangd::FuzzyFindRequest ) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   v1::FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto  : From.Scopes)
@@ -215,7 +217,7 @@
   RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto  : From.ProximityPaths) {
 llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
-if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot, ""))
+if (llvm::sys::path::replace_path_prefix(RelativePath, LocalIndexRoot, ""))
   RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
   RelativePath, llvm::sys::path::Style::windows));
   }
@@ -305,20 +307,20 @@
 
 llvm::Expected
 Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
   if (RelativePath.empty())
 return error("Empty relative path.");
   if (llvm::sys::path::is_absolute(RelativePath, llvm::sys::path::Style::posix))
 return error("RelativePath '{0}' is absolute.", RelativePath);
-  llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
+  llvm::SmallString<256> FullPath = llvm::StringRef(LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);
   auto Result = 

[clang-tools-extra] 68486f9 - [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC

2020-10-22 Thread Kirill Bobyrev via cfe-commits

Author: Kirill Bobyrev
Date: 2020-10-22T21:47:48+02:00
New Revision: 68486f9c3a509cdb23451a488f16c530919bab70

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

LOG: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp 
b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index 23ed67653b8b..ef7f77db83ca 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -56,17 +56,19 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 this->RemoteIndexRoot = llvm::sys::path::convert_to_slash(
 RemoteIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->RemoteIndexRoot);
-if (!Path.endswith(PosixSeparator))
-  *this->RemoteIndexRoot += PosixSeparator;
+llvm::StringRef Path(this->RemoteIndexRoot);
+if (!is_separator(this->RemoteIndexRoot.back(),
+  llvm::sys::path::Style::posix))
+  this->RemoteIndexRoot += PosixSeparator;
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 this->LocalIndexRoot = llvm::sys::path::convert_to_slash(
 LocalIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->LocalIndexRoot);
-if (!Path.endswith(PosixSeparator))
-  *this->LocalIndexRoot += PosixSeparator;
+llvm::StringRef Path(this->LocalIndexRoot);
+if (!is_separator(this->LocalIndexRoot.back(),
+  llvm::sys::path::Style::posix))
+  this->LocalIndexRoot += PosixSeparator;
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
@@ -83,7 +85,7 @@ Marshaller::fromProtobuf(const v1::LookupRequest *Message) {
 
 llvm::Expected
 Marshaller::fromProtobuf(const v1::FuzzyFindRequest *Message) {
-  assert(RemoteIndexRoot);
+  assert(!RemoteIndexRoot.empty());
   clangd::FuzzyFindRequest Result;
   Result.Query = Message->query();
   for (const auto  : Message->scopes())
@@ -93,7 +95,7 @@ Marshaller::fromProtobuf(const v1::FuzzyFindRequest *Message) 
{
 Result.Limit = Message->limit();
   Result.RestrictForCodeCompletion = Message->restricted_for_code_completion();
   for (const auto  : Message->proximity_paths()) {
-llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
+llvm::SmallString<256> LocalPath = llvm::StringRef(RemoteIndexRoot);
 llvm::sys::path::append(LocalPath, Path);
 // FuzzyFindRequest requires proximity paths to have platform-native format
 // in order for SymbolIndex to process the query correctly.
@@ -204,7 +206,7 @@ v1::LookupRequest Marshaller::toProtobuf(const 
clangd::LookupRequest ) {
 
 v1::FuzzyFindRequest
 Marshaller::toProtobuf(const clangd::FuzzyFindRequest ) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   v1::FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto  : From.Scopes)
@@ -215,7 +217,7 @@ Marshaller::toProtobuf(const clangd::FuzzyFindRequest 
) {
   
RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto  : From.ProximityPaths) {
 llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
-if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot, 
""))
+if (llvm::sys::path::replace_path_prefix(RelativePath, LocalIndexRoot, ""))
   RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
   RelativePath, llvm::sys::path::Style::windows));
   }
@@ -305,20 +307,20 @@ Marshaller::toProtobuf(const clangd::SymbolID ,
 
 llvm::Expected
 Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
   if (RelativePath.empty())
 return error("Empty relative path.");
   if (llvm::sys::path::is_absolute(RelativePath, 
llvm::sys::path::Style::posix))
 return error("RelativePath '{0}' is absolute.", RelativePath);
-  llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
+  llvm::SmallString<256> FullPath = llvm::StringRef(LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);
   auto Result = URI::createFile(FullPath);
   return Result.toString();
 }
 
 llvm::Expected Marshaller::uriToRelativePath(llvm::StringRef 

[PATCH] D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC

2020-10-22 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 300080.
kbobyrev added a comment.

Rebase on top of master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89852

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h

Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -96,8 +96,8 @@
   /// of them can be missing (if the machines are different they don't know each
   /// other's specifics and will only do one-way translation), but both can not
   /// be missing at the same time.
-  llvm::Optional RemoteIndexRoot;
-  llvm::Optional LocalIndexRoot;
+  std::string RemoteIndexRoot;
+  std::string LocalIndexRoot;
   llvm::BumpPtrAllocator Arena;
   llvm::UniqueStringSaver Strings;
 };
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -56,17 +56,19 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 this->RemoteIndexRoot = llvm::sys::path::convert_to_slash(
 RemoteIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->RemoteIndexRoot);
-if (!Path.endswith(PosixSeparator))
-  *this->RemoteIndexRoot += PosixSeparator;
+llvm::StringRef Path(this->RemoteIndexRoot);
+if (!is_separator(this->RemoteIndexRoot.back(),
+  llvm::sys::path::Style::posix))
+  this->RemoteIndexRoot += PosixSeparator;
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 this->LocalIndexRoot = llvm::sys::path::convert_to_slash(
 LocalIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->LocalIndexRoot);
-if (!Path.endswith(PosixSeparator))
-  *this->LocalIndexRoot += PosixSeparator;
+llvm::StringRef Path(this->LocalIndexRoot);
+if (!is_separator(this->LocalIndexRoot.back(),
+  llvm::sys::path::Style::posix))
+  this->LocalIndexRoot += PosixSeparator;
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
@@ -83,7 +85,7 @@
 
 llvm::Expected
 Marshaller::fromProtobuf(const v1::FuzzyFindRequest *Message) {
-  assert(RemoteIndexRoot);
+  assert(!RemoteIndexRoot.empty());
   clangd::FuzzyFindRequest Result;
   Result.Query = Message->query();
   for (const auto  : Message->scopes())
@@ -93,7 +95,7 @@
 Result.Limit = Message->limit();
   Result.RestrictForCodeCompletion = Message->restricted_for_code_completion();
   for (const auto  : Message->proximity_paths()) {
-llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
+llvm::SmallString<256> LocalPath = llvm::StringRef(RemoteIndexRoot);
 llvm::sys::path::append(LocalPath, Path);
 // FuzzyFindRequest requires proximity paths to have platform-native format
 // in order for SymbolIndex to process the query correctly.
@@ -204,7 +206,7 @@
 
 v1::FuzzyFindRequest
 Marshaller::toProtobuf(const clangd::FuzzyFindRequest ) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   v1::FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto  : From.Scopes)
@@ -215,7 +217,7 @@
   RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto  : From.ProximityPaths) {
 llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
-if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot, ""))
+if (llvm::sys::path::replace_path_prefix(RelativePath, LocalIndexRoot, ""))
   RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
   RelativePath, llvm::sys::path::Style::windows));
   }
@@ -305,20 +307,20 @@
 
 llvm::Expected
 Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
   if (RelativePath.empty())
 return error("Empty relative path.");
   if (llvm::sys::path::is_absolute(RelativePath, llvm::sys::path::Style::posix))
 return error("RelativePath '{0}' is absolute.", RelativePath);
-  llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
+  llvm::SmallString<256> FullPath = llvm::StringRef(LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);
   auto Result = URI::createFile(FullPath);
   return Result.toString();
 }
 
 llvm::Expected Marshaller::uriToRelativePath(llvm::StringRef URI) {
-  assert(RemoteIndexRoot);
+  

[PATCH] D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC

2020-10-22 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 300079.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Resolve post-LGTM comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89852

Files:
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h

Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -95,8 +95,8 @@
   /// of them can be missing (if the machines are different they don't know each
   /// other's specifics and will only do one-way translation), but both can not
   /// be missing at the same time.
-  llvm::Optional RemoteIndexRoot;
-  llvm::Optional LocalIndexRoot;
+  std::string RemoteIndexRoot;
+  std::string LocalIndexRoot;
   llvm::BumpPtrAllocator Arena;
   llvm::UniqueStringSaver Strings;
 };
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -56,17 +56,19 @@
 assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
 this->RemoteIndexRoot = llvm::sys::path::convert_to_slash(
 RemoteIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->RemoteIndexRoot);
-if (!Path.endswith(PosixSeparator))
-  *this->RemoteIndexRoot += PosixSeparator;
+llvm::StringRef Path(this->RemoteIndexRoot);
+if (!is_separator(this->RemoteIndexRoot.back(),
+  llvm::sys::path::Style::posix))
+  this->RemoteIndexRoot += PosixSeparator;
   }
   if (!LocalIndexRoot.empty()) {
 assert(llvm::sys::path::is_absolute(LocalIndexRoot));
 this->LocalIndexRoot = llvm::sys::path::convert_to_slash(
 LocalIndexRoot, llvm::sys::path::Style::windows);
-llvm::StringRef Path(*this->LocalIndexRoot);
-if (!Path.endswith(PosixSeparator))
-  *this->LocalIndexRoot += PosixSeparator;
+llvm::StringRef Path(this->LocalIndexRoot);
+if (!is_separator(this->LocalIndexRoot.back(),
+  llvm::sys::path::Style::posix))
+  this->LocalIndexRoot += PosixSeparator;
   }
   assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
 }
@@ -83,7 +85,7 @@
 
 llvm::Expected
 Marshaller::fromProtobuf(const FuzzyFindRequest *Message) {
-  assert(RemoteIndexRoot);
+  assert(!RemoteIndexRoot.empty());
   clangd::FuzzyFindRequest Result;
   Result.Query = Message->query();
   for (const auto  : Message->scopes())
@@ -93,7 +95,7 @@
 Result.Limit = Message->limit();
   Result.RestrictForCodeCompletion = Message->restricted_for_code_completion();
   for (const auto  : Message->proximity_paths()) {
-llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
+llvm::SmallString<256> LocalPath = llvm::StringRef(RemoteIndexRoot);
 llvm::sys::path::append(LocalPath, Path);
 // FuzzyFindRequest requires proximity paths to have platform-native format
 // in order for SymbolIndex to process the query correctly.
@@ -202,7 +204,7 @@
 }
 
 FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest ) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto  : From.Scopes)
@@ -213,7 +215,7 @@
   RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto  : From.ProximityPaths) {
 llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
-if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot, ""))
+if (llvm::sys::path::replace_path_prefix(RelativePath, LocalIndexRoot, ""))
   RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
   RelativePath, llvm::sys::path::Style::windows));
   }
@@ -301,20 +303,20 @@
 
 llvm::Expected
 Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
-  assert(LocalIndexRoot);
+  assert(!LocalIndexRoot.empty());
   assert(RelativePath == llvm::sys::path::convert_to_slash(RelativePath));
   if (RelativePath.empty())
 return error("Empty relative path.");
   if (llvm::sys::path::is_absolute(RelativePath, llvm::sys::path::Style::posix))
 return error("RelativePath '{0}' is absolute.", RelativePath);
-  llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
+  llvm::SmallString<256> FullPath = llvm::StringRef(LocalIndexRoot);
   llvm::sys::path::append(FullPath, RelativePath);
   auto Result = URI::createFile(FullPath);
   return Result.toString();
 }
 
 llvm::Expected Marshaller::uriToRelativePath(llvm::StringRef URI) {
-  

[clang] e787022 - [SourceManager] Skip module maps when searching files for macro arguments

2020-10-22 Thread Jan Korous via cfe-commits

Author: Jan Korous
Date: 2020-10-22T12:36:51-07:00
New Revision: e7870223d8b586cd89ffc7fddd63bc40447bd3fb

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

LOG: [SourceManager] Skip module maps when searching files for macro arguments

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

Added: 


Modified: 
clang/lib/Basic/SourceManager.cpp

Removed: 




diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index 9592b92f59bc..3bffcb548ebb 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -1761,7 +1761,12 @@ void SourceManager::computeMacroArgsCache(MacroArgsMap 
,
 if (Invalid)
   return;
 if (Entry.isFile()) {
-  SourceLocation IncludeLoc = Entry.getFile().getIncludeLoc();
+  auto File = Entry.getFile();
+  if (File.getFileCharacteristic() == C_User_ModuleMap ||
+  File.getFileCharacteristic() == C_System_ModuleMap)
+continue;
+
+  SourceLocation IncludeLoc = File.getIncludeLoc();
   bool IncludedInFID =
   (IncludeLoc.isValid() && isInFileID(IncludeLoc, FID)) ||
   // Predefined header doesn't have a valid include location in main



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


[PATCH] D86230: [SourceManager] Skip module maps when searching files for macro arguments

2020-10-22 Thread Jan Korous via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe7870223d8b5: [SourceManager] Skip module maps when 
searching files for macro arguments (authored by jkorous).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D86230?vs=292928=300076#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86230

Files:
  clang/lib/Basic/SourceManager.cpp


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1761,7 +1761,12 @@
 if (Invalid)
   return;
 if (Entry.isFile()) {
-  SourceLocation IncludeLoc = Entry.getFile().getIncludeLoc();
+  auto File = Entry.getFile();
+  if (File.getFileCharacteristic() == C_User_ModuleMap ||
+  File.getFileCharacteristic() == C_System_ModuleMap)
+continue;
+
+  SourceLocation IncludeLoc = File.getIncludeLoc();
   bool IncludedInFID =
   (IncludeLoc.isValid() && isInFileID(IncludeLoc, FID)) ||
   // Predefined header doesn't have a valid include location in main


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1761,7 +1761,12 @@
 if (Invalid)
   return;
 if (Entry.isFile()) {
-  SourceLocation IncludeLoc = Entry.getFile().getIncludeLoc();
+  auto File = Entry.getFile();
+  if (File.getFileCharacteristic() == C_User_ModuleMap ||
+  File.getFileCharacteristic() == C_System_ModuleMap)
+continue;
+
+  SourceLocation IncludeLoc = File.getIncludeLoc();
   bool IncludedInFID =
   (IncludeLoc.isValid() && isInFileID(IncludeLoc, FID)) ||
   // Predefined header doesn't have a valid include location in main
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D89974#2348181 , @Hahnfeld wrote:

> Don't we already have this via `PATH`? At least that was my motivation back 
> then and it worked without problems.

We do and it should indeed work for the tests. Setting an environment variable 
should be reasonably low burden if/when default path is not good enough.

PATH does suffer from the problem that ptxas location is not necessarily the 
right CUDA location. I.e. sometimes it's found via /usr/bin which I can not 
drop from the PATH, so one should be careful to *prepend* desired ptxas 
location.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

2020-10-22 Thread Kirill Bobyrev via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG81e5f298c431: [clangd] Give the server information about 
clients remote index protocol… (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/Index.proto
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -133,7 +133,7 @@
 
   // Paths transmitted over the wire can not be absolute, they have to be
   // relative.
-  Ref WithAbsolutePath;
+  v1::Ref WithAbsolutePath;
   *WithAbsolutePath.mutable_location()->mutable_file_path() =
   "/usr/local/user/home/HelloWorld.cpp";
   Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath);
@@ -282,7 +282,7 @@
   Sym.IncludeHeaders.pop_back();
   Serialized = ProtobufMarshaller.toProtobuf(Sym);
   ASSERT_TRUE(bool(Serialized));
-  HeaderWithReferences InvalidHeader;
+  v1::HeaderWithReferences InvalidHeader;
   InvalidHeader.set_header(convert_to_slash("/absolute/path/Header.h"));
   InvalidHeader.set_references(9000);
   *Serialized->add_headers() = InvalidHeader;
@@ -388,7 +388,7 @@
 }
 
 TEST(RemoteMarshallingTest, RelationsRequestFailingSerialization) {
-  RelationsRequest Serialized;
+  v1::RelationsRequest Serialized;
   Serialized.add_subjects("");
   Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
   auto Deserialized = ProtobufMarshaller.fromProtobuf();
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -38,32 +38,33 @@
   Marshaller() = delete;
   Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot);
 
-  llvm::Expected fromProtobuf(const Symbol );
-  llvm::Expected fromProtobuf(const Ref );
+  llvm::Expected fromProtobuf(const v1::Symbol );
+  llvm::Expected fromProtobuf(const v1::Ref );
   llvm::Expected>
-  fromProtobuf(const Relation );
+  fromProtobuf(const v1::Relation );
 
   llvm::Expected
-  fromProtobuf(const LookupRequest *Message);
+  fromProtobuf(const v1::LookupRequest *Message);
   llvm::Expected
-  fromProtobuf(const FuzzyFindRequest *Message);
-  llvm::Expected fromProtobuf(const RefsRequest *Message);
+  fromProtobuf(const v1::FuzzyFindRequest *Message);
+  llvm::Expected
+  fromProtobuf(const v1::RefsRequest *Message);
   llvm::Expected
-  fromProtobuf(const RelationsRequest *Message);
+  fromProtobuf(const v1::RelationsRequest *Message);
 
   /// toProtobuf() functions serialize native clangd types and strip IndexRoot
   /// from the file paths specific to indexing machine. fromProtobuf() functions
   /// deserialize clangd types and translate relative paths into machine-native
   /// URIs.
-  LookupRequest toProtobuf(const clangd::LookupRequest );
-  FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest );
-  RefsRequest toProtobuf(const clangd::RefsRequest );
-  RelationsRequest toProtobuf(const clangd::RelationsRequest );
+  v1::LookupRequest toProtobuf(const clangd::LookupRequest );
+  v1::FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest );
+  v1::RefsRequest toProtobuf(const clangd::RefsRequest );
+  v1::RelationsRequest toProtobuf(const clangd::RelationsRequest );
 
-  llvm::Expected toProtobuf(const clangd::Symbol );
-  llvm::Expected toProtobuf(const clangd::Ref );
-  llvm::Expected toProtobuf(const clangd::SymbolID ,
-  const clangd::Symbol );
+  llvm::Expected toProtobuf(const clangd::Symbol );
+  llvm::Expected toProtobuf(const clangd::Ref );
+  llvm::Expected toProtobuf(const clangd::SymbolID ,
+  const clangd::Symbol );
 
   /// Translates \p RelativePath into the absolute path and builds URI for the
   /// user machine. This translation happens on the client side with the
@@ -77,18 +78,18 @@
   llvm::Expected uriToRelativePath(llvm::StringRef URI);
 
 private:
-  clangd::SymbolLocation::Position fromProtobuf(const Position );
-  Position toProtobuf(const clangd::SymbolLocation::Position );
-  clang::index::SymbolInfo fromProtobuf(const SymbolInfo );
-  SymbolInfo toProtobuf(const 

[clang-tools-extra] 81e5f29 - [clangd] Give the server information about client's remote index protocol version

2020-10-22 Thread Kirill Bobyrev via cfe-commits

Author: Kirill Bobyrev
Date: 2020-10-22T21:35:18+02:00
New Revision: 81e5f298c431555d809f898c196945ca879c1150

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

LOG: [clangd] Give the server information about client's remote index protocol 
version

And also introduce Protobuf package versioning, it will help to deal
with breaking changes. Inroducing package version itself is a breaking
change, clients and servers need to be updated.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/index/remote/Client.cpp
clang-tools-extra/clangd/index/remote/Index.proto
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/remote/Client.cpp 
b/clang-tools-extra/clangd/index/remote/Client.cpp
index 131e4c0b2fce..d01dd4483974 100644
--- a/clang-tools-extra/clangd/index/remote/Client.cpp
+++ b/clang-tools-extra/clangd/index/remote/Client.cpp
@@ -15,6 +15,7 @@
 #include "marshalling/Marshalling.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
@@ -28,7 +29,8 @@ namespace {
 class IndexClient : public clangd::SymbolIndex {
   template 
   using StreamingCall = std::unique_ptr> (
-  remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &);
+  remote::v1::SymbolIndex::Stub::*)(grpc::ClientContext *,
+const RequestT &);
 
   template 
@@ -40,6 +42,7 @@ class IndexClient : public clangd::SymbolIndex {
 const auto RPCRequest = ProtobufMarshaller->toProtobuf(Request);
 SPAN_ATTACH(Tracer, "Request", RPCRequest.DebugString());
 grpc::ClientContext Context;
+Context.AddMetadata("version", clang::getClangToolFullVersion("clangd"));
 std::chrono::system_clock::time_point Deadline =
 std::chrono::system_clock::now() + DeadlineWaitingTime;
 Context.set_deadline(Deadline);
@@ -73,7 +76,7 @@ class IndexClient : public clangd::SymbolIndex {
   IndexClient(
   std::shared_ptr Channel, llvm::StringRef ProjectRoot,
   std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
-  : Stub(remote::SymbolIndex::NewStub(Channel)),
+  : Stub(remote::v1::SymbolIndex::NewStub(Channel)),
 ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
   /*LocalIndexRoot=*/ProjectRoot)),
 DeadlineWaitingTime(DeadlineTime) {
@@ -82,25 +85,26 @@ class IndexClient : public clangd::SymbolIndex {
 
   void lookup(const clangd::LookupRequest ,
   llvm::function_ref Callback) const 
{
-streamRPC(Request, ::SymbolIndex::Stub::Lookup, Callback);
+streamRPC(Request, ::v1::SymbolIndex::Stub::Lookup, Callback);
   }
 
   bool
   fuzzyFind(const clangd::FuzzyFindRequest ,
 llvm::function_ref Callback) const {
-return streamRPC(Request, ::SymbolIndex::Stub::FuzzyFind, Callback);
+return streamRPC(Request, ::v1::SymbolIndex::Stub::FuzzyFind,
+ Callback);
   }
 
   bool refs(const clangd::RefsRequest ,
 llvm::function_ref Callback) const {
-return streamRPC(Request, ::SymbolIndex::Stub::Refs, Callback);
+return streamRPC(Request, ::v1::SymbolIndex::Stub::Refs, Callback);
   }
 
   void
   relations(const clangd::RelationsRequest ,
 llvm::function_ref
 Callback) const {
-streamRPC(Request, ::SymbolIndex::Stub::Relations,
+streamRPC(Request, ::v1::SymbolIndex::Stub::Relations,
   // Unpack protobuf Relation.
   [&](std::pair SubjectAndObject) {
 Callback(SubjectAndObject.first, SubjectAndObject.second);
@@ -112,7 +116,7 @@ class IndexClient : public clangd::SymbolIndex {
   size_t estimateMemoryUsage() const { return 0; }
 
 private:
-  std::unique_ptr Stub;
+  std::unique_ptr Stub;
   std::unique_ptr ProtobufMarshaller;
   // Each request will be terminated if it takes too long.
   std::chrono::milliseconds DeadlineWaitingTime;

diff  --git a/clang-tools-extra/clangd/index/remote/Index.proto 
b/clang-tools-extra/clangd/index/remote/Index.proto
index 305164ffef77..34c49756b1da 100644
--- a/clang-tools-extra/clangd/index/remote/Index.proto
+++ b/clang-tools-extra/clangd/index/remote/Index.proto
@@ -8,7 +8,7 @@
 
 syntax = "proto3";
 
-package clang.clangd.remote;
+package clang.clangd.remote.v1;
 
 // Semantics of SymbolIndex match clangd::SymbolIndex with all required
 // structures corresponding to their clangd::* 

[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

2020-10-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall requested changes to this revision.
sammccall added a comment.
This revision now requires changes to proceed.

In D89862#2348173 , @kbobyrev wrote:

> Add package versioning and make current version v1.

Sorry, I didn't mean in this patch - seems not that related up to you 
whether to tie them together.

I don't think we need to put the actual messages into versioned namespaces, it 
makes a mess of the code. (We may need to do this later for some messages if we 
actually make use of this versioning, but we should pay for it if we use it - 
moving messages between namespaces is a backwards-compatible change)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

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


[PATCH] D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`

2020-10-22 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_cmath.h:330
 double>::type
 remquo(__T1 __x, __T2 __y, int *__quo) {
   return std::remquo((double)__x, (double)__y, __quo);

tra wrote:
> jdoerfert wrote:
> > The template overload I was talking about is this one.
> Ah. It makes more sense now.
> 
> The patch description may need to be rephrased. You've added a function 
> overload, but the description talks about the template which is *not* in the 
> patch.
> Something like `Added function overload for remquo(float...)...   the 
> template function overload for other types... is still behind OPENMP_NVPTX`
Will do:

Not affected by this patch is the `remquo` template overload which is
still hidden behind an ifdef for OpenMP. In the future we probably want
to remove the ifdef but that requires further testing.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89971

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


[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

2020-10-22 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:1020-1023
+// Resume the default rounding and exception modes.
+NewFPFeatures.setRoundingModeOverride(
+llvm::RoundingMode::NearestTiesToEven);
+NewFPFeatures.setFPExceptionModeOverride(LangOptions::FPE_Ignore);

sepavloff wrote:
> The previous values of rounding mode and exception behavior may differ from 
> the default values. For example, `#pragma STDC FENV_ROUND` may set constant 
> rounding direction different from FE_TONEAREST`. Similarly, command line 
> options may set exception behavior different from `ignore`.
> Can pragma stack be used for this?
> The previous values of rounding mode and exception behavior may differ from 
> the default values. For example, `#pragma STDC FENV_ROUND` may set constant 
> rounding direction different from FE_TONEAREST`. Similarly, command line 
> options may set exception behavior different from `ignore`.
> Can pragma stack be used for this?

I guess I could just NewFPFeatures.setAllowFEnvAccessOverride(false); and leave 
the other 2 unset, does that sound right? The values of the FPFeatures are 
being preserved around compound statements with FPFeaturesStateRAII


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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


[PATCH] D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_cmath.h:330
 double>::type
 remquo(__T1 __x, __T2 __y, int *__quo) {
   return std::remquo((double)__x, (double)__y, __quo);

jdoerfert wrote:
> The template overload I was talking about is this one.
Ah. It makes more sense now.

The patch description may need to be rephrased. You've added a function 
overload, but the description talks about the template which is *not* in the 
patch.
Something like `Added function overload for remquo(float...)...   the template 
function overload for other types... is still behind OPENMP_NVPTX`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89971

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


[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment.

In D89974#2348176 , @tra wrote:

> CUDA path is sort of a global configuration parameter for all CUDA 
> compilations. Perhaps we should consider allowing the user to specify a CUDA 
> search path candidate via environment variable. This should allow 
> transparently overriding preferred CUDA path without having to adjust all 
> builds. I can't say I like it, but it seems to be the least bad way (that I 
> can think of ATM) to address the dependency on something that only the end 
> user would know for sure.

Don't we already have this via `PATH`? At least that was my motivation back 
then and it worked without problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

> Shortcomings in the testrunner don't seem like a reason to introduce new 
> build-time configured default search paths into the driver.

+1

The root of the problem is that we rely on CUDA SDK as the external dependency 
and we have no way to reliably predict where the user will put it.
Generally speaking, the location of that dependency is specific to particular 
machine. I.e. I can have CUDA installed pretty much anywhere I want. Different 
distributions may install it anywhere they want. There may be multiple versions 
installed and clang has no basis to guess which one the user intends to use.

I do not see a way to make it work reliably without explicit input from the end 
user. Making the heuristic dependent on clang's build environment is not 
improving the situation, IMO. It may help some users, but it will disrupt 
others and it will introduce more confusion about where specific clang binary 
searches for CUDA installations.

CUDA path is sort of a global configuration parameter for all CUDA 
compilations. Perhaps we should consider allowing the user to specify a CUDA 
search path candidate via environment variable. This should allow transparently 
overriding preferred CUDA path without having to adjust all builds. I can't say 
I like it, but it seems to be the least bad way (that I can think of ATM) to 
address the dependency on something that only the end user would know for sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D89862: [clangd] Give the server information about client's remote index protocol version

2020-10-22 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 300066.
kbobyrev added a comment.

Add package versioning and make current version v1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89862

Files:
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/Index.proto
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
  clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
  clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -133,7 +133,7 @@
 
   // Paths transmitted over the wire can not be absolute, they have to be
   // relative.
-  Ref WithAbsolutePath;
+  v1::Ref WithAbsolutePath;
   *WithAbsolutePath.mutable_location()->mutable_file_path() =
   "/usr/local/user/home/HelloWorld.cpp";
   Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath);
@@ -282,7 +282,7 @@
   Sym.IncludeHeaders.pop_back();
   Serialized = ProtobufMarshaller.toProtobuf(Sym);
   ASSERT_TRUE(bool(Serialized));
-  HeaderWithReferences InvalidHeader;
+  v1::HeaderWithReferences InvalidHeader;
   InvalidHeader.set_header(convert_to_slash("/absolute/path/Header.h"));
   InvalidHeader.set_references(9000);
   *Serialized->add_headers() = InvalidHeader;
@@ -388,7 +388,7 @@
 }
 
 TEST(RemoteMarshallingTest, RelationsRequestFailingSerialization) {
-  RelationsRequest Serialized;
+  v1::RelationsRequest Serialized;
   Serialized.add_subjects("");
   Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
   auto Deserialized = ProtobufMarshaller.fromProtobuf();
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -38,32 +38,33 @@
   Marshaller() = delete;
   Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot);
 
-  llvm::Expected fromProtobuf(const Symbol );
-  llvm::Expected fromProtobuf(const Ref );
+  llvm::Expected fromProtobuf(const v1::Symbol );
+  llvm::Expected fromProtobuf(const v1::Ref );
   llvm::Expected>
-  fromProtobuf(const Relation );
+  fromProtobuf(const v1::Relation );
 
   llvm::Expected
-  fromProtobuf(const LookupRequest *Message);
+  fromProtobuf(const v1::LookupRequest *Message);
   llvm::Expected
-  fromProtobuf(const FuzzyFindRequest *Message);
-  llvm::Expected fromProtobuf(const RefsRequest *Message);
+  fromProtobuf(const v1::FuzzyFindRequest *Message);
+  llvm::Expected
+  fromProtobuf(const v1::RefsRequest *Message);
   llvm::Expected
-  fromProtobuf(const RelationsRequest *Message);
+  fromProtobuf(const v1::RelationsRequest *Message);
 
   /// toProtobuf() functions serialize native clangd types and strip IndexRoot
   /// from the file paths specific to indexing machine. fromProtobuf() functions
   /// deserialize clangd types and translate relative paths into machine-native
   /// URIs.
-  LookupRequest toProtobuf(const clangd::LookupRequest );
-  FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest );
-  RefsRequest toProtobuf(const clangd::RefsRequest );
-  RelationsRequest toProtobuf(const clangd::RelationsRequest );
+  v1::LookupRequest toProtobuf(const clangd::LookupRequest );
+  v1::FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest );
+  v1::RefsRequest toProtobuf(const clangd::RefsRequest );
+  v1::RelationsRequest toProtobuf(const clangd::RelationsRequest );
 
-  llvm::Expected toProtobuf(const clangd::Symbol );
-  llvm::Expected toProtobuf(const clangd::Ref );
-  llvm::Expected toProtobuf(const clangd::SymbolID ,
-  const clangd::Symbol );
+  llvm::Expected toProtobuf(const clangd::Symbol );
+  llvm::Expected toProtobuf(const clangd::Ref );
+  llvm::Expected toProtobuf(const clangd::SymbolID ,
+  const clangd::Symbol );
 
   /// Translates \p RelativePath into the absolute path and builds URI for the
   /// user machine. This translation happens on the client side with the
@@ -77,18 +78,18 @@
   llvm::Expected uriToRelativePath(llvm::StringRef URI);
 
 private:
-  clangd::SymbolLocation::Position fromProtobuf(const Position );
-  Position toProtobuf(const clangd::SymbolLocation::Position );
-  clang::index::SymbolInfo fromProtobuf(const SymbolInfo );
-  SymbolInfo toProtobuf(const clang::index::SymbolInfo );
+  clangd::SymbolLocation::Position fromProtobuf(const v1::Position );
+  v1::Position toProtobuf(const clangd::SymbolLocation::Position );
+  clang::index::SymbolInfo fromProtobuf(const v1::SymbolInfo 

[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

2020-10-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

In D82756#2348159 , @dexonsmith wrote:

> I have an idea: use `DEFAULT_VALUE` to keep current behaviour. Here's an 
> example to demonstrate.

Downside of this is that it's error prone, because it requires for correctness 
(but does not enforce) a particular option definition order. The alternative 
would be to formalize it in the options somehow.


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

https://reviews.llvm.org/D82756

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


[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

2020-10-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

I have an idea: use `DEFAULT_VALUE` to keep current behaviour. Here's an 
example to demonstrate.

`cl_mad_enable` is implied by `OPT_cl_unsafe_math_optimizations` or 
`OPT_cl_fast_relaxed_math`. Instead of setting the default value to `"false"` 
it could be set to `"CodeGenOpts.CLUnsafeMath || LangOpts.FastRelaxedMath"` 
(one hitch is that `CLUnsafeMath` doesn't currently exist).

You'd need to update the `OPTION_WITH_MARSHALLING_FLAG` definition in 
`parseSimpleArgs` to something like:

  #define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP,  
 \
   ALIAS, ALIASARGS, FLAGS, PARAM, 
HELPTEXT, \
   METAVAR, VALUES, SPELLING, ALWAYS_EMIT,  
 \
   KEYPATH, DEFAULT_VALUE, IS_POSITIVE) 
 \
this->KEYPATH = (Args.hasArg(OPT_##ID) && IS_POSITIVE) || (DEFAULT_VALUE);

@Bigcheese / @jansvoboda11 , WDYT?


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

https://reviews.llvm.org/D82756

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


[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment.

In D89974#2348124 , @jdoerfert wrote:

> I thought, right now we would configure clang with a cuda path XYZ

If you speak of the current state in the repository, AFAICT, Clang doesn't know 
about a CUDA installation that CMake may have found. There's just a list of 
default paths that are searched during runtime, dynamically enhanced by an 
entry from searching `ptxas` in `PATH`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`

2020-10-22 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D89971#2347909 , @tra wrote:

>> The template overload
>
> function overload.
>
>> is still hidden behind an ifdef for OpenMP. In the
>> future we probably want to remove the ifdef but that requires further
>> testing.
>
> I don't think it's the case.  I've just ran `clang++ -x cuda /dev/null  
> --cuda-host-only -dD -E ` and I do see the definitions for `tanh` just above 
> your change.

My comment was about the template I point out in the inline comment below. Also 
I was talking about the OpenMP declare variant overloads, not the cuda 
definitions.
TBH, I don't understand what you want to tell me with the remark about the cuda 
definitions and `tanh`. Nothing the OpenMP stuff does should impact cuda and 
tanh was there already, I'm confused :(




Comment at: clang/lib/Headers/__clang_cuda_cmath.h:330
 double>::type
 remquo(__T1 __x, __T2 __y, int *__quo) {
   return std::remquo((double)__x, (double)__y, __quo);

The template overload I was talking about is this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89971

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


[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

> I think the default should still let clang search for CUDA or require the 
> user to provide correct CUDA path. "Use CUDA path discovered by CMake at 
> build time" should be a non-default configuration option if/when it's needed 
> and appropriate.



> Having an option for a vendor to append/prepend a toolkit search location 
> seems useful, but currently this seems more like a workaround for an explicit 
> CUDA toolkit path not being passed to the testsuite.
> Shortcomings in the testrunner don't seem like a reason to introduce new 
> build-time configured default search paths into the driver.

I thought, right now we would configure clang with a cuda path XYZ and the 
clang we build still will lookup the default path instead. This requires you to 
specify XYZ when compiling/linking as well in order to not accidentally pick up 
a different cuda installation (or none at all). Maybe I misunderstood but if I 
didn't, the behavior we have right now seems wrong or at least incomplete. I 
should be able to build a clang on a system with a non-default cuda 
installation that will work "out-of-the-box" for users as it has the cuda path 
"backed-in". Again, I might misunderstand all this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D89852: [clangd] Get rid of llvm::Optional in Remote- and LocalIndexRoot; NFC

2020-10-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:60
+llvm::StringRef Path(this->RemoteIndexRoot);
 if (!Path.endswith(PosixSeparator))
+  this->RemoteIndexRoot += PosixSeparator;

nit: `if(!is_separator(this->RemoteIndexRoot.back(), posix))` same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89852

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


[PATCH] D89935: [clangd] NFC: Update FIXME comment regarding lack of c/dtor support

2020-10-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

> I thought the point of the comment was us not handling it properly rather 
> than LSP not supporting it (e.g. LSP does support Operator but we do not). 
> Then, the comment about ctor and dtor being indistinguishable probably 
> belongs to Protocol.h/cpp and SymbolKind there in particular?

If that was the case I would expect this comment to be above previous line, 
i.e. `index::getSymbolInfo`, as that's the one doing the clang-level mapping. 
Whereas `indexSymbolKindToSymbolKind` does LSP mapping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89935

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


[PATCH] D89832: [CUDA] Extract CUDA version from cuda.h if version.txt is not found

2020-10-22 Thread Evgeny Mankov via Phabricator via cfe-commits
emankov accepted this revision.
emankov added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:76-77
+return CudaVersion::CUDA_102;
+  if (raw_version < 11010)
+return CudaVersion::CUDA_110;
+  return CudaVersion::LATEST;

tra wrote:
> emankov wrote:
> > Please, add `CudaVersion::CUDA_111` declaration in `Cuda.h` and a 
> > corresponding `if` here. 
> > Btw, `switch` is possible here. 
> It does not serve any purpose here. 102/110 were added when clang was only 
> accepting specific versions. Now that it will accept any newer version,  
> Arguably it's 102/101 that should be gone until we implement the new 
> functionality. All of that would out of scope for this patch.
> 
> As for the switch, it would only work to match exact versions encoded in the 
> CUDA headers, including updates, patches, special private builds etc. I do 
> not have access to all of those versions, so I can not enumerate all of them. 
> Range checking is more robust.
Ok, I can "live" with the latest 110 for a while till the appearing of the new 
functionality in clang. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89832

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


[PATCH] D89801: [SystemZ][z/OS] Set short-enums as the default for z/OS

2020-10-22 Thread Abhina Sree 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 rG9bc02e892f54: [SystemZ][z/OS] Set short-enums as the default 
for z/OS (authored by Jonathan.Crowther, committed by abhina.sreeskantharajan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89801

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/zos-driver-defaults.c


Index: clang/test/Driver/zos-driver-defaults.c
===
--- /dev/null
+++ clang/test/Driver/zos-driver-defaults.c
@@ -0,0 +1,9 @@
+// RUN: %clang -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck 
--check-prefix=CHECK-SHORT-ENUMS %s
+// RUN: %clang -### --target=s390x-none-zos -fno-short-enums -fsyntax-only %s 
2>&1 | FileCheck %s
+// REQUIRES: clang-driver
+
+//CHECK-SHORT-ENUMS: -fshort-enums
+//CHECK-SHORT-ENUMS: -fno-signed-char
+
+//CHECK-NOT: -fshort-enums
+//CHECK: -fno-signed-char
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5645,9 +5645,9 @@
(RTTIMode == ToolChain::RM_Disabled)))
 CmdArgs.push_back("-fno-rtti");
 
-  // -fshort-enums=0 is default for all architectures except Hexagon.
+  // -fshort-enums=0 is default for all architectures except Hexagon and z/OS.
   if (Args.hasFlag(options::OPT_fshort_enums, options::OPT_fno_short_enums,
-   TC.getArch() == llvm::Triple::hexagon))
+   TC.getArch() == llvm::Triple::hexagon || Triple.isOSzOS()))
 CmdArgs.push_back("-fshort-enums");
 
   RenderCharacterOptions(Args, AuxTriple ? *AuxTriple : RawTriple, CmdArgs);


Index: clang/test/Driver/zos-driver-defaults.c
===
--- /dev/null
+++ clang/test/Driver/zos-driver-defaults.c
@@ -0,0 +1,9 @@
+// RUN: %clang -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-SHORT-ENUMS %s
+// RUN: %clang -### --target=s390x-none-zos -fno-short-enums -fsyntax-only %s 2>&1 | FileCheck %s
+// REQUIRES: clang-driver
+
+//CHECK-SHORT-ENUMS: -fshort-enums
+//CHECK-SHORT-ENUMS: -fno-signed-char
+
+//CHECK-NOT: -fshort-enums
+//CHECK: -fno-signed-char
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5645,9 +5645,9 @@
(RTTIMode == ToolChain::RM_Disabled)))
 CmdArgs.push_back("-fno-rtti");
 
-  // -fshort-enums=0 is default for all architectures except Hexagon.
+  // -fshort-enums=0 is default for all architectures except Hexagon and z/OS.
   if (Args.hasFlag(options::OPT_fshort_enums, options::OPT_fno_short_enums,
-   TC.getArch() == llvm::Triple::hexagon))
+   TC.getArch() == llvm::Triple::hexagon || Triple.isOSzOS()))
 CmdArgs.push_back("-fshort-enums");
 
   RenderCharacterOptions(Args, AuxTriple ? *AuxTriple : RawTriple, CmdArgs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 9bc02e8 - [SystemZ][z/OS] Set short-enums as the default for z/OS

2020-10-22 Thread Abhina Sreeskantharajan via cfe-commits

Author: Jonathan Crowther
Date: 2020-10-22T14:15:58-04:00
New Revision: 9bc02e892f54b37031dd81cfc4485d86cee8c757

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

LOG: [SystemZ][z/OS] Set short-enums as the default for z/OS

This patch sets short-enums to be the default for z/OS.

Reviewed By: abhina.sreeskantharajan

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

Added: 
clang/test/Driver/zos-driver-defaults.c

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0d3d7a47a3a8..61e367bd835d 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5645,9 +5645,9 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
(RTTIMode == ToolChain::RM_Disabled)))
 CmdArgs.push_back("-fno-rtti");
 
-  // -fshort-enums=0 is default for all architectures except Hexagon.
+  // -fshort-enums=0 is default for all architectures except Hexagon and z/OS.
   if (Args.hasFlag(options::OPT_fshort_enums, options::OPT_fno_short_enums,
-   TC.getArch() == llvm::Triple::hexagon))
+   TC.getArch() == llvm::Triple::hexagon || Triple.isOSzOS()))
 CmdArgs.push_back("-fshort-enums");
 
   RenderCharacterOptions(Args, AuxTriple ? *AuxTriple : RawTriple, CmdArgs);

diff  --git a/clang/test/Driver/zos-driver-defaults.c 
b/clang/test/Driver/zos-driver-defaults.c
new file mode 100644
index ..0afabbf5324c
--- /dev/null
+++ b/clang/test/Driver/zos-driver-defaults.c
@@ -0,0 +1,9 @@
+// RUN: %clang -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck 
--check-prefix=CHECK-SHORT-ENUMS %s
+// RUN: %clang -### --target=s390x-none-zos -fno-short-enums -fsyntax-only %s 
2>&1 | FileCheck %s
+// REQUIRES: clang-driver
+
+//CHECK-SHORT-ENUMS: -fshort-enums
+//CHECK-SHORT-ENUMS: -fno-signed-char
+
+//CHECK-NOT: -fshort-enums
+//CHECK: -fno-signed-char



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


[PATCH] D89801: [SystemZ][z/OS] Set short-enums as the default for z/OS

2020-10-22 Thread Jonathan Crowther via Phabricator via cfe-commits
Jonathan.Crowther updated this revision to Diff 300050.
Jonathan.Crowther added a comment.

Forgot to set the repository for my last patch. No code changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89801

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/zos-driver-defaults.c


Index: clang/test/Driver/zos-driver-defaults.c
===
--- /dev/null
+++ clang/test/Driver/zos-driver-defaults.c
@@ -0,0 +1,9 @@
+// RUN: %clang -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck 
--check-prefix=CHECK-SHORT-ENUMS %s
+// RUN: %clang -### --target=s390x-none-zos -fno-short-enums -fsyntax-only %s 
2>&1 | FileCheck %s
+// REQUIRES: clang-driver
+
+//CHECK-SHORT-ENUMS: -fshort-enums
+//CHECK-SHORT-ENUMS: -fno-signed-char
+
+//CHECK-NOT: -fshort-enums
+//CHECK: -fno-signed-char
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5560,9 +5560,9 @@
(RTTIMode == ToolChain::RM_Disabled)))
 CmdArgs.push_back("-fno-rtti");
 
-  // -fshort-enums=0 is default for all architectures except Hexagon.
+  // -fshort-enums=0 is default for all architectures except Hexagon and z/OS.
   if (Args.hasFlag(options::OPT_fshort_enums, options::OPT_fno_short_enums,
-   TC.getArch() == llvm::Triple::hexagon))
+   TC.getArch() == llvm::Triple::hexagon || Triple.isOSzOS()))
 CmdArgs.push_back("-fshort-enums");
 
   RenderCharacterOptions(Args, AuxTriple ? *AuxTriple : RawTriple, CmdArgs);


Index: clang/test/Driver/zos-driver-defaults.c
===
--- /dev/null
+++ clang/test/Driver/zos-driver-defaults.c
@@ -0,0 +1,9 @@
+// RUN: %clang -### --target=s390x-none-zos -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-SHORT-ENUMS %s
+// RUN: %clang -### --target=s390x-none-zos -fno-short-enums -fsyntax-only %s 2>&1 | FileCheck %s
+// REQUIRES: clang-driver
+
+//CHECK-SHORT-ENUMS: -fshort-enums
+//CHECK-SHORT-ENUMS: -fno-signed-char
+
+//CHECK-NOT: -fshort-enums
+//CHECK: -fno-signed-char
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5560,9 +5560,9 @@
(RTTIMode == ToolChain::RM_Disabled)))
 CmdArgs.push_back("-fno-rtti");
 
-  // -fshort-enums=0 is default for all architectures except Hexagon.
+  // -fshort-enums=0 is default for all architectures except Hexagon and z/OS.
   if (Args.hasFlag(options::OPT_fshort_enums, options::OPT_fno_short_enums,
-   TC.getArch() == llvm::Triple::hexagon))
+   TC.getArch() == llvm::Triple::hexagon || Triple.isOSzOS()))
 CmdArgs.push_back("-fshort-enums");
 
   RenderCharacterOptions(Args, AuxTriple ? *AuxTriple : RawTriple, CmdArgs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69844: [clang][Basic] Integrate SourceLocation with FoldingSet, NFCI

2020-10-22 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki updated this revision to Diff 300049.
miyuki added a comment.

Reverted accidentally removed chunk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69844

Files:
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/Analysis/PathDiagnostic.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2193,8 +2193,8 @@
   for (SourceRange range : Ranges) {
 if (!range.isValid())
   continue;
-hash.AddInteger(range.getBegin().getRawEncoding());
-hash.AddInteger(range.getEnd().getRawEncoding());
+hash.Add(range.getBegin());
+hash.Add(range.getEnd());
   }
 }
 
@@ -2216,8 +2216,8 @@
   for (SourceRange range : Ranges) {
 if (!range.isValid())
   continue;
-hash.AddInteger(range.getBegin().getRawEncoding());
-hash.AddInteger(range.getEnd().getRawEncoding());
+hash.Add(range.getBegin());
+hash.Add(range.getEnd());
   }
 }
 
Index: clang/lib/Basic/SourceLocation.cpp
===
--- clang/lib/Basic/SourceLocation.cpp
+++ clang/lib/Basic/SourceLocation.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -45,6 +46,11 @@
   return llvm::DenseMapInfo::getHashValue(ID);
 }
 
+void llvm::FoldingSetTrait::Profile(
+const SourceLocation , llvm::FoldingSetNodeID ) {
+  ID.AddInteger(X.ID);
+}
+
 void SourceLocation::print(raw_ostream , const SourceManager )const{
   if (!isValid()) {
 OS << "";
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -1083,9 +1083,9 @@
 //===--===//
 
 void PathDiagnosticLocation::Profile(llvm::FoldingSetNodeID ) const {
-  ID.AddInteger(Range.getBegin().getRawEncoding());
-  ID.AddInteger(Range.getEnd().getRawEncoding());
-  ID.AddInteger(Loc.getRawEncoding());
+  ID.Add(Range.getBegin());
+  ID.Add(Range.getEnd());
+  ID.Add(static_cast(Loc));
 }
 
 void PathDiagnosticPiece::Profile(llvm::FoldingSetNodeID ) const {
@@ -1095,8 +1095,8 @@
   ID.AddInteger((unsigned) getDisplayHint());
   ArrayRef Ranges = getRanges();
   for (const auto  : Ranges) {
-ID.AddInteger(I.getBegin().getRawEncoding());
-ID.AddInteger(I.getEnd().getRawEncoding());
+ID.Add(I.getBegin());
+ID.Add(I.getEnd());
   }
 }
 
Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -26,6 +26,9 @@
 
 template  struct DenseMapInfo;
 
+class FoldingSetNodeID;
+template  struct FoldingSetTrait;
+
 } // namespace llvm
 
 namespace clang {
@@ -87,6 +90,7 @@
   friend class ASTReader;
   friend class ASTWriter;
   friend class SourceManager;
+  friend class llvm::FoldingSetTrait;
 
   unsigned ID = 0;
 
@@ -501,6 +505,10 @@
 }
   };
 
+  template <> struct FoldingSetTrait {
+static void Profile(const clang::SourceLocation , FoldingSetNodeID );
+  };
+
   // Teach SmallPtrSet how to handle SourceLocation.
   template<>
   struct PointerLikeTypeTraits {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

The host's build environment already influences the compiler's defaults, e.g. 
D88929 . Especially libomptarget heavily uses 
it, and I would argue that these should be consistent, unless explicitly 
overwritten.

However, I see that OpenMP offloading is not the only use case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment.

I echo @tra's concerns.
Having an option for a vendor to append/prepend a toolkit search location seems 
useful, but currently this seems more like a workaround for an explicit CUDA 
toolkit path not being passed to the testsuite.
Shortcomings in the testrunner don't seem like a reason to introduce new 
build-time configured default search paths into the driver.

Aside, here's an overview of CMake+CUDA:

- FindCUDA is deprecated since 3.10 and obsolete since 3.17. Replaced by native 
language support and FindCUDAToolkit 
.
- CMake supports Clang as a CUDA compiler since 3.18.
- CMake supports separable compilation with Clang since 3.19.
- CMake doesn't support CUDA with Clang on Windows 
. I hope to work on it 
for 3.20.

Ideally LLVM would use the native CMake CUDA support and the testsuite would 
use the CUDA toolkit found by CMake where necessary.
Users would be able to override the selected toolkit by setting 
`CUDAToolkit_ROOT` when configuring Clang.
Unfortunately this would require CMake 3.18, so this'll have to wait a bit.

If there are any questions/issues on the CMake side let me know. I maintain 
Clang and CUDA support in CMake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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


[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

2020-10-22 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia resigned from this revision.
Anastasia added a comment.

Just to clarify aside from the concern I have raised regarding internal testing 
I am not in any strong opposition of this feature. So if the community decides 
that it is more important to have this feature than to keep the tests short I 
am fine with this. However, due to the limited time I will not be able to 
continue reviewing this change as I don't want to block the progress due to my 
priorities.


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

https://reviews.llvm.org/D82756

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


[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

2020-10-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a subscriber: majnemer.
erichkeane added a comment.

Turns out this patch: 
https://github.com/llvm/llvm-project/commit/2e1e0491b7098fcfe01945e8f62cafe1fcb3cf36
 is my problem. The issue has to do deducing a 'local' type in the return type. 
 As a result, we choose to mangle any type 'containing' auto as 'auto'.

So ours demangles as: ##public: __thiscall `void __cdecl 
usage(void)'::`1'operator (void)const ##
MSVC of course demangles as: ##public: __thiscall 
::operator double 
(__cdecl*)(int,float,double)(void)const ##

To me, it seems that the ResultType->getContainedAutoType is perhaps in the 
wrong here (also of interest, the dyn_cast, for a function that already returns 
AutoType?)? I'm not sure how to workaround this.  I can of course create an 
awkward reproducer of this (https://godbolt.org/z/KWPTTh), but I don't have a 
good idea on how to better handle this.  @majnemer I see you were the initial 
committer of this, and would like to see if you have any ideas?


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

https://reviews.llvm.org/D89559

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


[PATCH] D89445: clang/Basic: Remove ContentCache::getRawBuffer, NFC

2020-10-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG156e8b37024a: clang/Basic: Remove 
ContentCache::getRawBuffer, NFC (authored by dexonsmith).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89445

Files:
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1468,7 +1468,7 @@
 if (PP->getHeaderSearchInfo()
 .getHeaderSearchOpts()
 .ValidateASTInputFilesContent) {
-  auto *MemBuff = Cache->getRawBuffer();
+  auto MemBuff = Cache->getBufferIfLoaded();
   if (MemBuff)
 ContentHash = hash_value(MemBuff->getBuffer());
   else
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1542,7 +1542,7 @@
   = SourceMgr.getOrCreateContentCache(File, isSystem(FileCharacter));
 if (OverriddenBuffer && !ContentCache->BufferOverridden &&
 ContentCache->ContentsEntry == ContentCache->OrigEntry &&
-!ContentCache->getRawBuffer()) {
+!ContentCache->getBufferIfLoaded()) {
   auto Buffer = ReadBuffer(SLocEntryCursor, File->getName());
   if (!Buffer)
 return true;
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -155,7 +155,7 @@
 
   // Check that the file's size is the same as in the file entry (which may
   // have come from a stat cache).
-  if (getRawBuffer()->getBufferSize() != (size_t)ContentsEntry->getSize()) {
+  if (Buffer->getBufferSize() != (size_t)ContentsEntry->getSize()) {
 if (Diag.isDiagnosticInFlight())
   Diag.SetDelayedDiagnostic(diag::err_file_modified,
 ContentsEntry->getName());
@@ -363,7 +363,7 @@
 Clone->BufferOverridden = Cache->BufferOverridden;
 Clone->IsFileVolatile = Cache->IsFileVolatile;
 Clone->IsTransient = Cache->IsTransient;
-Clone->setUnownedBuffer(Cache->getRawBuffer());
+Clone->setUnownedBuffer(Cache->getBufferIfLoaded());
 return Clone;
   };
 
@@ -476,7 +476,8 @@
 SourceManager::getFakeContentCacheForRecovery() const {
   if (!FakeContentCacheForRecovery) {
 FakeContentCacheForRecovery = std::make_unique();
-FakeContentCacheForRecovery->setUnownedBuffer(getFakeBufferForRecovery());
+FakeContentCacheForRecovery->setUnownedBuffer(
+getFakeBufferForRecovery()->getMemBufferRef());
   }
   return FakeContentCacheForRecovery.get();
 }
@@ -751,10 +752,7 @@
   if (!SLoc.isFile() || MyInvalid)
 return None;
 
-  if (const llvm::MemoryBuffer *Buf =
-  SLoc.getFile().getContentCache()->getRawBuffer())
-return Buf->getBuffer();
-  return None;
+  return SLoc.getFile().getContentCache()->getBufferDataIfLoaded();
 }
 
 llvm::Optional SourceManager::getBufferDataOrNone(FileID FID) const {
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -194,9 +194,23 @@
 /// this content cache.  This is used for performance analysis.
 llvm::MemoryBuffer::BufferKind getMemoryBufferKind() const;
 
-/// Get the underlying buffer, returning NULL if the buffer is not
-/// yet available.
-const llvm::MemoryBuffer *getRawBuffer() const { return Buffer.get(); }
+/// Return the buffer, only if it has been loaded.
+/// specified FileID, returning None if it's not yet loaded.
+///
+/// \param FID The file ID whose contents will be returned.
+llvm::Optional getBufferIfLoaded() const {
+  if (Buffer)
+return Buffer->getMemBufferRef();
+  return None;
+}
+
+/// Return a StringRef to the source buffer data, only if it has already
+/// been loaded.
+llvm::Optional getBufferDataIfLoaded() const {
+  if (Buffer)
+return Buffer->getBuffer();
+  return None;
+}
 
 /// Set the buffer.
 void setBuffer(std::unique_ptr B) {
@@ -207,10 +221,10 @@
 /// Set the buffer to one that's not owned (or to nullptr).
 ///
 /// \pre Buffer cannot already be set.
-void setUnownedBuffer(const llvm::MemoryBuffer *B) {
+void setUnownedBuffer(llvm::Optional B) {
   assert(!Buffer && "Expected to be called right after construction");
   if (B)
-

  1   2   3   >