[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

[Matthew Voss via Phabricator via cfe-commits]

ormris accepted this revision.
ormris added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154923

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

[PATCH] D154923: [CodeGen] Support bitcode input containing multiple modules

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

This is a good way to resolve the inconsistencies I was looking at in my own 
review.

> I think whether a bitcode input file contains 2 modules (internal 
> implementation strategy) should not be a criterion to require an additional 
> driver option

Agreed. Most users don't know about the internal structure of bitcode modules, 
so I think it's best to use a consistent interface for all bitcode files.




Comment at: clang/lib/CodeGen/CodeGenAction.cpp:1174
+  else
+M = std::move(*MOrErr);
+}

Perhaps this variable name could be more descriptive. Maybe "FirstM" or 
something like that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154923

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

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

> You can be more specific that multi-module bitcode files are for 
> -fsanitize=cfi and -fwhole-program-vtables. And it will be more useful to 
> include an example in the summary.

Fixed.

> This is a bit odd as the -o file may no longer an output. This could be 
> fixed, but is this functionality really useful?

I think it's useful to allow this kind of disassembly. Users who want to 
quickly inspect a bitcode files produced by the compiler could run them through 
clang first. While llvm-dis is the normal tool to do this, but some users don't 
know or have access to llvm-dis.

> The nicer thing is that llvm-dis additionally calls ModuleSummaryIndex::print 
> to print the module summary index (if present).

True, though this functionality could be added. I think users are more likely 
to be concerned about how their code was optimized by the LTO pre-link 
pipeline, say, then what the summary contains.


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

https://reviews.llvm.org/D153906

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

[PATCH] D123971: [clang][PS4] Enable SplitLTOUnits by default

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123971

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

[PATCH] D123971: [clang][PS4] Enable SplitLTOUnits by default

[Matthew Voss 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 rG392b30680752: [clang][PS4] Enable SplitLTOUnits and Unified 
LTO by default (authored by ormris).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D123971?vs=530117=539331#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123971

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/split-lto-unit.c
  clang/test/CodeGenCXX/unified-cfi-lto.cpp
  clang/test/Driver/lto-unit.c
  clang/test/Driver/split-lto-unit.c


Index: clang/test/Driver/split-lto-unit.c
===
--- clang/test/Driver/split-lto-unit.c
+++ clang/test/Driver/split-lto-unit.c
@@ -7,6 +7,10 @@
 // RUN: %clang --target=x86_64-apple-darwin13.3.0 -### %s 
-fwhole-program-vtables -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
 // RUN: %clang --target=x86_64-scei-ps4 -### %s -fwhole-program-vtables 
-flto=full 2>&1 | FileCheck --check-prefix=UNIT %s
 // RUN: %clang --target=x86_64-scei-ps4 -### %s -fwhole-program-vtables 
-flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang --target=x86_64-sie-ps5 -### %s -fwhole-program-vtables 
-flto=full 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang --target=x86_64-sie-ps5 -### %s -fwhole-program-vtables 
-flto=thin 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang --target=x86_64-unknown-linux -### %s -fwhole-program-vtables 
-flto=full -funified-lto 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang --target=x86_64-unknown-linux -### %s -fwhole-program-vtables 
-flto=thin -funified-lto 2>&1 | FileCheck --check-prefix=UNIT %s
 
 // UNIT: "-fsplit-lto-unit"
 // NOUNIT-NOT: "-fsplit-lto-unit"
Index: clang/test/Driver/lto-unit.c
===
--- clang/test/Driver/lto-unit.c
+++ clang/test/Driver/lto-unit.c
@@ -4,5 +4,10 @@
 // RUN: %clang --target=x86_64-apple-darwin13.3.0 -### %s -flto=thin 2>&1 | 
FileCheck --check-prefix=UNIT %s
 // RUN: %clang --target=x86_64-scei-ps4 -### %s -flto=full 2>&1 | FileCheck 
--check-prefix=UNIT %s
 // RUN: %clang --target=x86_64-scei-ps4 -### %s -flto=thin 2>&1 | FileCheck 
--check-prefix=UNIT %s
+// RUN: %clang --target=x86_64-scei-ps4 -### %s -flto=full -funified-lto 2>&1 
| FileCheck --check-prefix=UNIT %s
+// RUN: %clang --target=x86_64-scei-ps4 -### %s -flto=thin -funified-lto 2>&1 
| FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang --target=x86_64-unknown-linux -### %s -flto=full -funified-lto 
2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang --target=x86_64-unknown-linux -### %s -flto=thin -funified-lto 
2>&1 | FileCheck --check-prefix=UNIT %s
 
 // UNIT: "-flto-unit"
+// NOUNIT-NOT: "-flto-unit"
Index: clang/test/CodeGenCXX/unified-cfi-lto.cpp
===
--- clang/test/CodeGenCXX/unified-cfi-lto.cpp
+++ clang/test/CodeGenCXX/unified-cfi-lto.cpp
@@ -1,6 +1,6 @@
 // Ensure that the frontend adds the proper metadata when CFI is
 // enabled.
-// RUN: %clang --target=x86_64-scei-ps4 -funified-lto -flto -fsanitize=cfi 
-fvisibility=hidden -c %s -o %t.o
+// RUN: %clang --target=x86_64-scei-ps4 -funified-lto -flto -fsanitize=cfi 
-fvisibility=hidden -fno-sanitize-ignorelist -c %s -o %t.o
 // RUN: llvm-dis %t.o -o %t1
 // RUN: FileCheck <%t1.0 %s
 
Index: clang/test/CodeGen/split-lto-unit.c
===
--- clang/test/CodeGen/split-lto-unit.c
+++ clang/test/CodeGen/split-lto-unit.c
@@ -1,6 +1,7 @@
 // ; Check that -flto=thin without -fsplit-lto-unit has EnableSplitLTOUnit = 0
 // RUN: %clang_cc1 -flto=thin -emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s
 // RUN: %clang_cc1 -flto=thin -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -flto=thin -funified-lto -triple=x86_64-scei-ps4 
-emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s
 // CHECK: !{i32 1, !"EnableSplitLTOUnit", i32 0}
 //
 // ; Check that -flto=thin with -fsplit-lto-unit has EnableSplitLTOUnit = 1
@@ -12,4 +13,9 @@
 // RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc < %s | 
llvm-dis -o - | FileCheck %s --implicit-check-not="EnableSplitLTOUnit" 
--check-prefix=SPLIT
 // RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm < %s | 
FileCheck %s --implicit-check-not="EnableSplitLTOUnit" --check-prefix=SPLIT
 
+// ; Check that regular LTO has EnableSplitLTOUnit = 1, if using distinct 
pipelines. For unified pipelines, use EnableSplitLTOUnit = 0.
+// RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc < %s | 
llvm-dis -o - | FileCheck %s --implicit-check-not="EnableSplitLTOUnit" 
--check-prefix=SPLIT
+// RUN: %clang_cc1 -flto -funified-lto -triple x86_64-scei-ps4 -emit-llvm-bc < 
%s | llvm-dis -o 

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks! Pushed as 048a0c246908291c82d2f4531d3df45a4c4a8a18 
.


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

https://reviews.llvm.org/D153906

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

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 538867.
ormris added a comment.

Attempt to fix pre-merge checks.


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

https://reviews.llvm.org/D153906

Files:
  clang/include/clang/CodeGen/CodeGenAction.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/split-lto-ir-support.cpp

Index: clang/test/Frontend/split-lto-ir-support.cpp
===
--- /dev/null
+++ clang/test/Frontend/split-lto-ir-support.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang -c -flto=thin %s -o %t0.bc
+// RUN: mkdir %t1.d
+// RUN: %clang -S -emit-llvm %t0.bc -o %t1.d/split-lto-ir-support.ll
+// RUN: FileCheck <%t1.d/split-lto-ir-support.0.ll --check-prefix M0 %s
+// RUN: FileCheck <%t1.d/split-lto-ir-support.1.ll --check-prefix M1 %s
+// RUN: rm -rf %t1.d
+// M0: @main
+// M1: anon.
+
+struct A {
+  virtual int c(int i) = 0;
+};
+
+struct B : A {
+  virtual int c(int i) {
+return i + 3;
+  }
+};
+
+int
+main(void)
+{
+  A *a = new B();
+
+  return a->c(0);
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1107,26 +1107,59 @@
   return std::move(Result);
 }
 
-std::unique_ptr
+std::vector>
 CodeGenAction::loadModule(MemoryBufferRef MBRef) {
   CompilerInstance  = getCompilerInstance();
   SourceManager  = CI.getSourceManager();
 
+  std::vector> Result;
+
+  if (!llvm::isBitcode((const unsigned char *)MBRef.getBufferStart(),
+   (const unsigned char *)MBRef.getBufferEnd())) {
+llvm::SMDiagnostic Err;
+std::unique_ptr M = parseIR(MBRef, Err, *VMContext);
+if (!M) {
+  // Translate from the diagnostic info to the SourceManager location if
+  // available.
+  // TODO: Unify this with ConvertBackendLocation()
+  SourceLocation Loc;
+  if (Err.getLineNo() > 0) {
+assert(Err.getColumnNo() >= 0);
+Loc = SM.translateFileLineCol(SM.getFileEntryForID(SM.getMainFileID()),
+  Err.getLineNo(), Err.getColumnNo() + 1);
+  }
+
+  // Strip off a leading diagnostic code if there is one.
+  StringRef Msg = Err.getMessage();
+  if (Msg.startswith("error: "))
+Msg = Msg.substr(7);
+
+  unsigned DiagID =
+  CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
+
+  CI.getDiagnostics().Report(Loc, DiagID) << Msg;
+  return {};
+}
+
+Result.push_back(std::move(M));
+return Result;
+  }
+
+  auto DiagErrors = [&](Error E) -> std::vector> {
+unsigned DiagID =
+CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
+handleAllErrors(std::move(E), [&](ErrorInfoBase ) {
+  CI.getDiagnostics().Report(DiagID) << EIB.message();
+});
+return {};
+  };
+
   // For ThinLTO backend invocations, ensure that the context
   // merges types based on ODR identifiers. We also need to read
   // the correct module out of a multi-module bitcode file.
   if (!CI.getCodeGenOpts().ThinLTOIndexFile.empty()) {
 VMContext->enableDebugTypeODRUniquing();
 
-auto DiagErrors = [&](Error E) -> std::unique_ptr {
-  unsigned DiagID =
-  CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
-  handleAllErrors(std::move(E), [&](ErrorInfoBase ) {
-CI.getDiagnostics().Report(DiagID) << EIB.message();
-  });
-  return {};
-};
-
 Expected> BMsOrErr = getBitcodeModuleList(MBRef);
 if (!BMsOrErr)
   return DiagErrors(BMsOrErr.takeError());
@@ -1138,43 +1171,33 @@
 if (!Bm) {
   auto M = std::make_unique("empty", *VMContext);
   M->setTargetTriple(CI.getTargetOpts().Triple);
-  return M;
+  Result.push_back(std::move(M));
+  return Result;
 }
 Expected> MOrErr =
 Bm->parseModule(*VMContext);
 if (!MOrErr)
   return DiagErrors(MOrErr.takeError());
-return std::move(*MOrErr);
+Result.push_back(std::move(MOrErr.get()));
+return Result;
   }
 
   // Load bitcode modules to link with, if we need to.
   if (loadLinkModules(CI))
-return nullptr;
+return {};
 
-  llvm::SMDiagnostic Err;
-  if (std::unique_ptr M = parseIR(MBRef, Err, *VMContext))
-return M;
-
-  // Translate from the diagnostic info to the SourceManager location if
-  // available.
-  // TODO: Unify this with ConvertBackendLocation()
-  SourceLocation Loc;
-  if (Err.getLineNo() > 0) {
-assert(Err.getColumnNo() >= 0);
-Loc = SM.translateFileLineCol(SM.getFileEntryForID(SM.getMainFileID()),
-  Err.getLineNo(), Err.getColumnNo() + 1);
-  }
-
-  // Strip off a leading diagnostic code if there is one.
-  StringRef Msg = Err.getMessage();
-  if (Msg.startswith("error: "))
-Msg = Msg.substr(7);
+  Expected> BMsOrErr = getBitcodeModuleList(MBRef);
+  if (!BMsOrErr)
+return 

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Yes, I'm working on a new test that's less target dependent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153906

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

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 535202.
ormris added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153906

Files:
  clang/include/clang/CodeGen/CodeGenAction.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/split-lto-ir-support.cpp

Index: clang/test/Frontend/split-lto-ir-support.cpp
===
--- /dev/null
+++ clang/test/Frontend/split-lto-ir-support.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang -O0 -Xclang -mconstructor-aliases -c -flto=thin -Xclang -fsplit-lto-unit -ffunction-sections %s -o %t0.bc
+// RUN: rm -rf %t1.d && mkdir -p %t1.d
+// RUN: %clang -S -emit-llvm %t0.bc -o %t1.d/split-lto-ir-support.ll
+// RUN: %clang -S %t1.d/split-lto-ir-support.0.ll %t1.d/split-lto-ir-support.1.ll
+// RUN: FileCheck c(0);
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1107,26 +1107,59 @@
   return std::move(Result);
 }
 
-std::unique_ptr
+std::vector>
 CodeGenAction::loadModule(MemoryBufferRef MBRef) {
   CompilerInstance  = getCompilerInstance();
   SourceManager  = CI.getSourceManager();
 
+  std::vector> Result;
+
+  if (!llvm::isBitcode((const unsigned char *)MBRef.getBufferStart(),
+   (const unsigned char *)MBRef.getBufferEnd())) {
+llvm::SMDiagnostic Err;
+std::unique_ptr M = parseIR(MBRef, Err, *VMContext);
+if (!M) {
+  // Translate from the diagnostic info to the SourceManager location if
+  // available.
+  // TODO: Unify this with ConvertBackendLocation()
+  SourceLocation Loc;
+  if (Err.getLineNo() > 0) {
+assert(Err.getColumnNo() >= 0);
+Loc = SM.translateFileLineCol(SM.getFileEntryForID(SM.getMainFileID()),
+  Err.getLineNo(), Err.getColumnNo() + 1);
+  }
+
+  // Strip off a leading diagnostic code if there is one.
+  StringRef Msg = Err.getMessage();
+  if (Msg.startswith("error: "))
+Msg = Msg.substr(7);
+
+  unsigned DiagID =
+  CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
+
+  CI.getDiagnostics().Report(Loc, DiagID) << Msg;
+  return {};
+}
+
+Result.push_back(std::move(M));
+return Result;
+  }
+
+  auto DiagErrors = [&](Error E) -> std::vector> {
+unsigned DiagID =
+CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
+handleAllErrors(std::move(E), [&](ErrorInfoBase ) {
+  CI.getDiagnostics().Report(DiagID) << EIB.message();
+});
+return {};
+  };
+
   // For ThinLTO backend invocations, ensure that the context
   // merges types based on ODR identifiers. We also need to read
   // the correct module out of a multi-module bitcode file.
   if (!CI.getCodeGenOpts().ThinLTOIndexFile.empty()) {
 VMContext->enableDebugTypeODRUniquing();
 
-auto DiagErrors = [&](Error E) -> std::unique_ptr {
-  unsigned DiagID =
-  CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
-  handleAllErrors(std::move(E), [&](ErrorInfoBase ) {
-CI.getDiagnostics().Report(DiagID) << EIB.message();
-  });
-  return {};
-};
-
 Expected> BMsOrErr = getBitcodeModuleList(MBRef);
 if (!BMsOrErr)
   return DiagErrors(BMsOrErr.takeError());
@@ -1138,43 +1171,33 @@
 if (!Bm) {
   auto M = std::make_unique("empty", *VMContext);
   M->setTargetTriple(CI.getTargetOpts().Triple);
-  return M;
+  Result.push_back(std::move(M));
+  return Result;
 }
 Expected> MOrErr =
 Bm->parseModule(*VMContext);
 if (!MOrErr)
   return DiagErrors(MOrErr.takeError());
-return std::move(*MOrErr);
+Result.push_back(std::move(MOrErr.get()));
+return Result;
   }
 
   // Load bitcode modules to link with, if we need to.
   if (loadLinkModules(CI))
-return nullptr;
-
-  llvm::SMDiagnostic Err;
-  if (std::unique_ptr M = parseIR(MBRef, Err, *VMContext))
-return M;
-
-  // Translate from the diagnostic info to the SourceManager location if
-  // available.
-  // TODO: Unify this with ConvertBackendLocation()
-  SourceLocation Loc;
-  if (Err.getLineNo() > 0) {
-assert(Err.getColumnNo() >= 0);
-Loc = SM.translateFileLineCol(SM.getFileEntryForID(SM.getMainFileID()),
-  Err.getLineNo(), Err.getColumnNo() + 1);
-  }
+return {};
 
-  // Strip off a leading diagnostic code if there is one.
-  StringRef Msg = Err.getMessage();
-  if (Msg.startswith("error: "))
-Msg = Msg.substr(7);
+  Expected> BMsOrErr = getBitcodeModuleList(MBRef);
+  if (!BMsOrErr)
+return DiagErrors(BMsOrErr.takeError());
 
-  unsigned DiagID =
-  CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
+  for (auto BM : BMsOrErr.get()) {
+  

[PATCH] D153906: [clang] Allow disassembly of multi-module bitcode files

[Matthew Voss via Phabricator via cfe-commits]

ormris created this revision.
ormris added reviewers: efriedma, rjmccall, MaskRay, aaron.ballman.
Herald added subscribers: steven_wu, hiraditya.
Herald added a project: All.
ormris requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang currently exits with an error message if asked to disassemble a
multi-module bitcode file. This might be confusing to some who are not
familiar with bitcode formats. In addition, multi-module bitcode files
are reasonably common, since they enable desired features. This patch
makes clang's behavior consistent for all common bitcode files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153906

Files:
  clang/include/clang/CodeGen/CodeGenAction.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/split-lto-ir-support.cpp

Index: clang/test/Frontend/split-lto-ir-support.cpp
===
--- /dev/null
+++ clang/test/Frontend/split-lto-ir-support.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang -O0 -Xclang -mconstructor-aliases -c -flto=thin -Xclang -fsplit-lto-unit -ffunction-sections %s -o %t0.bc
+// RUN: rm -rf %t1.d && mkdir -p %t1.d
+// RUN: %clang -S -emit-llvm %t0.bc -o %t1.d/split-lto-ir-support.ll
+// RUN: %clang -S %t1.d/split-lto-ir-support.0.ll %t1.d/split-lto-ir-support.1.ll
+// RUN: FileCheck c(0);
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1107,26 +1107,59 @@
   return std::move(Result);
 }
 
-std::unique_ptr
+std::vector>
 CodeGenAction::loadModule(MemoryBufferRef MBRef) {
   CompilerInstance  = getCompilerInstance();
   SourceManager  = CI.getSourceManager();
 
+  std::vector> Result;
+
+  if (!llvm::isBitcode((const unsigned char *)MBRef.getBufferStart(),
+   (const unsigned char *)MBRef.getBufferEnd())) {
+llvm::SMDiagnostic Err;
+std::unique_ptr M = parseIR(MBRef, Err, *VMContext);
+if (!M) {
+  // Translate from the diagnostic info to the SourceManager location if
+  // available.
+  // TODO: Unify this with ConvertBackendLocation()
+  SourceLocation Loc;
+  if (Err.getLineNo() > 0) {
+assert(Err.getColumnNo() >= 0);
+Loc = SM.translateFileLineCol(SM.getFileEntryForID(SM.getMainFileID()),
+  Err.getLineNo(), Err.getColumnNo() + 1);
+  }
+
+  // Strip off a leading diagnostic code if there is one.
+  StringRef Msg = Err.getMessage();
+  if (Msg.startswith("error: "))
+Msg = Msg.substr(7);
+
+  unsigned DiagID =
+  CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
+
+  CI.getDiagnostics().Report(Loc, DiagID) << Msg;
+  return {};
+}
+
+Result.push_back(std::move(M));
+return Result;
+  }
+
+  auto DiagErrors = [&](Error E) -> std::vector> {
+unsigned DiagID =
+CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
+handleAllErrors(std::move(E), [&](ErrorInfoBase ) {
+  CI.getDiagnostics().Report(DiagID) << EIB.message();
+});
+return {};
+  };
+
   // For ThinLTO backend invocations, ensure that the context
   // merges types based on ODR identifiers. We also need to read
   // the correct module out of a multi-module bitcode file.
   if (!CI.getCodeGenOpts().ThinLTOIndexFile.empty()) {
 VMContext->enableDebugTypeODRUniquing();
 
-auto DiagErrors = [&](Error E) -> std::unique_ptr {
-  unsigned DiagID =
-  CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
-  handleAllErrors(std::move(E), [&](ErrorInfoBase ) {
-CI.getDiagnostics().Report(DiagID) << EIB.message();
-  });
-  return {};
-};
-
 Expected> BMsOrErr = getBitcodeModuleList(MBRef);
 if (!BMsOrErr)
   return DiagErrors(BMsOrErr.takeError());
@@ -1138,43 +1171,33 @@
 if (!Bm) {
   auto M = std::make_unique("empty", *VMContext);
   M->setTargetTriple(CI.getTargetOpts().Triple);
-  return M;
+  Result.push_back(std::move(M));
+  return Result;
 }
 Expected> MOrErr =
 Bm->parseModule(*VMContext);
 if (!MOrErr)
   return DiagErrors(MOrErr.takeError());
-return std::move(*MOrErr);
+Result.push_back(std::move(MOrErr.get()));
+return Result;
   }
 
   // Load bitcode modules to link with, if we need to.
   if (loadLinkModules(CI))
-return nullptr;
-
-  llvm::SMDiagnostic Err;
-  if (std::unique_ptr M = parseIR(MBRef, Err, *VMContext))
-return M;
-
-  // Translate from the diagnostic info to the SourceManager location if
-  // available.
-  // TODO: Unify this with ConvertBackendLocation()
-  SourceLocation Loc;
-  if (Err.getLineNo() > 0) {
-assert(Err.getColumnNo() >= 0);
-Loc = SM.translateFileLineCol(SM.getFileEntryForID(SM.getMainFileID()),
-

[PATCH] D147660: [PS4][clang] Pass -flto-jobs argument to orbis-ld

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147660

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

[PATCH] D147660: [PS4][clang] Pass -flto-jobs argument to orbis-ld

[Matthew Voss 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 rG393a1c3b4fcd: [PS4][clang] Pass -flto-jobs argument to 
orbis-ld (authored by ormris).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147660

Files:
  clang/lib/Driver/ToolChains/PS4CPU.cpp
  clang/test/Driver/lto-jobs.c


Index: clang/test/Driver/lto-jobs.c
===
--- clang/test/Driver/lto-jobs.c
+++ clang/test/Driver/lto-jobs.c
@@ -7,6 +7,11 @@
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS-ACTION < %t %s
 //
 // CHECK-LINK-THIN-JOBS-ACTION: "-plugin-opt=jobs=5"
+//
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin -flto-jobs=5 2> %t
+// RUN: FileCheck -check-prefix=CHECK-PS4-LINK-THIN-JOBS-ACTION < %t %s
+//
+// CHECK-PS4-LINK-THIN-JOBS-ACTION: "-lto-thin-debug-options= 
-generate-arange-section -threads=5"
 
 // RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin 
-flto-jobs=5 2> %t
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS2-ACTION < %t %s
Index: clang/lib/Driver/ToolChains/PS4CPU.cpp
===
--- clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -180,6 +180,14 @@
 if (Arg *A = Args.getLastArg(options::OPT_fcrash_diagnostics_dir))
   AddCodeGenFlag(Twine("-crash-diagnostics-dir=") + A->getValue());
 
+StringRef Parallelism = getLTOParallelism(Args, D);
+if (!Parallelism.empty()) {
+  if (IsPS4)
+AddCodeGenFlag(Twine("-threads=") + Parallelism);
+  else
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=jobs=") + 
Parallelism));
+}
+
 if (IsPS4) {
   const char *Prefix = nullptr;
   if (D.getLTOMode() == LTOK_Thin)
@@ -193,12 +201,6 @@
 }
   }
 
-  if (IsPS5 && UseLTO) {
-StringRef Parallelism = getLTOParallelism(Args, D);
-if (!Parallelism.empty())
-  CmdArgs.push_back(Args.MakeArgString("-plugin-opt=jobs=" + Parallelism));
-  }
-
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
 TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 


Index: clang/test/Driver/lto-jobs.c
===
--- clang/test/Driver/lto-jobs.c
+++ clang/test/Driver/lto-jobs.c
@@ -7,6 +7,11 @@
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS-ACTION < %t %s
 //
 // CHECK-LINK-THIN-JOBS-ACTION: "-plugin-opt=jobs=5"
+//
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin -flto-jobs=5 2> %t
+// RUN: FileCheck -check-prefix=CHECK-PS4-LINK-THIN-JOBS-ACTION < %t %s
+//
+// CHECK-PS4-LINK-THIN-JOBS-ACTION: "-lto-thin-debug-options= -generate-arange-section -threads=5"
 
 // RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin -flto-jobs=5 2> %t
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS2-ACTION < %t %s
Index: clang/lib/Driver/ToolChains/PS4CPU.cpp
===
--- clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -180,6 +180,14 @@
 if (Arg *A = Args.getLastArg(options::OPT_fcrash_diagnostics_dir))
   AddCodeGenFlag(Twine("-crash-diagnostics-dir=") + A->getValue());
 
+StringRef Parallelism = getLTOParallelism(Args, D);
+if (!Parallelism.empty()) {
+  if (IsPS4)
+AddCodeGenFlag(Twine("-threads=") + Parallelism);
+  else
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=jobs=") + Parallelism));
+}
+
 if (IsPS4) {
   const char *Prefix = nullptr;
   if (D.getLTOMode() == LTOK_Thin)
@@ -193,12 +201,6 @@
 }
   }
 
-  if (IsPS5 && UseLTO) {
-StringRef Parallelism = getLTOParallelism(Args, D);
-if (!Parallelism.empty())
-  CmdArgs.push_back(Args.MakeArgString("-plugin-opt=jobs=" + Parallelism));
-  }
-
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
 TC.addSanitizerArgs(Args, CmdArgs, "-l", "");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147546: [PS4][clang] Fix the format of the LTO debug options passed to orbis-ld

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147546

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

[PATCH] D147546: [PS4][clang] Fix the format of the LTO debug options passed to orbis-ld

[Matthew Voss 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 rGc37b95b515a5: [PS4][clang] Fix the format of the LTO debug 
options passed to orbis-ld (authored by ormris).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147546

Files:
  clang/lib/Driver/ToolChains/PS4CPU.cpp
  clang/test/Driver/ps4-ps5-linker.c


Index: clang/test/Driver/ps4-ps5-linker.c
===
--- clang/test/Driver/ps4-ps5-linker.c
+++ clang/test/Driver/ps4-ps5-linker.c
@@ -7,8 +7,8 @@
 // RUN: %clang --target=x86_64-scei-ps5 -flto -fjmc %s -### 2>&1 | FileCheck 
--check-prefixes=CHECK-PS5-LTO,CHECK-PS5-LIB %s
 
 // CHECK-PS4-NOT: -enable-jmc-instrument
-// CHECK-PS4-THIN-LTO: -lto-thin-debug-options=-enable-jmc-instrument
-// CHECK-PS4-FULL-LTO: -lto-debug-options=-enable-jmc-instrument
+// CHECK-PS4-THIN-LTO: "-lto-thin-debug-options= -generate-arange-section 
-enable-jmc-instrument"
+// CHECK-PS4-FULL-LTO: "-lto-debug-options= -generate-arange-section 
-enable-jmc-instrument"
 // CHECK-PS5-NOT: -plugin-opt=-enable-jmc-instrument
 // CHECK-PS5-LTO: -plugin-opt=-enable-jmc-instrument
 
@@ -23,7 +23,7 @@
 // RUN: %clang --target=x86_64-scei-ps5 -fcrash-diagnostics-dir=mydumps %s 
-### 2>&1 | FileCheck --check-prefixes=CHECK-DIAG-PS5 %s
 // RUN: %clang --target=x86_64-scei-ps5 -flto -fcrash-diagnostics-dir=mydumps 
%s -### 2>&1 | FileCheck --check-prefixes=CHECK-DIAG-PS5-LTO %s
 
-// CHECK-DIAG-PS4-THIN-LTO: 
-lto-thin-debug-options=-crash-diagnostics-dir=mydumps
-// CHECK-DIAG-PS4-FULL-LTO: -lto-debug-options=-crash-diagnostics-dir=mydumps
+// CHECK-DIAG-PS4-THIN-LTO: "-lto-thin-debug-options= -generate-arange-section 
-crash-diagnostics-dir=mydumps"
+// CHECK-DIAG-PS4-FULL-LTO: "-lto-debug-options= -generate-arange-section 
-crash-diagnostics-dir=mydumps"
 // CHECK-DIAG-PS5-NOT: -plugin-opt=-crash-diagnostics-dir=mydumps
 // CHECK-DIAG-PS5-LTO: -plugin-opt=-crash-diagnostics-dir=mydumps
Index: clang/lib/Driver/ToolChains/PS4CPU.cpp
===
--- clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -160,18 +160,12 @@
   const bool IsPS5 = TC.getTriple().isPS5();
   assert(IsPS4 || IsPS5);
 
+  const char *PS4LTOArgs = "";
   auto AddCodeGenFlag = [&](Twine Flag) {
-const char *Prefix = nullptr;
-if (IsPS4 && D.getLTOMode() == LTOK_Thin)
-  Prefix = "-lto-thin-debug-options=";
-else if (IsPS4 && D.getLTOMode() == LTOK_Full)
-  Prefix = "-lto-debug-options=";
+if (IsPS4)
+  PS4LTOArgs = Args.MakeArgString(Twine(PS4LTOArgs) + " " + Flag);
 else if (IsPS5)
-  Prefix = "-plugin-opt=";
-else
-  llvm_unreachable("new LTO mode?");
-
-CmdArgs.push_back(Args.MakeArgString(Twine(Prefix) + Flag));
+  CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=") + Flag));
   };
 
   if (UseLTO) {
@@ -185,6 +179,18 @@
 
 if (Arg *A = Args.getLastArg(options::OPT_fcrash_diagnostics_dir))
   AddCodeGenFlag(Twine("-crash-diagnostics-dir=") + A->getValue());
+
+if (IsPS4) {
+  const char *Prefix = nullptr;
+  if (D.getLTOMode() == LTOK_Thin)
+Prefix = "-lto-thin-debug-options=";
+  else if (D.getLTOMode() == LTOK_Full)
+Prefix = "-lto-debug-options=";
+  else
+llvm_unreachable("new LTO mode?");
+
+  CmdArgs.push_back(Args.MakeArgString(Twine(Prefix) + PS4LTOArgs));
+}
   }
 
   if (IsPS5 && UseLTO) {


Index: clang/test/Driver/ps4-ps5-linker.c
===
--- clang/test/Driver/ps4-ps5-linker.c
+++ clang/test/Driver/ps4-ps5-linker.c
@@ -7,8 +7,8 @@
 // RUN: %clang --target=x86_64-scei-ps5 -flto -fjmc %s -### 2>&1 | FileCheck --check-prefixes=CHECK-PS5-LTO,CHECK-PS5-LIB %s
 
 // CHECK-PS4-NOT: -enable-jmc-instrument
-// CHECK-PS4-THIN-LTO: -lto-thin-debug-options=-enable-jmc-instrument
-// CHECK-PS4-FULL-LTO: -lto-debug-options=-enable-jmc-instrument
+// CHECK-PS4-THIN-LTO: "-lto-thin-debug-options= -generate-arange-section -enable-jmc-instrument"
+// CHECK-PS4-FULL-LTO: "-lto-debug-options= -generate-arange-section -enable-jmc-instrument"
 // CHECK-PS5-NOT: -plugin-opt=-enable-jmc-instrument
 // CHECK-PS5-LTO: -plugin-opt=-enable-jmc-instrument
 
@@ -23,7 +23,7 @@
 // RUN: %clang --target=x86_64-scei-ps5 -fcrash-diagnostics-dir=mydumps %s -### 2>&1 | FileCheck --check-prefixes=CHECK-DIAG-PS5 %s
 // RUN: %clang --target=x86_64-scei-ps5 -flto -fcrash-diagnostics-dir=mydumps %s -### 2>&1 | FileCheck --check-prefixes=CHECK-DIAG-PS5-LTO %s
 
-// CHECK-DIAG-PS4-THIN-LTO: -lto-thin-debug-options=-crash-diagnostics-dir=mydumps
-// CHECK-DIAG-PS4-FULL-LTO: -lto-debug-options=-crash-diagnostics-dir=mydumps
+// 

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Looks like there are some test failures associated with this commit: 
https://lab.llvm.org/buildbot/#/builders/123/builds/13698

The change list is a bit confusing, as it's only showing LLVM commits. But the 
change list includes the commit before and the commit after this one.

The tests in this patch are also failing in our internal CI. @ckandeler, could 
you take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D134296: [SIE] Always enable the .debug_aranges section when using LTO

[Matthew Voss 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 rG45c7da241f12: [PS4] Always enable the .debug_aranges section 
when using LTO (authored by ormris).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134296

Files:
  clang/lib/Driver/ToolChains/PS4CPU.cpp
  clang/test/Driver/debug-options.c
  clang/test/Driver/ps4-ps5-linker-jmc.c


Index: clang/test/Driver/ps4-ps5-linker-jmc.c
===
--- clang/test/Driver/ps4-ps5-linker-jmc.c
+++ clang/test/Driver/ps4-ps5-linker-jmc.c
@@ -6,10 +6,10 @@
 // RUN: %clang --target=x86_64-scei-ps5 -fjmc %s -### 2>&1 | FileCheck 
--check-prefixes=CHECK-PS5,CHECK-PS5-LIB %s
 // RUN: %clang --target=x86_64-scei-ps5 -flto -fjmc %s -### 2>&1 | FileCheck 
--check-prefixes=CHECK-PS5-LTO,CHECK-PS5-LIB %s
 
-// CHECK-PS4-NOT: "-enable-jmc-instrument"
+// CHECK-PS4-NOT: -enable-jmc-instrument
 
-// CHECK-PS4-THIN-LTO: "-lto-thin-debug-options=-enable-jmc-instrument"
-// CHECK-PS4-FULL-LTO: "-lto-debug-options=-enable-jmc-instrument"
+// CHECK-PS4-THIN-LTO: -lto-thin-debug-options=-enable-jmc-instrument
+// CHECK-PS4-FULL-LTO: -lto-debug-options=-enable-jmc-instrument
 
 // CHECK-PS5-NOT: "-enable-jmc-instrument"
 
Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -107,6 +107,14 @@
 // RUN: | FileCheck -check-prefix=CI %s
 // RUN: %clang -### -c %s -gsce -target x86_64-unknown-linux 2>&1 \
 // RUN: | FileCheck -check-prefix=NOCI %s
+// RUN: %clang -### %s -g -flto=thin -target x86_64-scei-ps4 2>&1 \
+// RUN: | FileCheck -check-prefix=SNLDTLTOGARANGE %s
+// RUN: %clang -### %s -g -flto=full -target x86_64-scei-ps4 2>&1 \
+// RUN: | FileCheck -check-prefix=SNLDFLTOGARANGE %s
+// RUN: %clang -### %s -g -flto -target x86_64-scei-ps5 2>&1 \
+// RUN: | FileCheck -check-prefix=LLDGARANGE %s
+// RUN: %clang -### %s -g -target x86_64-scei-ps5 2>&1 \
+// RUN: | FileCheck -check-prefix=LDGARANGE %s
 
 // On the AIX, -g defaults to -gdbx and limited debug info.
 // RUN: %clang -### -c -g %s -target powerpc-ibm-aix-xcoff 2>&1 \
@@ -365,6 +373,13 @@
 // NOPUB-NOT: -ggnu-pubnames
 // NOPUB-NOT: -gpubnames
 //
+
+// LDGARANGE: {{".*ld.*"}} {{.*}}
+// LDGARANGE-NOT: "-generate-arange-section"
+// LLDGARANGE: {{".*lld.*"}} {{.*}} "-generate-arange-section"
+// SNLDTLTOGARANGE: {{".*orbis-ld.*"}} {{.*}} 
"-lto-thin-debug-options=-generate-arange-section"
+// SNLDFLTOGARANGE: {{".*orbis-ld.*"}} {{.*}} 
"-lto-debug-options=-generate-arange-section"
+
 // PUB: -gpubnames
 //
 // RNGBSE: -fdebug-ranges-base-address
Index: clang/lib/Driver/ToolChains/PS4CPU.cpp
===
--- clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -159,17 +159,32 @@
   const bool IsPS5 = TC.getTriple().isPS5();
   assert(IsPS4 || IsPS5);
 
+  ArgStringList DbgOpts;
+
   // This tells LTO to perform JustMyCode instrumentation.
-  if (UseLTO && UseJMC) {
-if (IsPS4 && D.getLTOMode() == LTOK_Thin) {
-  CmdArgs.push_back("-lto-thin-debug-options=-enable-jmc-instrument");
-} else if (IsPS4 && D.getLTOMode() == LTOK_Full) {
-  CmdArgs.push_back("-lto-debug-options=-enable-jmc-instrument");
-} else if (IsPS5) {
-  CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-jmc-instrument");
-} else
-  llvm_unreachable("new LTO mode?");
+  if (UseLTO && UseJMC)
+DbgOpts.push_back("-enable-jmc-instrument");
+
+  // We default to creating the arange section, but LTO does not. Enable it
+  // here.
+  if (UseLTO)
+DbgOpts.push_back("-generate-arange-section");
+
+  if (UseLTO) {
+if (IsPS4) {
+  StringRef F = (D.getLTOMode() == LTOK_Thin) ?
+  "-lto-thin-debug-options=" : "-lto-debug-options=";
+  F = makeArgString(Args, F.data(), DbgOpts.front(), "");
+  DbgOpts.erase(DbgOpts.begin());
+  for (auto X : DbgOpts)
+F = makeArgString(Args, F.data(), " ", X);
+  CmdArgs.push_back(F.data());
+} else {
+  for (auto D : DbgOpts) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back(D);
+  }
+}
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))


Index: clang/test/Driver/ps4-ps5-linker-jmc.c
===
--- clang/test/Driver/ps4-ps5-linker-jmc.c
+++ clang/test/Driver/ps4-ps5-linker-jmc.c
@@ -6,10 +6,10 @@
 // RUN: %clang --target=x86_64-scei-ps5 -fjmc %s -### 2>&1 | FileCheck --check-prefixes=CHECK-PS5,CHECK-PS5-LIB %s
 // RUN: %clang --target=x86_64-scei-ps5 -flto -fjmc %s 

[PATCH] D119207: [CUDA][SPIRV] Assign global address space to CUDA kernel arguments

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Hi Shangwu,

I've reverted this change to unblock the buildbots and our internal CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119207

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

[PATCH] D109857: [test] Fix test failure in Clang :: Lexer/char-escapes-delimited.c

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109857

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

[PATCH] D109857: [test] Fix test failure in Clang :: Lexer/char-escapes-delimited.c

[Matthew Voss via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG003fed8269b3: [test] Fix test failure in Clang :: 
Lexer/char-escapes-delimited.c (authored by ormris).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109857

Files:
  clang/test/Lexer/char-escapes-delimited.c


Index: clang/test/Lexer/char-escapes-delimited.c
===
--- clang/test/Lexer/char-escapes-delimited.c
+++ clang/test/Lexer/char-escapes-delimited.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -fwchar-type=short -fno-signed-wchar -fsyntax-only 
-pedantic -verify %s
-// RUN: %clang_cc1 -x c -fwchar-type=short -fno-signed-wchar -fsyntax-only 
-pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu11 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -fwchar-type=short -fno-signed-wchar 
-fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu11 -fwchar-type=short -fno-signed-wchar 
-fsyntax-only -pedantic -verify %s
 
 const char *errors =
 "\u{}"  //expected-error {{delimited escape sequence cannot be empty}}


Index: clang/test/Lexer/char-escapes-delimited.c
===
--- clang/test/Lexer/char-escapes-delimited.c
+++ clang/test/Lexer/char-escapes-delimited.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu11 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu11 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
 
 const char *errors =
 "\u{}"  //expected-error {{delimited escape sequence cannot be empty}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109857: [test] Fix test failure in Clang :: Lexer/char-escapes-delimited.c

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

I think the test fix is all that's needed for now. This is somewhat urgent for 
us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109857

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

[PATCH] D109857: [test] Fix test failure in Clang :: Lexer/char-escapes-delimited.c

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 372839.
ormris added a comment.

- Downgrade to C11 and C++11


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109857

Files:
  clang/test/Lexer/char-escapes-delimited.c


Index: clang/test/Lexer/char-escapes-delimited.c
===
--- clang/test/Lexer/char-escapes-delimited.c
+++ clang/test/Lexer/char-escapes-delimited.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -fwchar-type=short -fno-signed-wchar -fsyntax-only 
-pedantic -verify %s
-// RUN: %clang_cc1 -x c -fwchar-type=short -fno-signed-wchar -fsyntax-only 
-pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu11 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -fwchar-type=short -fno-signed-wchar 
-fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu11 -fwchar-type=short -fno-signed-wchar 
-fsyntax-only -pedantic -verify %s
 
 const char *errors =
 "\u{}"  //expected-error {{delimited escape sequence cannot be empty}}


Index: clang/test/Lexer/char-escapes-delimited.c
===
--- clang/test/Lexer/char-escapes-delimited.c
+++ clang/test/Lexer/char-escapes-delimited.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu11 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu11 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
 
 const char *errors =
 "\u{}"  //expected-error {{delimited escape sequence cannot be empty}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109857: [test] Fix test failure in Clang :: Lexer/char-escapes-delimited.c

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

We saw this failure for our internal SIE (PlayStation) platform. I just tested 
on x86_64-unknown-linux-gnu using C99 and C++98, though, and the test fails. It 
looks like this feature is restricted to more recent standard versions somehow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109857

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

[PATCH] D109857: [test] Fix test failure in Clang :: Lexer/char-escapes-delimited.c

[Matthew Voss via Phabricator via cfe-commits]

ormris created this revision.
ormris added reviewers: cor3ntin, aaron.ballman.
ormris requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Specify the C and C++ standards explicitly for this test. This avoids failures 
for drivers that default to older standards.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109857

Files:
  clang/test/Lexer/char-escapes-delimited.c


Index: clang/test/Lexer/char-escapes-delimited.c
===
--- clang/test/Lexer/char-escapes-delimited.c
+++ clang/test/Lexer/char-escapes-delimited.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -fwchar-type=short -fno-signed-wchar -fsyntax-only 
-pedantic -verify %s
-// RUN: %clang_cc1 -x c -fwchar-type=short -fno-signed-wchar -fsyntax-only 
-pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++20 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu2x -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++20 -fwchar-type=short -fno-signed-wchar 
-fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu2x -fwchar-type=short -fno-signed-wchar 
-fsyntax-only -pedantic -verify %s
 
 const char *errors =
 "\u{}"  //expected-error {{delimited escape sequence cannot be empty}}


Index: clang/test/Lexer/char-escapes-delimited.c
===
--- clang/test/Lexer/char-escapes-delimited.c
+++ clang/test/Lexer/char-escapes-delimited.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
-// RUN: %clang_cc1 -x c -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++20 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu2x -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c++ -std=gnu++20 -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -x c -std=gnu2x -fwchar-type=short -fno-signed-wchar -fsyntax-only -pedantic -verify %s
 
 const char *errors =
 "\u{}"  //expected-error {{delimited escape sequence cannot be empty}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

This is also failing on several build bots: 
https://lab.llvm.org/buildbot/#/builders/139/builds/4812

Since it's been failing for a few hours, I'd like to revert this in the near 
future to unclog our internal CI and the buildbots.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91630

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

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

@hoyFB wrote:

> @ormris I'd like to follow up on this. We had a similar change internally 
> which led up to some gains when combined with SPGO, and we'd like to help 
> move forward with this patch here. Would you mind sharing the plan or 
> progress on your side? Thanks!

Sorry for the late reply. Unfortunately, we weren't able to get very far beyond 
the experiments you see here. Feel free to take it from here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69732

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

[PATCH] D98873: Document -fcrash-diagnostics-dir

[Matthew Voss via Phabricator via cfe-commits]

ormris accepted this revision.
ormris added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98873

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

[PATCH] D94327: [NFC] Specify C11 in loop-opt-setup.c

[Matthew Voss via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0386f3d4f418: [NFC] Specify C11 in loop-opt-setup.c 
(authored by ormris).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94327

Files:
  clang/test/Misc/loop-opt-setup.c


Index: clang/test/Misc/loop-opt-setup.c
===
--- clang/test/Misc/loop-opt-setup.c
+++ clang/test/Misc/loop-opt-setup.c
@@ -1,5 +1,5 @@
 // This tests loop unrolling and loop deletion (enabled under -O1)
-// RUN: %clang_cc1 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+// RUN: %clang_cc1 -std=c11 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | 
FileCheck %s
 // RUN: %clang_cc1 -std=c99 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | 
FileCheck %s --check-prefix C99
 
 extern int a[16];


Index: clang/test/Misc/loop-opt-setup.c
===
--- clang/test/Misc/loop-opt-setup.c
+++ clang/test/Misc/loop-opt-setup.c
@@ -1,5 +1,5 @@
 // This tests loop unrolling and loop deletion (enabled under -O1)
-// RUN: %clang_cc1 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+// RUN: %clang_cc1 -std=c11 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
 // RUN: %clang_cc1 -std=c99 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s --check-prefix C99
 
 extern int a[16];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94327: [NFC] Specify C11 in loop-opt-setup.c

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94327

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

[PATCH] D94327: [NFC] Specify C11 in loop-opt-setup.c

[Matthew Voss via Phabricator via cfe-commits]

ormris created this revision.
ormris added a reviewer: atmnpatel.
ormris requested review of this revision.
Herald added a project: clang.

This test was failing in our internal CI, since our driver does not default to 
C11. Adding this switch fixes the issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94327

Files:
  clang/test/Misc/loop-opt-setup.c


Index: clang/test/Misc/loop-opt-setup.c
===
--- clang/test/Misc/loop-opt-setup.c
+++ clang/test/Misc/loop-opt-setup.c
@@ -1,5 +1,5 @@
 // This tests loop unrolling and loop deletion (enabled under -O1)
-// RUN: %clang_cc1 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+// RUN: %clang_cc1 -std=c11 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | 
FileCheck %s
 // RUN: %clang_cc1 -std=c99 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | 
FileCheck %s --check-prefix C99
 
 extern int a[16];


Index: clang/test/Misc/loop-opt-setup.c
===
--- clang/test/Misc/loop-opt-setup.c
+++ clang/test/Misc/loop-opt-setup.c
@@ -1,5 +1,5 @@
 // This tests loop unrolling and loop deletion (enabled under -O1)
-// RUN: %clang_cc1 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+// RUN: %clang_cc1 -std=c11 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
 // RUN: %clang_cc1 -std=c99 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s --check-prefix C99
 
 extern int a[16];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79961: [PGO] Fix computation of fuction Hash

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

There's a few test failures related to this change on the PS4 bot. Could you 
take a look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/68111


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79961



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

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Hi Zach,

The tests in this patch are failing on this PS4 windows bot. Could you take a 
look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/30273/steps/test-check-all/logs/stdio

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72405



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

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

//Ping// @tejohnson


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69732



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

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

I've done testing with the following global parameters.

- The base for the branch is llvmorg-10-init-8655-g94a4a2c97f8
- Used llvm, clang, lld, and llvm-ar from this branch.
- The sqlite kvtest program was the test payload.

This test compared an unmodified compiler from the base of the branch with a 
modified compiler with this patch applied and the loop optimisation passes 
mentioned above moved to the backend. The results were as follows. All numbers 
in seconds.

| Run  | Modified LTO | Modified SPGO+LTO | Unmodified SPGO+LTO |
|  |  | - | --- |
| 1| 42.00| 41.73 | 42.08   |
| 2| 42.30| 39.49 | 42.45   |
| 3| 41.21| 42.46 | 42.49   |
| AVG: | 41.84| 41.23 | 42.34   |
|

TL;DR the average run using a compiler built with the modified SPGO pipeline is 
about a second faster. Definitely a positive initial result.

In D69732#1730732 , @tejohnson wrote:

> This probably needs to be taken over by someone who cares about full LTO 
> performance (@wristow or @ormris ?). This patch was some cleanup of the full 
> LTO sample PGO pipeline, but has a number of issues I enumerate in the 
> summary.


Given the performance improvements here, I'd like to develop this patch further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69732



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

[PATCH] D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: clang-tools-extra/unittests/clangd/HeadersTests.cpp:220
+  BarHeader = testPath("sub/bar.h");
+  EXPECT_EQ(calculate(BarHeader), "\"sub/bar.h\"");
 }

sammccall wrote:
> ormris wrote:
> > Quick nit: This test won't work on Windows as it only tests for POSIX-style 
> > path separators. You could easily add a Windows ifdef, though.
> testPath() returns an appropriate native path, and calculate() returns an 
> #include string (which always uses /).
> The "../" on line 217 looks a little more suspicious, but I think the clang 
> driver might silently Do What I Mean here.
> Are you seeing test failures?
Yes, here's the output.

```
[ RUN  ] HeadersTest.ShortenedInclude
C:\llvm\tools\clang\tools\extra\unittests\clangd\HeadersTests.cpp(220): error:  
 Expected: calculate(BarHeader)
  Which is: "\"sub\\bar.h\""
To be equal to: "\"sub/bar.h\""
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60873



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

[PATCH] D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: clang-tools-extra/unittests/clangd/HeadersTests.cpp:220
+  BarHeader = testPath("sub/bar.h");
+  EXPECT_EQ(calculate(BarHeader), "\"sub/bar.h\"");
 }

Quick nit: This test won't work on Windows as it only tests for POSIX-style 
path separators. You could easily add a Windows ifdef, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60873



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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: test/ClangScanDeps/regular_cdb.cpp:14
+// CHECK: regular_cdb.cpp
+// CHECK-NEXT: Inputs/header.h

Quick drive-by nit: This test won't work under Windows due to the path 
separator. To account for both platforms, you could use a pattern like 
"{{/|\\}}".


Repository:
  rC Clang

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

https://reviews.llvm.org/D60233



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

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Hi Ilya,

This commit is causing a test failure on the PS4 bot. Could you take a look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/42251

Thanks,
Matthew


Repository:
  rC Clang

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

https://reviews.llvm.org/D54630



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

[PATCH] D52058: Add template type and value parameter metadata nodes to template variable specializations

[Matthew Voss via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC343707: Add template type and value parameter metadata nodes 
to template variable… (authored by ormris, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52058?vs=167849=168152#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52058

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-info-template-member.cpp

Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -248,6 +248,11 @@
   llvm::DINodeArray CollectFunctionTemplateParams(const FunctionDecl *FD,
   llvm::DIFile *Unit);
 
+  /// A helper function to collect debug info for function template
+  /// parameters.
+  llvm::DINodeArray CollectVarTemplateParams(const VarDecl *VD,
+ llvm::DIFile *Unit);
+
   /// A helper function to collect debug info for template
   /// parameters.
   llvm::DINodeArray
@@ -645,7 +650,9 @@
   /// Collect various properties of a VarDecl.
   void collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
unsigned , QualType , StringRef ,
-   StringRef , llvm::DIScope *);
+   StringRef ,
+   llvm::MDTuple *,
+   llvm::DIScope *);
 
   /// Allocate a copy of \p A using the DebugInfoNames allocator
   /// and return a reference to it. If multiple arguments are given the strings
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -41,6 +41,7 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
@@ -1776,6 +1777,29 @@
   return llvm::DINodeArray();
 }
 
+llvm::DINodeArray CGDebugInfo::CollectVarTemplateParams(const VarDecl *VL,
+llvm::DIFile *Unit) {
+  if (auto *TS = dyn_cast(VL)) {
+auto T = TS->getSpecializedTemplateOrPartial();
+auto TA = TS->getTemplateArgs().asArray();
+// Collect parameters for a partial specialization
+if (T.is()) {
+  const TemplateParameterList *TList =
+T.get()
+->getTemplateParameters();
+  return CollectTemplateParams(TList, TA, Unit);
+}
+
+// Collect parameters for an explicit specialization
+if (T.is()) {
+  const TemplateParameterList *TList = T.get()
+->getTemplateParameters();
+  return CollectTemplateParams(TList, TA, Unit);
+}
+  }
+  return llvm::DINodeArray();
+}
+
 llvm::DINodeArray CGDebugInfo::CollectCXXTemplateParams(
 const ClassTemplateSpecializationDecl *TSpecial, llvm::DIFile *Unit) {
   // Always get the full list of parameters, not just the ones from
@@ -3070,6 +3094,7 @@
 void CGDebugInfo::collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
   unsigned , QualType ,
   StringRef , StringRef ,
+  llvm::MDTuple *,
   llvm::DIScope *) {
   Unit = getOrCreateFile(VD->getLocation());
   LineNo = getLineNumber(VD->getLocation());
@@ -3093,6 +3118,13 @@
   if (LinkageName == Name)
 LinkageName = StringRef();
 
+  if (isa(VD)) {
+llvm::DINodeArray parameterNodes = CollectVarTemplateParams(VD, &*Unit);
+TemplateParameters = parameterNodes.get();
+  } else {
+TemplateParameters = nullptr;
+  }
+
   // Since we emit declarations (DW_AT_members) for static members, place the
   // definition of those static members in the namespace they were declared in
   // in the source code (the lexical decl context).
@@ -3173,12 +3205,14 @@
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   llvm::DIScope *DContext = Unit;
   unsigned Line = getLineNumber(Loc);
+  llvm::MDTuple *TemplateParameters = nullptr;
 
-  collectVarDeclProps(VD, Unit, Line, T, Name, LinkageName, DContext);
+  collectVarDeclProps(VD, Unit, Line, T, Name, LinkageName, TemplateParameters,
+  DContext);
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   auto *GV = DBuilder.createTempGlobalVariableFwdDecl(
   DContext, Name, LinkageName, Unit, Line, getOrCreateType(T, Unit),
-  !VD->isExternallyVisible(), nullptr, Align);
+  !VD->isExternallyVisible(), nullptr, TemplateParameters, Align);
   FwdDeclReplaceMap.emplace_back(
   std::piecewise_construct,
   std::make_tuple(cast(VD->getCanonicalDecl())),
@@ -4089,7 +4123,9 @@
   unsigned LineNo;
   StringRef DeclName, LinkageName;
   QualType T;
-  collectVarDeclProps(D, Unit, LineNo, T, DeclName, LinkageName, DContext);
+  llvm::MDTuple 

[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks! I'll go ahead and commit this.


Repository:
  rC Clang

https://reviews.llvm.org/D52058



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

[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

//Ping Ping//


Repository:
  rC Clang

https://reviews.llvm.org/D52058



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

[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 167849.
ormris added a comment.

- Clarify CGDebugInfo::CollectVarTemplateParams


Repository:
  rC Clang

https://reviews.llvm.org/D52058

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-info-template-member.cpp

Index: test/CodeGenCXX/debug-info-template-member.cpp
===
--- test/CodeGenCXX/debug-info-template-member.cpp
+++ test/CodeGenCXX/debug-info-template-member.cpp
@@ -22,6 +22,19 @@
 // CHECK: [[X]] = !DIGlobalVariableExpression(var: [[XV:.*]], expr: !DIExpression())
 // CHECK: [[XV]] = distinct !DIGlobalVariable(name: "x",
 // CHECK-SAME:type: ![[OUTER_FOO_INNER_ID:[0-9]+]]
+//
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "var"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateTypeParameter(name: "T", type: [[TY:![0-9]+]])
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "var"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateTypeParameter(name: "P", type: {{![0-9]+}})
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "varray"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateValueParameter(name: "N", type: [[TY]], value: i32 1)
 
 // CHECK: ![[OUTER_FOO_INNER_ID:[0-9]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "inner"{{.*}}, identifier:
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
@@ -103,3 +116,15 @@
 // declaration/reference in the compile unit.
 // CHECK: !DISubprogram(name: "MyClass"
 // CHECK-SAME:  scope: [[C]]
+
+template 
+T var = T();
+template 
+P var = P();
+template 
+T varray[N];
+void f3() {
+  var = 1;
+  var = 1;
+  varray[0] = 1;
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -248,6 +248,11 @@
   llvm::DINodeArray CollectFunctionTemplateParams(const FunctionDecl *FD,
   llvm::DIFile *Unit);
 
+  /// A helper function to collect debug info for function template
+  /// parameters.
+  llvm::DINodeArray CollectVarTemplateParams(const VarDecl *VD,
+ llvm::DIFile *Unit);
+
   /// A helper function to collect debug info for template
   /// parameters.
   llvm::DINodeArray
@@ -645,7 +650,9 @@
   /// Collect various properties of a VarDecl.
   void collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
unsigned , QualType , StringRef ,
-   StringRef , llvm::DIScope *);
+   StringRef ,
+   llvm::MDTuple *,
+   llvm::DIScope *);
 
   /// Allocate a copy of \p A using the DebugInfoNames allocator
   /// and return a reference to it. If multiple arguments are given the strings
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -41,6 +41,7 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
@@ -1776,6 +1777,29 @@
   return llvm::DINodeArray();
 }
 
+llvm::DINodeArray CGDebugInfo::CollectVarTemplateParams(const VarDecl *VL,
+llvm::DIFile *Unit) {
+  if (auto *TS = dyn_cast(VL)) {
+auto T = TS->getSpecializedTemplateOrPartial();
+auto TA = TS->getTemplateArgs().asArray();
+// Collect parameters for a partial specialization
+if (T.is()) {
+  const TemplateParameterList *TList =
+T.get()
+->getTemplateParameters();
+  return CollectTemplateParams(TList, TA, Unit);
+}
+
+// Collect parameters for an explicit specialization
+if (T.is()) {
+  const TemplateParameterList *TList = T.get()
+->getTemplateParameters();
+  return CollectTemplateParams(TList, TA, Unit);
+}
+  }
+  return llvm::DINodeArray();
+}
+
 llvm::DINodeArray CGDebugInfo::CollectCXXTemplateParams(
 const ClassTemplateSpecializationDecl *TSpecial, llvm::DIFile *Unit) {
   // Always get the full list of parameters, not just the ones from
@@ -3070,6 +3094,7 @@
 void CGDebugInfo::collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
   unsigned , QualType ,
   StringRef , StringRef ,
+  llvm::MDTuple *,
   llvm::DIScope *) {
   Unit = getOrCreateFile(VD->getLocation());
   LineNo = getLineNumber(VD->getLocation());
@@ -3093,6 +3118,13 @@
   if (LinkageName == Name)
 LinkageName = StringRef();
 
+  if (isa(VD)) {
+llvm::DINodeArray 

[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

//Ping//


Repository:
  rC Clang

https://reviews.llvm.org/D52058



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

[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

In https://reviews.llvm.org/D52058#1237868, @JDevlieghere wrote:

> Generally this looks good, but I'd like for the other to have a look first 
> (at this and the other patch) before accepting.


Sounds good. Thanks for your comments!




Comment at: lib/CodeGen/CGDebugInfo.cpp:1783
+  if (auto *TS = dyn_cast(VL)) {
+if (TS->getSpecializedTemplateOrPartial()
+.is()) {

JDevlieghere wrote:
> JDevlieghere wrote:
> > Might be nice to add a comment here saying what you're doing in this block 
> > and below. Looks like the top one is for partial specialization and the 
> > bottom one for the general case?
> I also suggest to extract `TS->getSpecializedTemplateOrPartial()` into a 
> variable to make this a little less dense.
Hmm... Yeah. I'll take a look at clarifying this section.


Repository:
  rC Clang

https://reviews.llvm.org/D52058



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

[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 165789.
ormris added a comment.

- Removed unneeded comment
- Renamed variable


Repository:
  rC Clang

https://reviews.llvm.org/D52058

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-info-template-member.cpp

Index: test/CodeGenCXX/debug-info-template-member.cpp
===
--- test/CodeGenCXX/debug-info-template-member.cpp
+++ test/CodeGenCXX/debug-info-template-member.cpp
@@ -22,6 +22,19 @@
 // CHECK: [[X]] = !DIGlobalVariableExpression(var: [[XV:.*]], expr: !DIExpression())
 // CHECK: [[XV]] = distinct !DIGlobalVariable(name: "x",
 // CHECK-SAME:type: ![[OUTER_FOO_INNER_ID:[0-9]+]]
+//
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "var"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateTypeParameter(name: "T", type: [[TY:![0-9]+]])
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "var"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateTypeParameter(name: "P", type: {{![0-9]+}})
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "varray"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateValueParameter(name: "N", type: [[TY]], value: i32 1)
 
 // CHECK: ![[OUTER_FOO_INNER_ID:[0-9]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "inner"{{.*}}, identifier:
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
@@ -103,3 +116,15 @@
 // declaration/reference in the compile unit.
 // CHECK: !DISubprogram(name: "MyClass"
 // CHECK-SAME:  scope: [[C]]
+
+template 
+T var = T();
+template 
+P var = P();
+template 
+T varray[N];
+void f3() {
+  var = 1;
+  var = 1;
+  varray[0] = 1;
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -248,6 +248,11 @@
   llvm::DINodeArray CollectFunctionTemplateParams(const FunctionDecl *FD,
   llvm::DIFile *Unit);
 
+  /// A helper function to collect debug info for function template
+  /// parameters.
+  llvm::DINodeArray CollectVarTemplateParams(const VarDecl *VD,
+ llvm::DIFile *Unit);
+
   /// A helper function to collect debug info for template
   /// parameters.
   llvm::DINodeArray
@@ -645,7 +650,9 @@
   /// Collect various properties of a VarDecl.
   void collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
unsigned , QualType , StringRef ,
-   StringRef , llvm::DIScope *);
+   StringRef ,
+   llvm::MDTuple *,
+   llvm::DIScope *);
 
   /// Allocate a copy of \p A using the DebugInfoNames allocator
   /// and return a reference to it. If multiple arguments are given the strings
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -41,6 +41,7 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
@@ -1776,6 +1777,30 @@
   return llvm::DINodeArray();
 }
 
+llvm::DINodeArray CGDebugInfo::CollectVarTemplateParams(const VarDecl *VL,
+llvm::DIFile *Unit) {
+  if (auto *TS = dyn_cast(VL)) {
+if (TS->getSpecializedTemplateOrPartial()
+.is()) {
+  const TemplateParameterList *TList =
+  TS->getSpecializedTemplateOrPartial()
+  .get()
+  ->getTemplateParameters();
+  return CollectTemplateParams(TList, TS->getTemplateArgs().asArray(),
+   Unit);
+}
+
+if (TS->getSpecializedTemplateOrPartial().is()) {
+  const TemplateParameterList *TList = TS->getSpecializedTemplateOrPartial()
+   .get()
+   ->getTemplateParameters();
+  return CollectTemplateParams(TList, TS->getTemplateArgs().asArray(),
+   Unit);
+}
+  }
+  return llvm::DINodeArray();
+}
+
 llvm::DINodeArray CGDebugInfo::CollectCXXTemplateParams(
 const ClassTemplateSpecializationDecl *TSpecial, llvm::DIFile *Unit) {
   // Always get the full list of parameters, not just the ones from
@@ -3070,6 +3095,7 @@
 void CGDebugInfo::collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
   unsigned , QualType ,
   StringRef , StringRef ,
+  llvm::MDTuple *,
   

[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:3126
+  } else {
+templateParameters = nullptr;//llvm::DINodeArray().get();
+  }

JDevlieghere wrote:
> What's the meaning of this comment? 
Hmm... That should be removed.



Comment at: lib/CodeGen/CGDebugInfo.h:654
+   StringRef ,
+   llvm::MDTuple *,
+   llvm::DIScope *);

JDevlieghere wrote:
> s/templateParameters/TemplateParameters/ (same for the rest of this patch)
OK. Will fix.


Repository:
  rC Clang

https://reviews.llvm.org/D52058



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

[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris created this revision.
ormris added reviewers: dblaikie, aprantl, probinson, JDevlieghere, clayborg, 
jingham.

This patch adds the associated template parameters to the DWARF name attribute
of all template variable specializations, mirroring how they are referenced in
the source code.

Original review: https://reviews.llvm.org/D44842

This version updates the LLVM-C interface.


Repository:
  rC Clang

https://reviews.llvm.org/D52058

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-info-template-member.cpp

Index: test/CodeGenCXX/debug-info-template-member.cpp
===
--- test/CodeGenCXX/debug-info-template-member.cpp
+++ test/CodeGenCXX/debug-info-template-member.cpp
@@ -22,6 +22,19 @@
 // CHECK: [[X]] = !DIGlobalVariableExpression(var: [[XV:.*]], expr: !DIExpression())
 // CHECK: [[XV]] = distinct !DIGlobalVariable(name: "x",
 // CHECK-SAME:type: ![[OUTER_FOO_INNER_ID:[0-9]+]]
+//
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "var"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateTypeParameter(name: "T", type: [[TY:![0-9]+]])
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "var"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateTypeParameter(name: "P", type: {{![0-9]+}})
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "varray"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateValueParameter(name: "N", type: [[TY]], value: i32 1)
 
 // CHECK: ![[OUTER_FOO_INNER_ID:[0-9]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "inner"{{.*}}, identifier:
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
@@ -103,3 +116,15 @@
 // declaration/reference in the compile unit.
 // CHECK: !DISubprogram(name: "MyClass"
 // CHECK-SAME:  scope: [[C]]
+
+template 
+T var = T();
+template 
+P var = P();
+template 
+T varray[N];
+void f3() {
+  var = 1;
+  var = 1;
+  varray[0] = 1;
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -248,6 +248,11 @@
   llvm::DINodeArray CollectFunctionTemplateParams(const FunctionDecl *FD,
   llvm::DIFile *Unit);
 
+  /// A helper function to collect debug info for function template
+  /// parameters.
+  llvm::DINodeArray CollectVarTemplateParams(const VarDecl *VD,
+ llvm::DIFile *Unit);
+
   /// A helper function to collect debug info for template
   /// parameters.
   llvm::DINodeArray
@@ -645,7 +650,9 @@
   /// Collect various properties of a VarDecl.
   void collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
unsigned , QualType , StringRef ,
-   StringRef , llvm::DIScope *);
+   StringRef ,
+   llvm::MDTuple *,
+   llvm::DIScope *);
 
   /// Allocate a copy of \p A using the DebugInfoNames allocator
   /// and return a reference to it. If multiple arguments are given the strings
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -41,6 +41,7 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
@@ -1776,6 +1777,30 @@
   return llvm::DINodeArray();
 }
 
+llvm::DINodeArray CGDebugInfo::CollectVarTemplateParams(const VarDecl *VL,
+llvm::DIFile *Unit) {
+  if (auto *TS = dyn_cast(VL)) {
+if (TS->getSpecializedTemplateOrPartial()
+.is()) {
+  const TemplateParameterList *TList =
+  TS->getSpecializedTemplateOrPartial()
+  .get()
+  ->getTemplateParameters();
+  return CollectTemplateParams(TList, TS->getTemplateArgs().asArray(),
+   Unit);
+}
+
+if (TS->getSpecializedTemplateOrPartial().is()) {
+  const TemplateParameterList *TList = TS->getSpecializedTemplateOrPartial()
+   .get()
+   ->getTemplateParameters();
+  return CollectTemplateParams(TList, TS->getTemplateArgs().asArray(),
+   Unit);
+}
+  }
+  return llvm::DINodeArray();
+}
+
 llvm::DINodeArray CGDebugInfo::CollectCXXTemplateParams(
 const ClassTemplateSpecializationDecl *TSpecial, llvm::DIFile *Unit) {
   // Always get the full list of parameters, not just the ones from
@@ -3070,6 +3095,7 @@
 void 

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

In https://reviews.llvm.org/D47044#1130339, @NoQ wrote:

> I'm still curious whether this also works:
>
>   void foo() {
> const A  = B();
> bar();
>   }
>  
>   void bar() {
> for (int i = 0; i < 10; ++i) {}
>   }
>
>
> Though we can land this patch and deal with this later.


@NoQ The above example doesn't crash.


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks @george.karpenkov . I was wondering why it didn't close automatically.


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris closed this revision.
ormris added a comment.

This change has been committed, so I'm closing this review.

https://reviews.llvm.org/rC334554


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Hmm... I'll take a look.


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris marked an inline comment as done.
ormris added a comment.

Sounds good. I'll go ahead and commit this. Thanks for the review!


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 151046.
ormris added a comment.

Remove unneeded header


Repository:
  rC Clang

https://reviews.llvm.org/D47044

Files:
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/loop-widening-preserve-reference-type.cpp


Index: test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 
-analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() {
+  const A  = B();
+  for (int i = 0; i < 10; ++i) { }
+  clang_analyzer_eval( != 0); // expected-warning{{TRUE}}
+}   // expected-warning@-1{{reference cannot be 
bound to dereferenced null pointer in well-defined C++ code; comparison may be 
assumed to always evaluate to true}}
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -14,10 +14,16 @@
 ///
 
//===--===//
 
+#include "clang/AST/AST.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
 
 using namespace clang;
 using namespace ento;
+using namespace clang::ast_matchers;
+
+const auto MatchRef = "matchref";
 
 /// Return the loops condition Stmt or NULL if LoopStmt is not a loop
 static const Expr *getLoopCondition(const Stmt *LoopStmt) {
@@ -49,6 +55,7 @@
   // TODO Nested loops are currently widened as a result of the invalidation
   //  being so inprecise. When the invalidation is improved, the handling
   //  of nested loops will also need to be improved.
+  ASTContext  = LCtx->getAnalysisDeclContext()->getASTContext();
   const StackFrameContext *STC = LCtx->getCurrentStackFrame();
   MemRegionManager  = PrevState->getStateManager().getRegionManager();
   const MemRegion *Regions[] = {MRMgr.getStackLocalsRegion(STC),
@@ -60,6 +67,18 @@
  RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);
   }
 
+  // References should not be invalidated.
+  auto Matches = 
match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef,
+   *LCtx->getDecl()->getBody(), ASTCtx);
+  for (BoundNodes Match : Matches) {
+const VarDecl *VD = Match.getNodeAs(MatchRef);
+assert(VD);
+const VarRegion *VarMem = MRMgr.getVarRegion(VD, LCtx);
+ITraits.setTrait(VarMem,
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  }
+
+
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
   // pointer shoule remain unchanged.


Index: test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() {
+  const A  = B();
+  for (int i = 0; i < 10; ++i) { }
+  clang_analyzer_eval( != 0); // expected-warning{{TRUE}}
+}   // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -14,10 +14,16 @@
 ///
 //===--===//
 
+#include "clang/AST/AST.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
 
 using namespace clang;
 using namespace ento;
+using namespace clang::ast_matchers;
+
+const auto MatchRef = "matchref";
 
 /// Return the loops condition Stmt or NULL if LoopStmt is not a loop
 static const Expr *getLoopCondition(const Stmt *LoopStmt) {
@@ -49,6 +55,7 @@
   // TODO Nested loops are currently widened as a result of the invalidation
   //  being so inprecise. When the invalidation is improved, the handling
   //  of nested loops will also need to be improved.
+  ASTContext  = 

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 151043.
ormris added a comment.

- Use match function iterators rather than a callback class
- Update test


Repository:
  rC Clang

https://reviews.llvm.org/D47044

Files:
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/loop-widening-preserve-reference-type.cpp


Index: test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 
-analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() {
+  const A  = B();
+  for (int i = 0; i < 10; ++i) { }
+  clang_analyzer_eval( != 0); // expected-warning{{TRUE}}
+}   // expected-warning@-1{{reference cannot be 
bound to dereferenced null pointer in well-defined C++ code; comparison may be 
assumed to always evaluate to true}}
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -14,10 +14,17 @@
 ///
 
//===--===//
 
+#include "clang/AST/AST.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
+#include "llvm/ADT/SmallSet.h"
 
 using namespace clang;
 using namespace ento;
+using namespace clang::ast_matchers;
+
+const auto MatchRef = "matchref";
 
 /// Return the loops condition Stmt or NULL if LoopStmt is not a loop
 static const Expr *getLoopCondition(const Stmt *LoopStmt) {
@@ -49,6 +56,7 @@
   // TODO Nested loops are currently widened as a result of the invalidation
   //  being so inprecise. When the invalidation is improved, the handling
   //  of nested loops will also need to be improved.
+  ASTContext  = LCtx->getAnalysisDeclContext()->getASTContext();
   const StackFrameContext *STC = LCtx->getCurrentStackFrame();
   MemRegionManager  = PrevState->getStateManager().getRegionManager();
   const MemRegion *Regions[] = {MRMgr.getStackLocalsRegion(STC),
@@ -60,6 +68,18 @@
  RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);
   }
 
+  // References should not be invalidated.
+  auto Matches = 
match(findAll(stmt(hasDescendant(varDecl(hasType(referenceType())).bind(MatchRef,
+   *LCtx->getDecl()->getBody(), ASTCtx);
+  for (BoundNodes Match : Matches) {
+const VarDecl *VD = Match.getNodeAs(MatchRef);
+assert(VD);
+const VarRegion *VarMem = MRMgr.getVarRegion(VD, LCtx);
+ITraits.setTrait(VarMem,
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  }
+
+
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
   // pointer shoule remain unchanged.


Index: test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() {
+  const A  = B();
+  for (int i = 0; i < 10; ++i) { }
+  clang_analyzer_eval( != 0); // expected-warning{{TRUE}}
+}   // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -14,10 +14,17 @@
 ///
 //===--===//
 
+#include "clang/AST/AST.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
+#include "llvm/ADT/SmallSet.h"
 
 using namespace clang;
 using namespace ento;
+using namespace clang::ast_matchers;
+
+const auto MatchRef = "matchref";
 
 /// Return the loops condition Stmt or NULL if LoopStmt is not a loop
 static const Expr *getLoopCondition(const Stmt *LoopStmt) {
@@ -49,6 +56,7 @@
   // TODO Nested loops are currently widened as a result of the invalidation
   //  being so inprecise. When the 

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:89
+new Callback(LCtx, MRMgr, ITraits));
+  Finder.matchAST(ASTCtx);
+

NoQ wrote:
> ormris wrote:
> > ormris wrote:
> > > NoQ wrote:
> > > > NoQ wrote:
> > > > > george.karpenkov wrote:
> > > > > > ormris wrote:
> > > > > > > george.karpenkov wrote:
> > > > > > > > IMO using the iterator directly (e.g. like it was done in 
> > > > > > > > https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp#L213)
> > > > > > > >  leads to a much cleaner code and saves you from having to 
> > > > > > > > define a callback class.
> > > > > > > Hmm... I think that's a better approach. Let me see if I can get 
> > > > > > > that working.
> > > > > > @ormris Yeah I'm really not sure why all examples use the callback 
> > > > > > API by default.
> > > > > Also, please match only the local AST, i.e. the body of the function 
> > > > > under analysis, which can be obtained as 
> > > > > `LCtx->getDecl()->getBody()`. There's no need to scan the whole 
> > > > > translation unit.
> > > > Actually not sure, would widening screw the previous stack frames as 
> > > > well? We should test that, i guess. And even then, it's better to match 
> > > > a few stack frames in the current stack trace than to match the whole 
> > > > translation unit.
> > > I can't seem to get the new syntax to work. The following assert(0) is 
> > > never triggered.
> > > 
> > > ```
> > >   auto Matches = match(varDecl(hasType(referenceType())).bind(MatchRef),
> > >*LCtx->getDecl()->getBody(), ASTCtx);
> > >   for (BoundNodes Match : Matches) {
> > > assert(0 && "anything");
> > > const VarDecl *VD = Match.getNodeAs(MatchRef);
> > > const VarRegion *VarMem = MRMgr.getVarRegion(VD, LCtx);
> > > ITraits.setTrait(VarMem,
> > >  
> > > RegionAndSymbolInvalidationTraits::TK_PreserveContents);
> > >   }
> > > 
> > > ```
> > It appears that "decl()" produces no matches...
> Mmm, i think when you're matching using `match` rather than `matchAST`, you 
> need to write a match for the exact statement rather than any sub-statement. 
> I.e., those are like "match" vs. "find". I.e., try wraping your matcher into 
> `stmt(hasDescendant(...))`, where `stmt()` would match the whole function 
> body (most likely a `CompoundStmt` for the curly braces around the function 
> body, but there are some other cases).
Looks like that worked. Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:89
+new Callback(LCtx, MRMgr, ITraits));
+  Finder.matchAST(ASTCtx);
+

ormris wrote:
> NoQ wrote:
> > NoQ wrote:
> > > george.karpenkov wrote:
> > > > ormris wrote:
> > > > > george.karpenkov wrote:
> > > > > > IMO using the iterator directly (e.g. like it was done in 
> > > > > > https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp#L213)
> > > > > >  leads to a much cleaner code and saves you from having to define a 
> > > > > > callback class.
> > > > > Hmm... I think that's a better approach. Let me see if I can get that 
> > > > > working.
> > > > @ormris Yeah I'm really not sure why all examples use the callback API 
> > > > by default.
> > > Also, please match only the local AST, i.e. the body of the function 
> > > under analysis, which can be obtained as `LCtx->getDecl()->getBody()`. 
> > > There's no need to scan the whole translation unit.
> > Actually not sure, would widening screw the previous stack frames as well? 
> > We should test that, i guess. And even then, it's better to match a few 
> > stack frames in the current stack trace than to match the whole translation 
> > unit.
> I can't seem to get the new syntax to work. The following assert(0) is never 
> triggered.
> 
> ```
>   auto Matches = match(varDecl(hasType(referenceType())).bind(MatchRef),
>*LCtx->getDecl()->getBody(), ASTCtx);
>   for (BoundNodes Match : Matches) {
> assert(0 && "anything");
> const VarDecl *VD = Match.getNodeAs(MatchRef);
> const VarRegion *VarMem = MRMgr.getVarRegion(VD, LCtx);
> ITraits.setTrait(VarMem,
>  RegionAndSymbolInvalidationTraits::TK_PreserveContents);
>   }
> 
> ```
It appears that "decl()" produces no matches...


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:89
+new Callback(LCtx, MRMgr, ITraits));
+  Finder.matchAST(ASTCtx);
+

NoQ wrote:
> NoQ wrote:
> > george.karpenkov wrote:
> > > ormris wrote:
> > > > george.karpenkov wrote:
> > > > > IMO using the iterator directly (e.g. like it was done in 
> > > > > https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp#L213)
> > > > >  leads to a much cleaner code and saves you from having to define a 
> > > > > callback class.
> > > > Hmm... I think that's a better approach. Let me see if I can get that 
> > > > working.
> > > @ormris Yeah I'm really not sure why all examples use the callback API by 
> > > default.
> > Also, please match only the local AST, i.e. the body of the function under 
> > analysis, which can be obtained as `LCtx->getDecl()->getBody()`. There's no 
> > need to scan the whole translation unit.
> Actually not sure, would widening screw the previous stack frames as well? We 
> should test that, i guess. And even then, it's better to match a few stack 
> frames in the current stack trace than to match the whole translation unit.
I can't seem to get the new syntax to work. The following assert(0) is never 
triggered.

```
  auto Matches = match(varDecl(hasType(referenceType())).bind(MatchRef),
   *LCtx->getDecl()->getBody(), ASTCtx);
  for (BoundNodes Match : Matches) {
assert(0 && "anything");
const VarDecl *VD = Match.getNodeAs(MatchRef);
const VarRegion *VarMem = MRMgr.getVarRegion(VD, LCtx);
ITraits.setTrait(VarMem,
 RegionAndSymbolInvalidationTraits::TK_PreserveContents);
  }

```


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:89
+new Callback(LCtx, MRMgr, ITraits));
+  Finder.matchAST(ASTCtx);
+

george.karpenkov wrote:
> IMO using the iterator directly (e.g. like it was done in 
> https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp#L213)
>  leads to a much cleaner code and saves you from having to define a callback 
> class.
Hmm... I think that's a better approach. Let me see if I can get that working.


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris marked 2 inline comments as done.
ormris added a comment.

Thanks for the review @MTC! I'll wait for @NoQ's feedback.


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris marked 3 inline comments as done.
ormris added a comment.

Thanks for the comments so far.




Comment at: test/Analysis/loop-widening-invalid-type.cpp:1
+// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 
-analyzer-config widen-loops=true -verify %s
+

MTC wrote:
> I think it's better to add more expressive tests. Like:
> 
> ```
> struct A {
>   int x;
>   A(int x) : x(x) {}
> };
> 
> void invalid_type_region_access() {
>   const A  = A(10);
>   for(int i = 0; i < 10; ++i) {}
>   clang_analyzer_eval(a.x ==10); // expected-warning{{TRUE}}
> }
> ```
> 
> I think should use more related names instead of 
> `loop-widening-invalid-type.cpp`, like `loop-widening-reference-type`.
Agreed. Fixed.



Comment at: test/Analysis/loop-widening-invalid-type.cpp:8
+
+void invalid_type_region_access() { // expected-no-diagnostics
+  const A  = B();

MTC wrote:
> I don't know what the purpose of the test is, is the comment `no-crash` 
> better?
I've changed the test to (hopefully) look for a valid address for "x".


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 150567.
ormris added a comment.
Herald added a subscriber: mikhail.ramalho.

- Reformat with clang-format-diff.py
- Rename test
- Modify test to use clang_analyzer_eval


Repository:
  rC Clang

https://reviews.llvm.org/D47044

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/loop-widening-preserve-reference-type.cpp

Index: test/Analysis/loop-widening-preserve-reference-type.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-preserve-reference-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() {
+  const A  = B();
+  for (int i = 0; i < 10; ++i) { }
+  clang_analyzer_eval( == ); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -15,9 +15,15 @@
 //===--===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
+#include "clang/AST/AST.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
+#include "llvm/ADT/SmallSet.h"
 
 using namespace clang;
 using namespace ento;
+using namespace clang::ast_matchers;
 
 /// Return the loops condition Stmt or NULL if LoopStmt is not a loop
 static const Expr *getLoopCondition(const Stmt *LoopStmt) {
@@ -33,10 +39,26 @@
   }
 }
 
+struct Callback : public MatchFinder::MatchCallback {
+  const LocationContext *LCtx;
+  MemRegionManager 
+  RegionAndSymbolInvalidationTraits 
+  explicit Callback(const LocationContext *LCtx_, MemRegionManager _,
+RegionAndSymbolInvalidationTraits _)
+  : LCtx(LCtx_), MRMgr(MRMgr_), ITraits(ITraits_) {}
+  virtual void run(const MatchFinder::MatchResult ) override {
+const VarDecl *VD = Result.Nodes.getNodeAs("match");
+const VarRegion *VarMem = MRMgr.getVarRegion(VD, LCtx);
+ITraits.setTrait(VarMem,
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  }
+};
+
 namespace clang {
 namespace ento {
 
 ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
+ASTContext ,
 const LocationContext *LCtx,
 unsigned BlockCount, const Stmt *LoopStmt) {
 
@@ -60,6 +82,12 @@
  RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);
   }
 
+  // References should not be invalidated.
+  MatchFinder Finder;
+  Finder.addMatcher(varDecl(hasType(referenceType())).bind("match"),
+new Callback(LCtx, MRMgr, ITraits));
+  Finder.matchAST(ASTCtx);
+
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
   // pointer shoule remain unchanged.
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1854,8 +1854,8 @@
   return;
 // Widen.
 const LocationContext *LCtx = Pred->getLocationContext();
-ProgramStateRef WidenedState =
-getWidenedLoopState(Pred->getState(), LCtx, BlockCount, Term);
+ProgramStateRef WidenedState = getWidenedLoopState(
+Pred->getState(), AMgr.getASTContext(), LCtx, BlockCount, Term);
 nodeBuilder.generateNode(WidenedState, Pred);
 return;
   }
Index: include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
@@ -27,6 +27,7 @@
 /// Widen the loop by invalidating anything that might be modified
 /// by the loop body in any iteration.
 ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
+ASTContext ,
 const LocationContext *LCtx,
 unsigned BlockCount, const Stmt *LoopStmt);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris updated this revision to Diff 150377.
ormris added a comment.

Use AST matchers to select references for preservation


Repository:
  rC Clang

https://reviews.llvm.org/D47044

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/loop-widening-invalid-type.cpp

Index: test/Analysis/loop-widening-invalid-type.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-invalid-type.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() { // expected-no-diagnostics
+  const A  = B();
+  for(int i = 0; i < 10; ++i) {}
+}
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -15,9 +15,15 @@
 //===--===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
+#include "clang/AST/AST.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
+#include "llvm/ADT/SmallSet.h"
 
 using namespace clang;
 using namespace ento;
+using namespace clang::ast_matchers;
 
 /// Return the loops condition Stmt or NULL if LoopStmt is not a loop
 static const Expr *getLoopCondition(const Stmt *LoopStmt) {
@@ -33,10 +39,27 @@
   }
 }
 
+struct Callback : public MatchFinder::MatchCallback {
+  const LocationContext *LCtx;
+  MemRegionManager 
+  RegionAndSymbolInvalidationTraits 
+  explicit Callback(const LocationContext *LCtx_,
+MemRegionManager _,
+RegionAndSymbolInvalidationTraits _) : LCtx(LCtx_),
+   MRMgr(MRMgr_),
+   ITraits(ITraits_) {}
+  virtual void run(const MatchFinder::MatchResult ) override {
+const VarDecl *VD = Result.Nodes.getNodeAs("match");
+const VarRegion *VarMem = MRMgr.getVarRegion(VD, LCtx);
+ITraits.setTrait(VarMem,
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  }
+};
+
 namespace clang {
 namespace ento {
 
-ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
+ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState, ASTContext ,
 const LocationContext *LCtx,
 unsigned BlockCount, const Stmt *LoopStmt) {
 
@@ -60,6 +83,12 @@
  RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);
   }
 
+  //References should not be invalidated.
+  MatchFinder Finder;
+  Finder.addMatcher(varDecl(hasType(referenceType())).bind("match"), new Callback(LCtx, MRMgr, ITraits));
+  Finder.matchAST(ASTCtx);
+
+
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
   // pointer shoule remain unchanged.
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1855,7 +1855,7 @@
 // Widen.
 const LocationContext *LCtx = Pred->getLocationContext();
 ProgramStateRef WidenedState =
-getWidenedLoopState(Pred->getState(), LCtx, BlockCount, Term);
+getWidenedLoopState(Pred->getState(), AMgr.getASTContext(), LCtx, BlockCount, Term);
 nodeBuilder.generateNode(WidenedState, Pred);
 return;
   }
Index: include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
@@ -26,7 +26,7 @@
 ///
 /// Widen the loop by invalidating anything that might be modified
 /// by the loop body in any iteration.
-ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
+ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState, ASTContext ,
 const LocationContext *LCtx,
 unsigned BlockCount, const Stmt *LoopStmt);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Hmm... Thanks for these tips. I'll see what I can find.


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

This is my first static analyzer fix, so it may take me some time to figure 
this out.


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

That could work. Is the implementation similar?


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47044: Ensure that we only visit a destructor for a reference if type information is available.

[Matthew Voss via Phabricator via cfe-commits]

ormris created this revision.
ormris added reviewers: dcoughlin, NoQ, xazax.hun, george.karpenkov.
Herald added a subscriber: rnkovacs.

Loop widening can invalidate an object reference. If the analyzer attempts to 
visit the destructor to a non-existent object it will crash. This patch ensures 
that type information is available before attempting to visit the object.


Repository:
  rC Clang

https://reviews.llvm.org/D47044

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/loop-widening-invalid-type.cpp


Index: test/Analysis/loop-widening-invalid-type.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-invalid-type.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 
-analyzer-config widen-loops=true -verify %s
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() { // expected-no-diagnostics
+  const A  = B();
+  for(int i = 0; i < 10; ++i) {}
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1044,6 +1044,10 @@
   return;
 }
 Region = ValueRegion->getBaseRegion();
+if (!isa(Region))
+  // Loop widening will sometimes invalidate typed regions.
+  return;
+ 
 varType = cast(Region)->getValueType();
   }
 


Index: test/Analysis/loop-widening-invalid-type.cpp
===
--- /dev/null
+++ test/Analysis/loop-widening-invalid-type.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+struct A {
+  ~A() {}
+};
+struct B : public A {};
+
+void invalid_type_region_access() { // expected-no-diagnostics
+  const A  = B();
+  for(int i = 0; i < 10; ++i) {}
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1044,6 +1044,10 @@
   return;
 }
 Region = ValueRegion->getBaseRegion();
+if (!isa(Region))
+  // Loop widening will sometimes invalidate typed regions.
+  return;
+ 
 varType = cast(Region)->getValueType();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46139: Add template type and value parameter metadata nodes to template variable specializations.

[Matthew Voss via Phabricator via cfe-commits]

ormris abandoned this revision.
ormris added a comment.

After some further inspection, support for the LLVM side of patch needs a few 
significant additions. Abandoning this revision.


Repository:
  rC Clang

https://reviews.llvm.org/D46139



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

[PATCH] D46139: Add template type and value parameter metadata nodes to template variable specializations.

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Thanks for spotting that @probinson. Will update.


Repository:
  rC Clang

https://reviews.llvm.org/D46139



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

[PATCH] D46139: Add template type and value parameter metadata nodes to template variable specializations.

[Matthew Voss via Phabricator via cfe-commits]

ormris created this revision.
ormris added reviewers: dblaikie, probinson, aprantl, JDevlieghere, clayborg.

Depends on LLVM patch: https://reviews.llvm.org/D46138

Resolves PR22119


Repository:
  rC Clang

https://reviews.llvm.org/D46139

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGenCXX/debug-info-template-member.cpp

Index: test/CodeGenCXX/debug-info-template-member.cpp
===
--- test/CodeGenCXX/debug-info-template-member.cpp
+++ test/CodeGenCXX/debug-info-template-member.cpp
@@ -22,6 +22,19 @@
 // CHECK: [[X]] = !DIGlobalVariableExpression(var: [[XV:.*]], expr: !DIExpression())
 // CHECK: [[XV]] = distinct !DIGlobalVariable(name: "x",
 // CHECK-SAME:type: ![[OUTER_FOO_INNER_ID:[0-9]+]]
+//
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "var"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateTypeParameter(name: "T", type: [[TY:![0-9]+]])
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "var"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateTypeParameter(name: "P", type: {{![0-9]+}})
+// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable(
+// CHECK-SAME: name: "varray"
+// CHECK-SAME: templateParams: {{![0-9]+}}
+// CHECK: !DITemplateValueParameter(name: "N", type: [[TY]], value: i32 1)
 
 // CHECK: ![[OUTER_FOO_INNER_ID:[0-9]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "inner"{{.*}}, identifier:
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
@@ -103,3 +116,15 @@
 // declaration/reference in the compile unit.
 // CHECK: !DISubprogram(name: "MyClass"
 // CHECK-SAME:  scope: [[C]]
+
+template 
+T var = T();
+template 
+P var = P();
+template 
+T varray[N];
+void f3() {
+  var = 1;
+  var = 1;
+  varray[0] = 1;
+}
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -244,6 +244,11 @@
   llvm::DINodeArray CollectFunctionTemplateParams(const FunctionDecl *FD,
   llvm::DIFile *Unit);
 
+  /// A helper function to collect debug info for function template
+  /// parameters.
+  llvm::DINodeArray CollectVarTemplateParams(const VarDecl *VD,
+ llvm::DIFile *Unit);
+
   /// A helper function to collect debug info for template
   /// parameters.
   llvm::DINodeArray
@@ -621,7 +626,9 @@
   /// Collect various properties of a VarDecl.
   void collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
unsigned , QualType , StringRef ,
-   StringRef , llvm::DIScope *);
+   StringRef ,
+   llvm::MDTuple *,
+   llvm::DIScope *);
 
   /// Allocate a copy of \p A using the DebugInfoNames allocator
   /// and return a reference to it. If multiple arguments are given the strings
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -41,6 +41,7 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
@@ -1679,6 +1680,30 @@
   return llvm::DINodeArray();
 }
 
+llvm::DINodeArray CGDebugInfo::CollectVarTemplateParams(const VarDecl *VL,
+llvm::DIFile *Unit) {
+  if (auto *TS = dyn_cast(VL)) {
+if (TS->getSpecializedTemplateOrPartial()
+.is()) {
+  const TemplateParameterList *TList =
+  TS->getSpecializedTemplateOrPartial()
+  .get()
+  ->getTemplateParameters();
+  return CollectTemplateParams(TList, TS->getTemplateArgs().asArray(),
+   Unit);
+}
+
+if (TS->getSpecializedTemplateOrPartial().is()) {
+  const TemplateParameterList *TList = TS->getSpecializedTemplateOrPartial()
+   .get()
+   ->getTemplateParameters();
+  return CollectTemplateParams(TList, TS->getTemplateArgs().asArray(),
+   Unit);
+}
+  }
+  return llvm::DINodeArray();
+}
+
 llvm::DINodeArray CGDebugInfo::CollectCXXTemplateParams(
 const ClassTemplateSpecializationDecl *TSpecial, llvm::DIFile *Unit) {
   // Always get the full list of parameters, not just the ones from
@@ -2965,6 +2990,7 @@
 void CGDebugInfo::collectVarDeclProps(const VarDecl *VD, llvm::DIFile *,
   unsigned , QualType ,
   StringRef , StringRef ,
+   

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris abandoned this revision.
ormris added a comment.

I'm abandoning this revision based on some internal feedback. Thanks for the 
review @JDevlieghere and @jingham!


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

No such luck...
Patch:

  Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  ===
  --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
(revision 329079)
  +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
(working copy)
  @@ -400,8 +400,9 @@
   break;
 case lldb::eLanguageTypeC_plus_plus:
 case lldb::eLanguageTypeC_plus_plus_11:
  +m_compiler->getLangOpts().CPlusPlus11 = true;
 case lldb::eLanguageTypeC_plus_plus_14:
  -m_compiler->getLangOpts().CPlusPlus11 = true;
  +m_compiler->getLangOpts().CPlusPlus14 = true;
   m_compiler->getHeaderSearchOpts().UseLibcxx = true;
   LLVM_FALLTHROUGH;
 case lldb::eLanguageTypeC_plus_plus_03:

Output:

  (lldb) e crazy
  error: use of undeclared identifier 'crazy'
  error: expected '(' for function-style cast or type construction
  error: expected expression


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

About to test with CPlusPlus14 enabled

Here's the output from those two commands.

  (lldb) frame var -g crazy
  error: no variable named 'crazy' found in this frame
  (lldb) frame var -g crazy
  error: no variable named 'crazy' found in this frame
  (lldb)


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

@jingham 
Doesn't look like it works.

  $ cat repro.cpp
  template  T crazy = T();
  
  int main(void) {
crazy = 5;
return crazy;
  }
  $ clang++ -g repro.cpp
  $ lldb a.out
  ...
  (lldb) frame variable
  (lldb) expr crazy
  error: use of undeclared identifier 'crazy'
  error: 1 errors parsing expression
  (lldb) expr crazy
  error: use of undeclared identifier 'crazy'
  error: expected '(' for function-style cast or type construction
  error: expected expression
  error: 3 errors parsing expression
  (lldb)


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

@jingham Will do.


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

//Ping//


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:2992
+  std::string NameString = Name.str();
+  llvm::raw_string_ostream ParameterizedName(NameString);
+  ParameterizedName << "<";

JDevlieghere wrote:
> Why not use Name.str() directly?
I got "error: invalid initialization of non-const reference ... from rvalue 
...". Using NameString fixed the issue.


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris added a project: debug-info.
ormris added a comment.

Adding debug-info project tag.


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

[Matthew Voss via Phabricator via cfe-commits]

ormris created this revision.
ormris added reviewers: dblaikie, aprantl, probinson, JDevlieghere.

This patch adds the associated template parameters to the DWARF name attribute
of all template variable specializations, mirroring how they are referenced in
the source code.


Repository:
  rC Clang

https://reviews.llvm.org/D44842

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-template.cpp


Index: test/CodeGenCXX/debug-info-template.cpp
===
--- test/CodeGenCXX/debug-info-template.cpp
+++ test/CodeGenCXX/debug-info-template.cpp
@@ -160,3 +160,14 @@
 };
 
 PaddingAtEndTemplate<> PaddedTemplateObj;
+
+// RUN: %clang -S -emit-llvm -target x86_64-unknown_unknown -g %s -o - 
-std=c++14 | FileCheck %s --check-prefix=CXX14
+// CXX14: !DIGlobalVariable(name: "vartemp"
+// CXX14: !DIGlobalVariable(name: "arraytemp"
+template  T vartemp = T();
+template  T arraytemp[N];
+
+void func() {
+  vartemp = 5;
+  arraytemp[0] = 1;
+}
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2982,8 +2982,27 @@
 
   Name = VD->getName();
   if (VD->getDeclContext() && !isa(VD->getDeclContext()) &&
-  !isa(VD->getDeclContext()))
+  !isa(VD->getDeclContext())) {
 LinkageName = CGM.getMangledName(VD);
+// If this node refers to an instantiation of a variable template, add the
+// template parameters to its name. This disambiguates it from other
+// instantiations.
+if (auto *VSD = dyn_cast(VD)) {
+  std::string NameString = Name.str();
+  llvm::raw_string_ostream ParameterizedName(NameString);
+  ParameterizedName << "<";
+  bool first = true;
+  for (auto Parameter : VSD->getTemplateArgs().asArray()) {
+if (!first)
+  ParameterizedName << ",";
+Parameter.print(getPrintingPolicy(), ParameterizedName);
+first = false;
+  }
+  ParameterizedName << ">";
+  Name = internString(ParameterizedName.str());
+}
+  }
+
   if (LinkageName == Name)
 LinkageName = StringRef();
 


Index: test/CodeGenCXX/debug-info-template.cpp
===
--- test/CodeGenCXX/debug-info-template.cpp
+++ test/CodeGenCXX/debug-info-template.cpp
@@ -160,3 +160,14 @@
 };
 
 PaddingAtEndTemplate<> PaddedTemplateObj;
+
+// RUN: %clang -S -emit-llvm -target x86_64-unknown_unknown -g %s -o - -std=c++14 | FileCheck %s --check-prefix=CXX14
+// CXX14: !DIGlobalVariable(name: "vartemp"
+// CXX14: !DIGlobalVariable(name: "arraytemp"
+template  T vartemp = T();
+template  T arraytemp[N];
+
+void func() {
+  vartemp = 5;
+  arraytemp[0] = 1;
+}
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2982,8 +2982,27 @@
 
   Name = VD->getName();
   if (VD->getDeclContext() && !isa(VD->getDeclContext()) &&
-  !isa(VD->getDeclContext()))
+  !isa(VD->getDeclContext())) {
 LinkageName = CGM.getMangledName(VD);
+// If this node refers to an instantiation of a variable template, add the
+// template parameters to its name. This disambiguates it from other
+// instantiations.
+if (auto *VSD = dyn_cast(VD)) {
+  std::string NameString = Name.str();
+  llvm::raw_string_ostream ParameterizedName(NameString);
+  ParameterizedName << "<";
+  bool first = true;
+  for (auto Parameter : VSD->getTemplateArgs().asArray()) {
+if (!first)
+  ParameterizedName << ",";
+Parameter.print(getPrintingPolicy(), ParameterizedName);
+first = false;
+  }
+  ParameterizedName << ">";
+  Name = internString(ParameterizedName.str());
+}
+  }
+
   if (LinkageName == Name)
 LinkageName = StringRef();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44218: Correct the alignment for the PS4 target

[Matthew Voss via Phabricator via cfe-commits]

ormris abandoned this revision.
ormris added a comment.

@echristo accepted this revision via cfe-commits. As it's now commited, I'm 
abandoning this revision.


Repository:
  rC Clang

https://reviews.llvm.org/D44218



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

[PATCH] D44218: Correct the alignment for the PS4 target

[Matthew Voss via Phabricator via cfe-commits]

ormris created this revision.
ormris added reviewers: rsmith, craig.topper, echristo, erichkeane.

See above.


Repository:
  rC Clang

https://reviews.llvm.org/D44218

Files:
  lib/Basic/Targets/OSTargets.h
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -8965,6 +8965,9 @@
 // PS4:#define __x86_64__ 1
 // PS4:#define unix 1
 //
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=x86_64-scei-ps4 < 
/dev/null | FileCheck -match-full-lines -check-prefix PS4-CXX %s
+// PS4-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 32UL
+//
 // RUN: %clang_cc1 -E -dM -triple=x86_64-pc-mingw32 < /dev/null | FileCheck 
-match-full-lines -check-prefix X86-64-DECLSPEC %s
 // RUN: %clang_cc1 -E -dM -fms-extensions -triple=x86_64-unknown-mingw32 < 
/dev/null | FileCheck -match-full-lines -check-prefix X86-64-DECLSPEC %s
 // X86-64-DECLSPEC: #define __declspec{{.*}}
Index: lib/Basic/Targets/OSTargets.h
===
--- lib/Basic/Targets/OSTargets.h
+++ lib/Basic/Targets/OSTargets.h
@@ -485,6 +485,7 @@
 default:
 case llvm::Triple::x86_64:
   this->MCountName = ".mcount";
+  this->NewAlign = 256;
   break;
 }
   }


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -8965,6 +8965,9 @@
 // PS4:#define __x86_64__ 1
 // PS4:#define unix 1
 //
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=x86_64-scei-ps4 < /dev/null | FileCheck -match-full-lines -check-prefix PS4-CXX %s
+// PS4-CXX:#define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 32UL
+//
 // RUN: %clang_cc1 -E -dM -triple=x86_64-pc-mingw32 < /dev/null | FileCheck -match-full-lines -check-prefix X86-64-DECLSPEC %s
 // RUN: %clang_cc1 -E -dM -fms-extensions -triple=x86_64-unknown-mingw32 < /dev/null | FileCheck -match-full-lines -check-prefix X86-64-DECLSPEC %s
 // X86-64-DECLSPEC: #define __declspec{{.*}}
Index: lib/Basic/Targets/OSTargets.h
===
--- lib/Basic/Targets/OSTargets.h
+++ lib/Basic/Targets/OSTargets.h
@@ -485,6 +485,7 @@
 default:
 case llvm::Triple::x86_64:
   this->MCountName = ".mcount";
+  this->NewAlign = 256;
   break;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36411: Restore previous structure ABI for bitfields with 'packed' attribute for PS4 targets

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Great. Thanks for the review! I don't have commit access currently.


https://reviews.llvm.org/D36411



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

[PATCH] D36411: Restore previous structure ABI for bitfields with 'packed' attribute for PS4 targets

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

An ABI change was introduced in r254596 that modified structure layouts when 
the 'packed' attribute was used on one-byte bitfields. Since the PS4 target 
needs to maintain backwards compatibility for all structure layouts, this 
change reintroduces the old behavior for PS4 targets only. It also introduces 
PS4 specific cases to the relevant test.

See the following review for discussion of r254596.
https://reviews.llvm.org/D14872


https://reviews.llvm.org/D36411



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