[PATCH] D144680: [Coroutines] Avoid creating conditional cleanup markers in suspend block

[Wei Wang 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 rGce7eb2e05544: [Coroutines] Avoid creating conditional 
cleanup markers in suspend block (authored by weiwang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144680

Files:
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCoroutines/pr59181.cpp

Index: clang/test/CodeGenCoroutines/pr59181.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/pr59181.cpp
@@ -0,0 +1,60 @@
+// Test for PR59181. Tests that no conditional cleanup is created around await_suspend.
+//
+// REQUIRES: x86-registered-target
+//
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - -std=c++20 -disable-llvm-passes -fsanitize-address-use-after-scope | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct Task {
+  int value_;
+  struct promise_type {
+Task get_return_object() {
+  return Task{0};
+}
+
+std::suspend_never initial_suspend() noexcept {
+  return {};
+}
+
+std::suspend_never final_suspend() noexcept {
+  return {};
+}
+
+void return_value(Task t) noexcept {}
+void unhandled_exception() noexcept {}
+
+auto await_transform(Task t) {
+  struct Suspension {
+auto await_ready() noexcept { return false;}
+auto await_suspend(std::coroutine_handle<> coro) {
+  coro.destroy();
+}
+
+auto await_resume() noexcept {
+  return 0;
+}
+  };
+  return Suspension{};
+}
+  };
+};
+
+Task bar(bool cond) {
+  co_return cond ? Task{ co_await Task{}}: Task{};
+}
+
+void foo() {
+  bar(true);
+}
+
+// CHECK: cleanup.cont:{{.*}}
+// CHECK-NEXT: load i8
+// CHECK-NEXT: trunc
+// CHECK-NEXT: store i1 false
+// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF:%ref.tmp[0-9]+]])
+
+// CHECK: await.suspend:{{.*}}
+// CHECK-NOT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF]])
+// CHECK: call void @_ZZN4Task12promise_type15await_transformES_EN10Suspension13await_suspendESt16coroutine_handleIvE
+// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[REF]])
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -333,6 +333,7 @@
   // in this header.
   struct CGCoroInfo {
 std::unique_ptr Data;
+bool InSuspendBlock = false;
 CGCoroInfo();
 ~CGCoroInfo();
   };
@@ -342,6 +343,10 @@
 return CurCoro.Data != nullptr;
   }
 
+  bool inSuspendBlock() const {
+return isCoroutine() && CurCoro.InSuspendBlock;
+  }
+
   /// CurGD - The GlobalDecl for the current function being compiled.
   GlobalDecl CurGD;
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -541,13 +541,17 @@
   // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end
   // marker. Instead, start the lifetime of a conditional temporary earlier
   // so that it's unconditional. Don't do this with sanitizers which need
-  // more precise lifetime marks.
+  // more precise lifetime marks. However when inside an "await.suspend"
+  // block, we should always avoid conditional cleanup because it creates
+  // boolean marker that lives across await_suspend, which can destroy coro
+  // frame.
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
-  !SanOpts.has(SanitizerKind::HWAddress) &&
-  !SanOpts.has(SanitizerKind::Memory) &&
-  !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
+  ((!SanOpts.has(SanitizerKind::HWAddress) &&
+!SanOpts.has(SanitizerKind::Memory) &&
+!CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) ||
+   inSuspendBlock())) {
 OldConditional = OutermostConditional;
 OutermostConditional = nullptr;
 
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -198,7 +198,9 @@
   auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
   auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
 
+  CGF.CurCoro.InSuspendBlock = true;
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
+  CGF.CurCoro.InSuspendBlock = false;
   if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
 // Veto suspension if requested by bool returning await_suspend.
 BasicBlock 

[PATCH] D144680: [Coroutines] Avoid creating conditional cleanup markers in suspend block

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 501205.
weiwang added a comment.

add target triple


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144680

Files:
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCoroutines/pr59181.cpp

Index: clang/test/CodeGenCoroutines/pr59181.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/pr59181.cpp
@@ -0,0 +1,60 @@
+// Test for PR59181. Tests that no conditional cleanup is created around await_suspend.
+//
+// REQUIRES: x86-registered-target
+//
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - -std=c++20 -disable-llvm-passes -fsanitize-address-use-after-scope | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct Task {
+  int value_;
+  struct promise_type {
+Task get_return_object() {
+  return Task{0};
+}
+
+std::suspend_never initial_suspend() noexcept {
+  return {};
+}
+
+std::suspend_never final_suspend() noexcept {
+  return {};
+}
+
+void return_value(Task t) noexcept {}
+void unhandled_exception() noexcept {}
+
+auto await_transform(Task t) {
+  struct Suspension {
+auto await_ready() noexcept { return false;}
+auto await_suspend(std::coroutine_handle<> coro) {
+  coro.destroy();
+}
+
+auto await_resume() noexcept {
+  return 0;
+}
+  };
+  return Suspension{};
+}
+  };
+};
+
+Task bar(bool cond) {
+  co_return cond ? Task{ co_await Task{}}: Task{};
+}
+
+void foo() {
+  bar(true);
+}
+
+// CHECK: cleanup.cont:{{.*}}
+// CHECK-NEXT: load i8
+// CHECK-NEXT: trunc
+// CHECK-NEXT: store i1 false
+// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF:%ref.tmp[0-9]+]])
+
+// CHECK: await.suspend:{{.*}}
+// CHECK-NOT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF]])
+// CHECK: call void @_ZZN4Task12promise_type15await_transformES_EN10Suspension13await_suspendESt16coroutine_handleIvE
+// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[REF]])
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -333,6 +333,7 @@
   // in this header.
   struct CGCoroInfo {
 std::unique_ptr Data;
+bool InSuspendBlock = false;
 CGCoroInfo();
 ~CGCoroInfo();
   };
@@ -342,6 +343,10 @@
 return CurCoro.Data != nullptr;
   }
 
+  bool inSuspendBlock() const {
+return isCoroutine() && CurCoro.InSuspendBlock;
+  }
+
   /// CurGD - The GlobalDecl for the current function being compiled.
   GlobalDecl CurGD;
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -541,13 +541,17 @@
   // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end
   // marker. Instead, start the lifetime of a conditional temporary earlier
   // so that it's unconditional. Don't do this with sanitizers which need
-  // more precise lifetime marks.
+  // more precise lifetime marks. However when inside an "await.suspend"
+  // block, we should always avoid conditional cleanup because it creates
+  // boolean marker that lives across await_suspend, which can destroy coro
+  // frame.
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
-  !SanOpts.has(SanitizerKind::HWAddress) &&
-  !SanOpts.has(SanitizerKind::Memory) &&
-  !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
+  ((!SanOpts.has(SanitizerKind::HWAddress) &&
+!SanOpts.has(SanitizerKind::Memory) &&
+!CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) ||
+   inSuspendBlock())) {
 OldConditional = OutermostConditional;
 OutermostConditional = nullptr;
 
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -198,7 +198,9 @@
   auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
   auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
 
+  CGF.CurCoro.InSuspendBlock = true;
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
+  CGF.CurCoro.InSuspendBlock = false;
   if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
 // Veto suspension if requested by bool returning await_suspend.
 BasicBlock *RealSuspendBlock =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D144680: [Coroutines] Avoid creating conditional cleanup markers in suspend block

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 500958.
weiwang edited the summary of this revision.
weiwang added a comment.

add test case and some comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144680

Files:
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCoroutines/pr59181.cpp

Index: clang/test/CodeGenCoroutines/pr59181.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/pr59181.cpp
@@ -0,0 +1,58 @@
+// Test for PR59181. Tests that no conditional cleanup is created around await_suspend.
+
+// RUN: %clang_cc1 -emit-llvm %s -o - -std=c++20 -disable-llvm-passes -fsanitize-address-use-after-scope | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct Task {
+  int value_;
+  struct promise_type {
+Task get_return_object() {
+  return Task{0};
+}
+
+std::suspend_never initial_suspend() noexcept {
+  return {};
+}
+
+std::suspend_never final_suspend() noexcept {
+  return {};
+}
+
+void return_value(Task t) noexcept {}
+void unhandled_exception() noexcept {}
+
+auto await_transform(Task t) {
+  struct Suspension {
+auto await_ready() noexcept { return false;}
+auto await_suspend(std::coroutine_handle<> coro) {
+  coro.destroy();
+}
+
+auto await_resume() noexcept {
+  return 0;
+}
+  };
+  return Suspension{};
+}
+  };
+};
+
+Task bar(bool cond) {
+  co_return cond ? Task{ co_await Task{}}: Task{};
+}
+
+void foo() {
+  bar(true);
+}
+
+// CHECK: cleanup.cont:{{.*}}
+// CHECK-NEXT: load i8
+// CHECK-NEXT: trunc
+// CHECK-NEXT: store i1 false
+// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF:%ref.tmp[0-9]+]])
+
+// CHECK: await.suspend:{{.*}}
+// CHECK-NOT: call void @llvm.lifetime.start.p0(i64 8, ptr [[REF]])
+// CHECK: call void @_ZZN4Task12promise_type15await_transformES_EN10Suspension13await_suspendESt16coroutine_handleIvE
+// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[REF]])
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -333,6 +333,7 @@
   // in this header.
   struct CGCoroInfo {
 std::unique_ptr Data;
+bool InSuspendBlock = false;
 CGCoroInfo();
 ~CGCoroInfo();
   };
@@ -342,6 +343,10 @@
 return CurCoro.Data != nullptr;
   }
 
+  bool inSuspendBlock() const {
+return isCoroutine() && CurCoro.InSuspendBlock;
+  }
+
   /// CurGD - The GlobalDecl for the current function being compiled.
   GlobalDecl CurGD;
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -541,13 +541,17 @@
   // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end
   // marker. Instead, start the lifetime of a conditional temporary earlier
   // so that it's unconditional. Don't do this with sanitizers which need
-  // more precise lifetime marks.
+  // more precise lifetime marks. However when inside an "await.suspend"
+  // block, we should always avoid conditional cleanup because it creates
+  // boolean marker that lives across await_suspend, which can destroy coro
+  // frame.
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
-  !SanOpts.has(SanitizerKind::HWAddress) &&
-  !SanOpts.has(SanitizerKind::Memory) &&
-  !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
+  ((!SanOpts.has(SanitizerKind::HWAddress) &&
+!SanOpts.has(SanitizerKind::Memory) &&
+!CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) ||
+   inSuspendBlock())) {
 OldConditional = OutermostConditional;
 OutermostConditional = nullptr;
 
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -198,7 +198,9 @@
   auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
   auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
 
+  CGF.CurCoro.InSuspendBlock = true;
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
+  CGF.CurCoro.InSuspendBlock = false;
   if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
 // Veto suspension if requested by bool returning await_suspend.
 BasicBlock *RealSuspendBlock =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144680: [Coroutines] Avoid creating conditional cleanup markers in suspend block

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D144680#4149149 , @ChuanqiXu wrote:

> BTW, what is the conclusion for the concern about sanitizers? Would change 
> make sanitizers to perform false positive diagnostics?

This change is limited to the `await.suspend` block, which only does codegen 
for `awaiter.await_suspend(coroutine_handle::from_promise(p))`. In this 
case, I think the only asan check removed by the change is the conditional 
marker for cleanup the temporary `coroutine_handler` used as the parameter of 
`await_suspend`, so my understanding is that it shouldn't affect asan 
diagnostic.

To show the difference it makes to the source from issue #59181

Before:

  %ref.tmp25 = alloca %"struct.std::__1::coroutine_handle.6", align 8
  ...
  // asan check inserted here
  bool cond_cleanup_marker = false;
  
  if (cond) {
  // get awaiter
  ...
  
  if (!awaiter.await_ready()) {
  call void @llvm.lifetime.start.p0(i64 8, ptr %ref.tmp25)
  
  // asan check inserted here
cond_cleanup_marker = true;

  // store handler to %ref.tmp25
...

  awaiter.await_suspend();
  
  // asan check inserted here
if (cond_cleanup_marker)
call void @llvm.lifetime.end.p0(i64 8, ptr %ref.tmp25)
  call i8 @llvm.coro.suspend(...)
...
  }
  ...
  } else {
  ... 
  }
  ...
  lpad:
  ...
  // asan check inserted here
  if (cond_cleanup_marker)
  call void @llvm.lifetime.end.p0(i64 8, ptr %ref.tmp25)

The issue is only reproduced in `-O0`, because ``cond_cleanup_marker` is 
optimized away in `-O1` or up opt level.

After:

  %ref.tmp25 = alloca %"struct.std::__1::coroutine_handle.6", align 8
  ...
  
  call void @llvm.lifetime.start.p0(i64 8, ptr %ref.tmp25)
  
  if (cond) {
  ...
  if (!awaiter.await_ready()) {
  // store handler to %ref.tmp25
...
  
awaiter.await_suspend();
call void @llvm.lifetime.end.p0(i64 8, ptr %ref.tmp25)
call i8 @llvm.coro.suspend(...)
...
  }
  ...
  } else {
  ... 
  }
  ...
  lpad:
  call void @llvm.lifetime.end.p0(i64 8, ptr %ref.tmp25)

This also matches with the IR without asan.




Comment at: clang/lib/CodeGen/CGExpr.cpp:544
   // so that it's unconditional. Don't do this with sanitizers which need
   // more precise lifetime marks.
   ConditionalEvaluation *OldConditional = nullptr;

ChuanqiXu wrote:
> We should add comment to explain why we add a forward path for coroutines.
Will do



Comment at: clang/lib/CodeGen/CodeGenFunction.h:336
 std::unique_ptr Data;
+bool InSuspendBlock = false;
 CGCoroInfo();

bruno wrote:
> Should this live inside CGCoroData instead?
`CGCoroData` is forward declared here, so it does not know what's inside.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144680

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

[PATCH] D144680: [Coroutines] Avoid creating conditional cleanup markers in suspend block

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: ChuanqiXu, hoy, wenlei.
Herald added a project: All.
weiwang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144680

Files:
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h


Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -333,6 +333,7 @@
   // in this header.
   struct CGCoroInfo {
 std::unique_ptr Data;
+bool InSuspendBlock = false;
 CGCoroInfo();
 ~CGCoroInfo();
   };
@@ -342,6 +343,10 @@
 return CurCoro.Data != nullptr;
   }
 
+  bool inSuspendBlock() const {
+return isCoroutine() && CurCoro.InSuspendBlock;
+  }
+
   /// CurGD - The GlobalDecl for the current function being compiled.
   GlobalDecl CurGD;
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -545,9 +545,10 @@
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
-  !SanOpts.has(SanitizerKind::HWAddress) &&
-  !SanOpts.has(SanitizerKind::Memory) &&
-  !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
+  ((!SanOpts.has(SanitizerKind::HWAddress) &&
+!SanOpts.has(SanitizerKind::Memory) &&
+!CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) ||
+   inSuspendBlock())) {
 OldConditional = OutermostConditional;
 OutermostConditional = nullptr;
 
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -198,7 +198,9 @@
   auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
   auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
 
+  CGF.CurCoro.InSuspendBlock = true;
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
+  CGF.CurCoro.InSuspendBlock = false;
   if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
 // Veto suspension if requested by bool returning await_suspend.
 BasicBlock *RealSuspendBlock =


Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -333,6 +333,7 @@
   // in this header.
   struct CGCoroInfo {
 std::unique_ptr Data;
+bool InSuspendBlock = false;
 CGCoroInfo();
 ~CGCoroInfo();
   };
@@ -342,6 +343,10 @@
 return CurCoro.Data != nullptr;
   }
 
+  bool inSuspendBlock() const {
+return isCoroutine() && CurCoro.InSuspendBlock;
+  }
+
   /// CurGD - The GlobalDecl for the current function being compiled.
   GlobalDecl CurGD;
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -545,9 +545,10 @@
   ConditionalEvaluation *OldConditional = nullptr;
   CGBuilderTy::InsertPoint OldIP;
   if (isInConditionalBranch() && !E->getType().isDestructedType() &&
-  !SanOpts.has(SanitizerKind::HWAddress) &&
-  !SanOpts.has(SanitizerKind::Memory) &&
-  !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) {
+  ((!SanOpts.has(SanitizerKind::HWAddress) &&
+!SanOpts.has(SanitizerKind::Memory) &&
+!CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) ||
+   inSuspendBlock())) {
 OldConditional = OutermostConditional;
 OutermostConditional = nullptr;
 
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -198,7 +198,9 @@
   auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
   auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
 
+  CGF.CurCoro.InSuspendBlock = true;
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
+  CGF.CurCoro.InSuspendBlock = false;
   if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
 // Veto suspension if requested by bool returning await_suspend.
 BasicBlock *RealSuspendBlock =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122759: [time-report] Add timers to codegen actions

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 419261.
weiwang added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122759

Files:
  clang/include/clang/Frontend/FrontendAction.h
  clang/include/clang/Parse/ParseAST.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/lib/Parse/ParseAST.cpp

Index: clang/lib/Parse/ParseAST.cpp
===
--- clang/lib/Parse/ParseAST.cpp
+++ clang/lib/Parse/ParseAST.cpp
@@ -23,6 +23,7 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/Timer.h"
 #include 
 #include 
 
@@ -111,7 +112,8 @@
   ParseAST(*S.get(), PrintStats, SkipFunctionBodies);
 }
 
-void clang::ParseAST(Sema , bool PrintStats, bool SkipFunctionBodies) {
+void clang::ParseAST(Sema , bool PrintStats, bool SkipFunctionBodies,
+ bool TimePassesIsEnabled) {
   // Collect global stats on Decls/Stmts (until we have a module streamer).
   if (PrintStats) {
 Decl::EnableStatistics();
@@ -152,6 +154,8 @@
 
   if (HaveLexer) {
 llvm::TimeTraceScope TimeScope("Frontend");
+llvm::NamedRegionTimer T("Parse AST", "Parse AST", "Clang Compilation",
+ "Clang Compilation", TimePassesIsEnabled);
 P.Initialize();
 Parser::DeclGroupPtrTy ADecl;
 Sema::ModuleImportState ImportState;
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -186,6 +186,9 @@
 Act = std::make_unique(std::move(Act),
 FEOpts.ASTMergeFiles);
 
+  if (CI.getCodeGenOpts().TimePasses)
+Act->enableTimePasses();
+
   return Act;
 }
 
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -1137,7 +1137,7 @@
 CI.createSema(getTranslationUnitKind(), CompletionConsumer);
 
   ParseAST(CI.getSema(), CI.getFrontendOpts().ShowStats,
-   CI.getFrontendOpts().SkipFunctionBodies);
+   CI.getFrontendOpts().SkipFunctionBodies, isTimePassesEnabled());
 }
 
 void PluginASTAction::anchor() { }
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -294,6 +294,9 @@
 void HandleTranslationUnit(ASTContext ) override {
   {
 llvm::TimeTraceScope TimeScope("Frontend");
+llvm::NamedRegionTimer T("IR Generation", "IR Generation",
+ "Clang Compilation", "Clang Compilation",
+ CodeGenOpts.TimePasses);
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (TimerIsEnabled) {
   LLVMIRGenerationRefCount += 1;
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1500,6 +1500,8 @@
   {
 PrettyStackTraceString CrashInfo("Optimizer");
 llvm::TimeTraceScope TimeScope("Optimizer");
+llvm::NamedRegionTimer T("Optimizer", "Optimizer", "Clang Compilation",
+ "Clang Compilation", CodeGenOpts.TimePasses);
 MPM.run(*TheModule, MAM);
   }
 }
@@ -1536,6 +1538,9 @@
   {
 PrettyStackTraceString CrashInfo("Code generation");
 llvm::TimeTraceScope TimeScope("CodeGenPasses");
+llvm::NamedRegionTimer T("CodeGenPasses", "CodeGenPasses",
+ "Clang Compilation", "Clang Compilation",
+ CodeGenOpts.TimePasses);
 CodeGenPasses.run(*TheModule);
   }
 }
Index: clang/include/clang/Parse/ParseAST.h
===
--- clang/include/clang/Parse/ParseAST.h
+++ clang/include/clang/Parse/ParseAST.h
@@ -44,7 +44,8 @@
   /// Parse the main file known to the preprocessor, producing an
   /// abstract syntax tree.
   void ParseAST(Sema , bool PrintStats = false,
-bool SkipFunctionBodies = false);
+bool SkipFunctionBodies = false,
+bool TimePassesIsEnabled = false);
 
 }  // end namespace clang
 
Index: clang/include/clang/Frontend/FrontendAction.h
===
--- clang/include/clang/Frontend/FrontendAction.h
+++ clang/include/clang/Frontend/FrontendAction.h
@@ -37,6 +37,7 @@
   FrontendInputFile CurrentInput;
  

[PATCH] D122759: [time-report] Add timers to codegen actions

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: ormris, hoy, wenlei.
Herald added a project: All.
weiwang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122759

Files:
  clang/include/clang/Frontend/FrontendAction.h
  clang/include/clang/Parse/ParseAST.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/lib/Parse/ParseAST.cpp

Index: clang/lib/Parse/ParseAST.cpp
===
--- clang/lib/Parse/ParseAST.cpp
+++ clang/lib/Parse/ParseAST.cpp
@@ -23,6 +23,7 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/Timer.h"
 #include 
 #include 
 
@@ -111,7 +112,8 @@
   ParseAST(*S.get(), PrintStats, SkipFunctionBodies);
 }
 
-void clang::ParseAST(Sema , bool PrintStats, bool SkipFunctionBodies) {
+void clang::ParseAST(Sema , bool PrintStats, bool SkipFunctionBodies,
+ bool TimePassesIsEnabled) {
   // Collect global stats on Decls/Stmts (until we have a module streamer).
   if (PrintStats) {
 Decl::EnableStatistics();
@@ -152,6 +154,8 @@
 
   if (HaveLexer) {
 llvm::TimeTraceScope TimeScope("Frontend");
+llvm::NamedRegionTimer T("Parse AST", "Parse AST", "Clang Compilation",
+ "Clang Commpilation", TimePassesIsEnabled);
 P.Initialize();
 Parser::DeclGroupPtrTy ADecl;
 Sema::ModuleImportState ImportState;
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -186,6 +186,9 @@
 Act = std::make_unique(std::move(Act),
 FEOpts.ASTMergeFiles);
 
+  if (CI.getCodeGenOpts().TimePasses)
+Act->enableTimePasses();
+
   return Act;
 }
 
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -1137,7 +1137,7 @@
 CI.createSema(getTranslationUnitKind(), CompletionConsumer);
 
   ParseAST(CI.getSema(), CI.getFrontendOpts().ShowStats,
-   CI.getFrontendOpts().SkipFunctionBodies);
+   CI.getFrontendOpts().SkipFunctionBodies, isTimePassesEnabled());
 }
 
 void PluginASTAction::anchor() { }
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -294,6 +294,9 @@
 void HandleTranslationUnit(ASTContext ) override {
   {
 llvm::TimeTraceScope TimeScope("Frontend");
+llvm::NamedRegionTimer T("IR Generation", "IR Generation",
+ "Clang Compilation", "Clang Compilation",
+ CodeGenOpts.TimePasses);
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (TimerIsEnabled) {
   LLVMIRGenerationRefCount += 1;
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1500,6 +1500,8 @@
   {
 PrettyStackTraceString CrashInfo("Optimizer");
 llvm::TimeTraceScope TimeScope("Optimizer");
+llvm::NamedRegionTimer T("Optimizer", "Optimizer", "Clang Compilation",
+ "Clang Compilation", CodeGenOpts.TimePasses);
 MPM.run(*TheModule, MAM);
   }
 }
@@ -1536,6 +1538,9 @@
   {
 PrettyStackTraceString CrashInfo("Code generation");
 llvm::TimeTraceScope TimeScope("CodeGenPasses");
+llvm::NamedRegionTimer T("CodeGenPasses", "CodeGenPasses",
+ "Clang Compilation", "Clang Compilation",
+ CodeGenOpts.TimePasses);
 CodeGenPasses.run(*TheModule);
   }
 }
Index: clang/include/clang/Parse/ParseAST.h
===
--- clang/include/clang/Parse/ParseAST.h
+++ clang/include/clang/Parse/ParseAST.h
@@ -44,7 +44,8 @@
   /// Parse the main file known to the preprocessor, producing an
   /// abstract syntax tree.
   void ParseAST(Sema , bool PrintStats = false,
-bool SkipFunctionBodies = false);
+bool SkipFunctionBodies = false,
+bool TimePassesIsEnabled = false);
 
 }  // end namespace clang
 
Index: clang/include/clang/Frontend/FrontendAction.h
===
--- clang/include/clang/Frontend/FrontendAction.h
+++ 

[PATCH] D117605: [time-trace] Add optimizer and codegen regions to NPM

[Wei Wang 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 rG55d887b83364: [time-trace] Add optimizer and codegen regions 
to NPM (authored by weiwang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117605

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1492,8 +1492,11 @@
   }
 
   // Now that we have all of the passes ready, run them.
-  PrettyStackTraceString CrashInfo("Optimizer");
-  MPM.run(*TheModule, MAM);
+  {
+PrettyStackTraceString CrashInfo("Optimizer");
+llvm::TimeTraceScope TimeScope("Optimizer");
+MPM.run(*TheModule, MAM);
+  }
 }
 
 void EmitAssemblyHelper::RunCodegenPipeline(
@@ -1525,8 +1528,11 @@
 return;
   }
 
-  PrettyStackTraceString CrashInfo("Code generation");
-  CodeGenPasses.run(*TheModule);
+  {
+PrettyStackTraceString CrashInfo("Code generation");
+llvm::TimeTraceScope TimeScope("CodeGenPasses");
+CodeGenPasses.run(*TheModule);
+  }
 }
 
 /// A clean version of `EmitAssembly` that uses the new pass manager.


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1492,8 +1492,11 @@
   }
 
   // Now that we have all of the passes ready, run them.
-  PrettyStackTraceString CrashInfo("Optimizer");
-  MPM.run(*TheModule, MAM);
+  {
+PrettyStackTraceString CrashInfo("Optimizer");
+llvm::TimeTraceScope TimeScope("Optimizer");
+MPM.run(*TheModule, MAM);
+  }
 }
 
 void EmitAssemblyHelper::RunCodegenPipeline(
@@ -1525,8 +1528,11 @@
 return;
   }
 
-  PrettyStackTraceString CrashInfo("Code generation");
-  CodeGenPasses.run(*TheModule);
+  {
+PrettyStackTraceString CrashInfo("Code generation");
+llvm::TimeTraceScope TimeScope("CodeGenPasses");
+CodeGenPasses.run(*TheModule);
+  }
 }
 
 /// A clean version of `EmitAssembly` that uses the new pass manager.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117605: [time-trace] Add optimizer and codegen regions to NPM

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: ormris, hoy, wenlei.
weiwang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Optimizer and codegen regions were only added to legacy PM. Add them to NPM as 
well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117605

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1491,8 +1491,11 @@
   }
 
   // Now that we have all of the passes ready, run them.
-  PrettyStackTraceString CrashInfo("Optimizer");
-  MPM.run(*TheModule, MAM);
+  {
+PrettyStackTraceString CrashInfo("Optimizer");
+llvm::TimeTraceScope TimeScope("Optimizer");
+MPM.run(*TheModule, MAM);
+  }
 }
 
 void EmitAssemblyHelper::RunCodegenPipeline(
@@ -1524,8 +1527,11 @@
 return;
   }
 
-  PrettyStackTraceString CrashInfo("Code generation");
-  CodeGenPasses.run(*TheModule);
+  {
+PrettyStackTraceString CrashInfo("Code generation");
+llvm::TimeTraceScope TimeScope("CodeGenPasses");
+CodeGenPasses.run(*TheModule);
+  }
 }
 
 /// A clean version of `EmitAssembly` that uses the new pass manager.


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1491,8 +1491,11 @@
   }
 
   // Now that we have all of the passes ready, run them.
-  PrettyStackTraceString CrashInfo("Optimizer");
-  MPM.run(*TheModule, MAM);
+  {
+PrettyStackTraceString CrashInfo("Optimizer");
+llvm::TimeTraceScope TimeScope("Optimizer");
+MPM.run(*TheModule, MAM);
+  }
 }
 
 void EmitAssemblyHelper::RunCodegenPipeline(
@@ -1524,8 +1527,11 @@
 return;
   }
 
-  PrettyStackTraceString CrashInfo("Code generation");
-  CodeGenPasses.run(*TheModule);
+  {
+PrettyStackTraceString CrashInfo("Code generation");
+llvm::TimeTraceScope TimeScope("CodeGenPasses");
+CodeGenPasses.run(*TheModule);
+  }
 }
 
 /// A clean version of `EmitAssembly` that uses the new pass manager.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Wei Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa075d6722283: [Sema] fix nondeterminism in 
ASTContext::getDeducedTemplateSpecializationType (authored by weiwang).

Changed prior to commit:
  https://reviews.llvm.org/D112481?vs=382375=388616#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112481

Files:
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5676,6 +5676,9 @@
 
   auto *DTST = new (*this, TypeAlignment)
   DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+  llvm::FoldingSetNodeID TempID;
+  DTST->Profile(TempID);
+  assert(ID == TempID && "ID does not match");
   Types.push_back(DTST);
   DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);
   return QualType(DTST, 0);
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -5073,8 +5073,10 @@
   static void Profile(llvm::FoldingSetNodeID , TemplateName Template,
   QualType Deduced, bool IsDependent) {
 Template.Profile(ID);
-ID.AddPointer(Deduced.getAsOpaquePtr());
-ID.AddBoolean(IsDependent);
+QualType CanonicalType =
+Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
+ID.AddPointer(CanonicalType.getAsOpaquePtr());
+ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
   static bool classof(const Type *T) {


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5676,6 +5676,9 @@
 
   auto *DTST = new (*this, TypeAlignment)
   DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+  llvm::FoldingSetNodeID TempID;
+  DTST->Profile(TempID);
+  assert(ID == TempID && "ID does not match");
   Types.push_back(DTST);
   DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);
   return QualType(DTST, 0);
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -5073,8 +5073,10 @@
   static void Profile(llvm::FoldingSetNodeID , TemplateName Template,
   QualType Deduced, bool IsDependent) {
 Template.Profile(ID);
-ID.AddPointer(Deduced.getAsOpaquePtr());
-ID.AddBoolean(IsDependent);
+QualType CanonicalType =
+Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
+ID.AddPointer(CanonicalType.getAsOpaquePtr());
+ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
   static bool classof(const Type *T) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

This issue has been blocking our internal module re-enablement for some time 
now, and we really appreciate any feedback. We also wonder if only 
`DeducedTemplateSpecializationType` is affected or it could also happen to 
other types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112481

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

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Wei Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb283d55c90dd: [openmp] Emit deferred diag only when device 
compilation presents (authored by weiwang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109175

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/SemaCUDA/openmp-target.cu


Index: clang/test/SemaCUDA/openmp-target.cu
===
--- clang/test/SemaCUDA/openmp-target.cu
+++ clang/test/SemaCUDA/openmp-target.cu
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64 -verify=expected,dev \
 // RUN:-verify-ignore-unexpected=note \
-// RUN:-fopenmp -fopenmp-version=50 -o - %s
+// RUN:-fopenmp -fopenmp-version=50 
-fopenmp-targets=amdgcn-amd-amdhsa -o - %s
 // RUN: %clang_cc1 -triple x86_64 -verify -verify-ignore-unexpected=note\
-// RUN:-fopenmp -fopenmp-version=50 -o - -x c++ %s
+// RUN:-fopenmp -fopenmp-version=50 
-fopenmp-targets=amdgcn-amd-amdhsa -o - -x c++ %s
 // RUN: %clang_cc1 -triple x86_64 -verify=dev -verify-ignore-unexpected=note\
 // RUN:-fcuda-is-device -o - %s
 
Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -1,11 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45 
-fopenmp -fopenmp-version=45 -fnoopenmp-use-tls -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - 
%s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp -fopenmp-targets=x86_64-apple-macos10.7.0 
-fnoopenmp-use-tls -ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5,dev5 
-fopenmp -fopenmp-is-device -fopenmp-targets=x86_64-apple-macos10.7.0 
-aux-triple x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 -o - 
%s
 
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device -fnoopenmp-use-tls 
-ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd 
-fopenmp-targets=x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device 
-fopenmp-targets=x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45 
-fopenmp-version=45 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5 
-fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 #pragma omp end declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp end declare target'}}
 
 int a, b, z; // omp5-error {{variable captured in declare target region must 
appear in a to clause}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12609,7 +12609,9 @@
 VDecl->setInitStyle(VarDecl::ListInit);
   }
 
-  if (LangOpts.OpenMP && VDecl->isFileVarDecl())
+  if (LangOpts.OpenMP &&
+  (LangOpts.OpenMPIsDevice || !LangOpts.OMPTargetTriples.empty()) &&
+  VDecl->isFileVarDecl())
 DeclsToCheckForDeferredDiags.insert(VDecl);
   CheckCompleteVariableDeclaration(VDecl);
 }
@@ -14839,7 +14841,9 @@
 DiscardCleanupsInEvaluationContext();
   }
 
-  if (FD && (LangOpts.OpenMP || LangOpts.CUDA || LangOpts.SYCLIsDevice)) {
+  if (FD && ((LangOpts.OpenMP && (LangOpts.OpenMPIsDevice ||
+  !LangOpts.OMPTargetTriples.empty())) ||
+ LangOpts.CUDA || LangOpts.SYCLIsDevice)) {
 auto ES = getEmissionStatus(FD);
 if (ES == Sema::FunctionEmissionStatus::Emitted ||
 ES == Sema::FunctionEmissionStatus::Unknown)


Index: clang/test/SemaCUDA/openmp-target.cu
===
--- clang/test/SemaCUDA/openmp-target.cu
+++ clang/test/SemaCUDA/openmp-target.cu
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64 -verify=expected,dev \
 // RUN:-verify-ignore-unexpected=note \
-// RUN:-fopenmp -fopenmp-version=50 -o - %s
+// RUN:-fopenmp -fopenmp-version=50 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s
 // RUN: %clang_cc1 -triple x86_64 -verify 

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D109175#2989997 , @yaxunl wrote:

> In D109175#2989823 , @weiwang wrote:
>
>> In D109175#2987048 , @jdoerfert 
>> wrote:
>>
>>> In D109175#2986782 , @yaxunl 
>>> wrote:
>>>
 I agree with Johannes and Alexey that deferred diags are only needed when 
 LangOpts.OMPTargetTriples.empty(). However, I am not sure whether it is 
 only needed in device compilation.

 For other offloading languages like CUDA/HIP it is needed in both device 
 and host compilation.
>>>
>>> Technically, we might even want to delay in host only mode for OpenMP, but 
>>> that is something we can revisit (e.g., by dynamically setting a flag based 
>>> on the directives we've seen).
>>> @yaxunl Should we for now check if there is any associated offload job?
>>
>> Shall we go ahead and get this change in and think about more longer term 
>> solution later?
>
> LGTM. This patch should be sufficient to limit deferred diags to OpenMP with 
> offloading. Device compilation is covered by OpenMPIsDevice and host 
> compilation is covered by !LangOpts.OMPTargetTriples.empty(). I will leave 
> the decision to Johannes.

Thanks. @jdoerfert. Could you approve this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109175

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

[PATCH] D109175: [openmp] Emit deferred diag only when device compilation presents

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D109175#2987048 , @jdoerfert wrote:

> In D109175#2986782 , @yaxunl wrote:
>
>> I agree with Johannes and Alexey that deferred diags are only needed when 
>> LangOpts.OMPTargetTriples.empty(). However, I am not sure whether it is only 
>> needed in device compilation.
>>
>> For other offloading languages like CUDA/HIP it is needed in both device and 
>> host compilation.
>
> Technically, we might even want to delay in host only mode for OpenMP, but 
> that is something we can revisit (e.g., by dynamically setting a flag based 
> on the directives we've seen).
> @yaxunl Should we for now check if there is any associated offload job?

Shall we go ahead and get this change in and think about more longer term 
solution later?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109175

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

[PATCH] D109175: [openmp] Add clang cc1 option -fopenmp-skip-deferred-diags

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 370630.
weiwang added a comment.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109175

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/SemaCUDA/openmp-target.cu


Index: clang/test/SemaCUDA/openmp-target.cu
===
--- clang/test/SemaCUDA/openmp-target.cu
+++ clang/test/SemaCUDA/openmp-target.cu
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64 -verify=expected,dev \
 // RUN:-verify-ignore-unexpected=note \
-// RUN:-fopenmp -fopenmp-version=50 -o - %s
+// RUN:-fopenmp -fopenmp-version=50 
-fopenmp-targets=amdgcn-amd-amdhsa -o - %s
 // RUN: %clang_cc1 -triple x86_64 -verify -verify-ignore-unexpected=note\
-// RUN:-fopenmp -fopenmp-version=50 -o - -x c++ %s
+// RUN:-fopenmp -fopenmp-version=50 
-fopenmp-targets=amdgcn-amd-amdhsa -o - -x c++ %s
 // RUN: %clang_cc1 -triple x86_64 -verify=dev -verify-ignore-unexpected=note\
 // RUN:-fcuda-is-device -o - %s
 
Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -1,11 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45 
-fopenmp -fopenmp-version=45 -fnoopenmp-use-tls -ferror-limit 100 -o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - 
%s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp -fopenmp-targets=x86_64-apple-macos10.7.0 
-fnoopenmp-use-tls -ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5,dev5 
-fopenmp -fopenmp-is-device -fopenmp-targets=x86_64-apple-macos10.7.0 
-aux-triple x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 -o - 
%s
 
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
-// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device -fnoopenmp-use-tls 
-ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd 
-fopenmp-targets=x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device 
-fopenmp-targets=x86_64-apple-macos10.7.0 -fnoopenmp-use-tls -ferror-limit 100 
-o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45 
-fopenmp-version=45 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5 
-fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 #pragma omp end declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp end declare target'}}
 
 int a, b, z; // omp5-error {{variable captured in declare target region must 
appear in a to clause}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12609,7 +12609,9 @@
 VDecl->setInitStyle(VarDecl::ListInit);
   }
 
-  if (LangOpts.OpenMP && VDecl->isFileVarDecl())
+  if (LangOpts.OpenMP &&
+  (LangOpts.OpenMPIsDevice || !LangOpts.OMPTargetTriples.empty()) &&
+  VDecl->isFileVarDecl())
 DeclsToCheckForDeferredDiags.insert(VDecl);
   CheckCompleteVariableDeclaration(VDecl);
 }
@@ -14839,7 +14841,9 @@
 DiscardCleanupsInEvaluationContext();
   }
 
-  if (FD && (LangOpts.OpenMP || LangOpts.CUDA || LangOpts.SYCLIsDevice)) {
+  if (FD && ((LangOpts.OpenMP && (LangOpts.OpenMPIsDevice ||
+  !LangOpts.OMPTargetTriples.empty())) ||
+ LangOpts.CUDA || LangOpts.SYCLIsDevice)) {
 auto ES = getEmissionStatus(FD);
 if (ES == Sema::FunctionEmissionStatus::Emitted ||
 ES == Sema::FunctionEmissionStatus::Unknown)


Index: clang/test/SemaCUDA/openmp-target.cu
===
--- clang/test/SemaCUDA/openmp-target.cu
+++ clang/test/SemaCUDA/openmp-target.cu
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64 -verify=expected,dev \
 // RUN:-verify-ignore-unexpected=note \
-// RUN:-fopenmp -fopenmp-version=50 -o - %s
+// RUN:-fopenmp -fopenmp-version=50 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s
 // RUN: %clang_cc1 -triple x86_64 -verify -verify-ignore-unexpected=note\
-// RUN:-fopenmp -fopenmp-version=50 -o - -x c++ %s
+// RUN:-fopenmp -fopenmp-version=50 

[PATCH] D109175: [openmp] Add clang cc1 option -fopenmp-skip-deferred-diags

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D109175#2980900 , @jdoerfert wrote:

> In D109175#2980806 , @weiwang wrote:
>
>> In D109175#2980744 , @jdoerfert 
>> wrote:
>>
>>> Why do we need this flag, is the absence of -fopenmp-targets not sufficient?
>>
>> Just double checked, this is the full omp related options currently in use:
>>
>>   "-fopenmp"
>>   "-fopenmp-version=31"
>>   "-fopenmp-version=31"
>>   "-fopenmp-cuda-parallel-target-regions"
>>
>> We saw a huge number of `DECLS_TO_CHECK_FOR_DEFERRED_DIAGS` records. I don't 
>> know if this has anything to do with omp version being 31, since prior 5.0, 
>> everything is available on host.
>
> I don't think we are selective right now. As I was saying, disable deferred 
> parsing if fopenmp-targets is missing, no need for this option.

Sure I can certainly make the change. To make sure I understand you correctly: 
if -fopenmp-targets (or maybe fopenmp-is-device too) is not given from cmdline, 
we can just skip the deferred diags like this option does?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109175

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

[PATCH] D109175: [openmp] Add clang cc1 option -fopenmp-skip-deferred-diags

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D109175#2980744 , @jdoerfert wrote:

> Why do we need this flag, is the absence of -fopenmp-targets not sufficient?

Just double checked, this is the full omp related options currently in use:

  "-fopenmp"
  "-fopenmp-version=31"
  "-fopenmp-version=31"
  "-fopenmp-cuda-parallel-target-regions"

We saw a huge number of `DECLS_TO_CHECK_FOR_DEFERRED_DIAGS` records. I don't 
know if this has anything to do with omp version being 31, since prior 5.0, 
everything is available on host and `getEmissionStatus` would return 
`FunctionEmissionStatus::Unknown` in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109175

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

[PATCH] D109175: [openmp] Add clang cc1 option -fopenmp-skip-deferred-diags

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D109175#2980446 , @lebedev.ri 
wrote:

> In D109175#2980333 , @weiwang wrote:
>
>> Our internal codebase never uses the target directive. Once the deferred 
>> diags is bypassed, we observed 18% e2e build time improvement.
>
> Is that with `-fopenmp` or without?
> That seems, kinda a lot more than i would have expected,
> perhaps there are some other ways to reduce the overhead other than this 
> approach?

This is with -fopenmp and no other omp related flags. I'd prefer a more generic 
way of fixing this, but right now this seems to be most direct way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109175

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

[PATCH] D109175: [openmp] Add clang cc1 option -fopenmp-skip-deferred-diags

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

Our internal codebase never uses the target directive. Once the deferred diags 
is bypassed, we observed 18% e2e build time improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109175

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

[PATCH] D109175: [openmp] Add clang cc1 option -fopenmp-skip-deferred-diags

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: hoy, dexonsmith, wenlei, dang, guansong, yaxunl.
weiwang requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

There are cases where no target compilation/offloading would ever happen, but
we are still paying the price for handling deferred diags. The new cc1 option
allows user to explictly skip deferred diags handling in FE.

This results in considerable build time improvement from our internal
workloads.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109175

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/OpenMP/declare_target_messages.cpp


Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device -fnoopenmp-use-tls 
-ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp45 
-fopenmp-version=45 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5,dev5 
-fopenmp -fopenmp-is-device -fopenmp-targets=x86_64-apple-macos10.7.0 
-aux-triple x86_64-apple-macos10.7.0 -fnoopenmp-use-tls 
-fopenmp-skip-deferred-diags -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 
-verify=expected,omp5,host5 -fopenmp-simd -fopenmp-is-device -fnoopenmp-use-tls 
-fopenmp-skip-deferred-diags -ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5 
-fopenmp -fnoopenmp-use-tls -fopenmp-skip-deferred-diags -ferror-limit 100 -o - 
%s
 #pragma omp end declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp end declare target'}}
 
 int a, b, z; // omp5-error {{variable captured in declare target region must 
appear in a to clause}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12609,7 +12609,8 @@
 VDecl->setInitStyle(VarDecl::ListInit);
   }
 
-  if (LangOpts.OpenMP && VDecl->isFileVarDecl())
+  if (LangOpts.OpenMP && !LangOpts.OpenMPSkipDeferredDiags &&
+  VDecl->isFileVarDecl())
 DeclsToCheckForDeferredDiags.insert(VDecl);
   CheckCompleteVariableDeclaration(VDecl);
 }
@@ -14839,7 +14840,8 @@
 DiscardCleanupsInEvaluationContext();
   }
 
-  if (FD && (LangOpts.OpenMP || LangOpts.CUDA || LangOpts.SYCLIsDevice)) {
+  if (FD && ((LangOpts.OpenMP && !LangOpts.OpenMPSkipDeferredDiags) ||
+ LangOpts.CUDA || LangOpts.SYCLIsDevice)) {
 auto ES = getEmissionStatus(FD);
 if (ES == Sema::FunctionEmissionStatus::Emitted ||
 ES == Sema::FunctionEmissionStatus::Unknown)
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3489,6 +3489,9 @@
   if (Opts.OpenMPCUDAForceFullRuntime)
 GenerateArg(Args, OPT_fopenmp_cuda_force_full_runtime, SA);
 
+  if (Opts.OpenMPSkipDeferredDiags)
+GenerateArg(Args, OPT_fopenmp_skip_deferred_diags, SA);
+
   // The arguments used to set Optimize, OptimizeSize and NoInlineDefine are
   // generated from CodeGenOptions.
 
@@ -3928,6 +3931,10 @@
   Opts.OpenMPIsDevice && (T.isNVPTX() || T.isAMDGCN()) &&
   Args.hasArg(options::OPT_fopenmp_cuda_force_full_runtime);
 
+  Opts.OpenMPSkipDeferredDiags =
+  Opts.OpenMP && !Opts.OpenMPSimd && !IsTargetSpecified &&
+  Args.hasArg(options::OPT_fopenmp_skip_deferred_diags);
+
   // FIXME: Eliminate this dependency.
   unsigned Opt = getOptimizationLevel(Args, IK, Diags),
OptSize = getOptimizationLevelSize(Args);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5799,6 +5799,8 @@
   HelpText<"Generate code only for an OpenMP target device.">;
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
   HelpText<"Path to the IR file produced by the frontend for the host.">;
+def fopenmp_skip_deferred_diags : Flag<["-"], "fopenmp-skip-deferred-diags">,
+  HelpText<"Do not generate deferred diagnostics for function host/device 
availability. Use this flag only when no offloading or omp target directive is 
used.">;
 
 
//===--===//
 // SYCL Options
Index: 

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D70172#2964118 , @sugak wrote:

> Hi @yaxunl! I'm working on upgrading a large codebase from LLVM-9 to LLVM-12. 
> I noticed on average 10% compilation speed regression that seems to be caused 
> this change. We use Clang modules and historically provide `-fopenmp` 
> compiler flag by default. The problem seems to be that compiling and 
> importing modules is now slower, with the generated modules size increased by 
> 2X. llvm-bcanalyzer tool shows that it's dominated by 
> `DECLS_TO_CHECK_FOR_DEFERRED_DIAGS`.  If I understand it right, your change 
> is only relevant when target offloading is used. I inspected all of `#pragma 
> omp` directives and can confirm that we don't use it.
>
> I see that most of this code is gated by OpenMP flag. I wonder if there is a 
> finer grain way to enable openmp parallel code generation without target 
> offloading? Would it make sense to extend this code to check if 
> `-fopenom-targets` is set before recording 
> `DECLS_TO_CHECK_FOR_DEFERRED_DIAGS`?
>
> Note, this was measured with @weiwang's https://reviews.llvm.org/D101793.

We did an internal measurement by not adding decls into deferred diags, and 
that resolves the build regression. Wonder if we can have a special case for 
emitting diag as they are encountered when everything is on host side.


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

https://reviews.llvm.org/D70172

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D101793#2772717 , @yaxunl wrote:

> In D101793#2772461 , @weiwang wrote:
>
>> Thanks for the approval!
>>
>> Just want to understand the list of "decls to check for deferred 
>> diagnostics" better, where are these decls coming from? And why do they need 
>> to be checked for warnings? I see decls from libc are in the list, but I 
>> have no idea why are they selected.
>
> For offloading languages e.g. OpenMP/CUDA/HIP, there are apparent errors in 
> functions shared between host and device. However, unless these functions are 
> sure to be emitted on device or host, these errors should not be emitted. 
> These errors are so called deferred error messages. The function decls which 
> need to be checked are recorded. After AST is finalized, the AST of these 
> functions are iterated. If a function is found sure to be emitted, the 
> deferred error message in it are emitted.

Thanks! So the `DeclsToCheckForDeferredDiags` contains the candidate decls to 
be checked. The decls are selected because they would generate diags in the 
context of offloading languages, but whether or not an diag will be emitted is 
deferred till traversal of the AST is performed. I still don't quite understand 
why would libc functions be in the candidate list? They look simple enough (and 
I think they've been there forever).  For example `__uint16_identity` 
(https://code.woboq.org/userspace/glibc/bits/uintn-identity.h.html#32),

  static inline __uint16_t __uint16_identity(__uint16_t __x) {
  return __x;
  }

I don't see how this function would generate diag either on host or device.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

Thanks for the approval!

Just want to understand the list of "decls to check for deferred diagnostics" 
better, where are these decls coming from? And why do they need to be checked 
for warnings? I see decls from libc are in the list, but I have no idea why are 
they selected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang 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 rGe6b8320c0a63: [clang][AST] Improve AST Reader/Writer memory 
footprint (authored by weiwang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

Files:
  clang/include/clang/Sema/ExternalSemaSource.h
  clang/include/clang/Sema/MultiplexExternalSemaSource.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Sema/MultiplexExternalSemaSource.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4667,9 +4667,9 @@
   }
 
   // Build a record containing all of the DeclsToCheckForDeferredDiags.
-  RecordData DeclsToCheckForDeferredDiags;
+  SmallVector DeclsToCheckForDeferredDiags;
   for (auto *D : SemaRef.DeclsToCheckForDeferredDiags)
-AddDeclRef(D, DeclsToCheckForDeferredDiags);
+DeclsToCheckForDeferredDiags.push_back(GetDeclRef(D));
 
   RecordData DeclUpdatesOffsetsRecord;
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3810,7 +3810,7 @@
 
 case DECLS_TO_CHECK_FOR_DEFERRED_DIAGS:
   for (unsigned I = 0, N = Record.size(); I != N; ++I)
-DeclsToCheckForDeferredDiags.push_back(getGlobalDeclID(F, Record[I]));
+DeclsToCheckForDeferredDiags.insert(getGlobalDeclID(F, Record[I]));
   break;
 }
   }
@@ -8333,18 +8333,15 @@
 }
 
 void ASTReader::ReadDeclsToCheckForDeferredDiags(
-llvm::SmallVector ) {
-  for (unsigned I = 0, N = DeclsToCheckForDeferredDiags.size(); I != N;
-   ++I) {
-auto *D = dyn_cast_or_null(
-GetDecl(DeclsToCheckForDeferredDiags[I]));
+llvm::SmallSetVector ) {
+  for (auto I : DeclsToCheckForDeferredDiags) {
+auto *D = dyn_cast_or_null(GetDecl(I));
 if (D)
-  Decls.push_back(D);
+  Decls.insert(D);
   }
   DeclsToCheckForDeferredDiags.clear();
 }
 
-
 void ASTReader::ReadReferencedSelectors(
SmallVectorImpl> ) {
   if (ReferencedSelectorsData.empty())
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12539,7 +12539,7 @@
   }
 
   if (LangOpts.OpenMP && VDecl->isFileVarDecl())
-DeclsToCheckForDeferredDiags.push_back(VDecl);
+DeclsToCheckForDeferredDiags.insert(VDecl);
   CheckCompleteVariableDeclaration(VDecl);
 }
 
@@ -14773,7 +14773,7 @@
 auto ES = getEmissionStatus(FD);
 if (ES == Sema::FunctionEmissionStatus::Emitted ||
 ES == Sema::FunctionEmissionStatus::Unknown)
-  DeclsToCheckForDeferredDiags.push_back(FD);
+  DeclsToCheckForDeferredDiags.insert(FD);
   }
 
   return dcl;
Index: clang/lib/Sema/MultiplexExternalSemaSource.cpp
===
--- clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -268,7 +268,7 @@
 }
 
 void MultiplexExternalSemaSource::ReadDeclsToCheckForDeferredDiags(
-llvm::SmallVector ) {
+llvm::SmallSetVector ) {
   for(size_t i = 0; i < Sources.size(); ++i)
 Sources[i]->ReadDeclsToCheckForDeferredDiags(Decls);
 }
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -402,8 +402,8 @@
   /// headers. The declarations themselves are stored as declaration
   /// IDs, since they will be written out to an EAGERLY_DESERIALIZED_DECLS
   /// record.
-  SmallVector EagerlyDeserializedDecls;
-  SmallVector ModularCodegenDecls;
+  SmallVector EagerlyDeserializedDecls;
+  SmallVector ModularCodegenDecls;
 
   /// DeclContexts that have received extensions since their serialized
   /// form.
Index: clang/include/clang/Serialization/ASTReader.h
===
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -767,21 +767,21 @@
   /// This contains the data loaded from all EAGERLY_DESERIALIZED_DECLS blocks
   /// in the chain. The referenced declarations are deserialized and passed to
   /// the consumer eagerly.
-  SmallVector EagerlyDeserializedDecls;
+  SmallVector EagerlyDeserializedDecls;
 
   /// The IDs of all tentative definitions stored in the chain.
   ///
   /// Sema keeps track of all tentative definitions in a TU because 

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 346866.
weiwang added a comment.

make both ASTReader::DeclsToCheckForDeferredDiags and 
Sema::DeclsToCheckForDeferredDiags SmallSetVector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

Files:
  clang/include/clang/Sema/ExternalSemaSource.h
  clang/include/clang/Sema/MultiplexExternalSemaSource.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Sema/MultiplexExternalSemaSource.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4667,9 +4667,9 @@
   }
 
   // Build a record containing all of the DeclsToCheckForDeferredDiags.
-  RecordData DeclsToCheckForDeferredDiags;
+  SmallVector DeclsToCheckForDeferredDiags;
   for (auto *D : SemaRef.DeclsToCheckForDeferredDiags)
-AddDeclRef(D, DeclsToCheckForDeferredDiags);
+DeclsToCheckForDeferredDiags.push_back(GetDeclRef(D));
 
   RecordData DeclUpdatesOffsetsRecord;
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3810,7 +3810,7 @@
 
 case DECLS_TO_CHECK_FOR_DEFERRED_DIAGS:
   for (unsigned I = 0, N = Record.size(); I != N; ++I)
-DeclsToCheckForDeferredDiags.push_back(getGlobalDeclID(F, Record[I]));
+DeclsToCheckForDeferredDiags.insert(getGlobalDeclID(F, Record[I]));
   break;
 }
   }
@@ -8333,18 +8333,15 @@
 }
 
 void ASTReader::ReadDeclsToCheckForDeferredDiags(
-llvm::SmallVector ) {
-  for (unsigned I = 0, N = DeclsToCheckForDeferredDiags.size(); I != N;
-   ++I) {
-auto *D = dyn_cast_or_null(
-GetDecl(DeclsToCheckForDeferredDiags[I]));
+llvm::SmallSetVector ) {
+  for (auto I : DeclsToCheckForDeferredDiags) {
+auto *D = dyn_cast_or_null(GetDecl(I));
 if (D)
-  Decls.push_back(D);
+  Decls.insert(D);
   }
   DeclsToCheckForDeferredDiags.clear();
 }
 
-
 void ASTReader::ReadReferencedSelectors(
SmallVectorImpl> ) {
   if (ReferencedSelectorsData.empty())
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12539,7 +12539,7 @@
   }
 
   if (LangOpts.OpenMP && VDecl->isFileVarDecl())
-DeclsToCheckForDeferredDiags.push_back(VDecl);
+DeclsToCheckForDeferredDiags.insert(VDecl);
   CheckCompleteVariableDeclaration(VDecl);
 }
 
@@ -14773,7 +14773,7 @@
 auto ES = getEmissionStatus(FD);
 if (ES == Sema::FunctionEmissionStatus::Emitted ||
 ES == Sema::FunctionEmissionStatus::Unknown)
-  DeclsToCheckForDeferredDiags.push_back(FD);
+  DeclsToCheckForDeferredDiags.insert(FD);
   }
 
   return dcl;
Index: clang/lib/Sema/MultiplexExternalSemaSource.cpp
===
--- clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -268,7 +268,7 @@
 }
 
 void MultiplexExternalSemaSource::ReadDeclsToCheckForDeferredDiags(
-llvm::SmallVector ) {
+llvm::SmallSetVector ) {
   for(size_t i = 0; i < Sources.size(); ++i)
 Sources[i]->ReadDeclsToCheckForDeferredDiags(Decls);
 }
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -402,8 +402,8 @@
   /// headers. The declarations themselves are stored as declaration
   /// IDs, since they will be written out to an EAGERLY_DESERIALIZED_DECLS
   /// record.
-  SmallVector EagerlyDeserializedDecls;
-  SmallVector ModularCodegenDecls;
+  SmallVector EagerlyDeserializedDecls;
+  SmallVector ModularCodegenDecls;
 
   /// DeclContexts that have received extensions since their serialized
   /// form.
Index: clang/include/clang/Serialization/ASTReader.h
===
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -767,21 +767,21 @@
   /// This contains the data loaded from all EAGERLY_DESERIALIZED_DECLS blocks
   /// in the chain. The referenced declarations are deserialized and passed to
   /// the consumer eagerly.
-  SmallVector EagerlyDeserializedDecls;
+  SmallVector EagerlyDeserializedDecls;
 
   /// The IDs of all tentative definitions stored in the chain.
   ///
   /// Sema keeps track of all tentative definitions in a TU because it has to
   /// complete them and pass them on to CodeGen. Thus, 

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D101793#2772021 , @yaxunl wrote:

> In D101793#2769297 , @weiwang wrote:
>
>> Tried to make `Sema::DeclsToCheckForDeferredDiags` `llvm::SmallSetVector`. 
>> The heap RSS did drop significantly (from peak 100GB to 59GB) , but not as 
>> good as the current fix (peak 26GB), which makes 
>> `ASTReader::DeclsToCheckForDeferredDiags` `llvm::SmallSetVector`.
>>
>> I think the reason is that the duplicated decls are read from multiple 
>> module file sources (`ASTReader::ReadAST()` -> `ASTReader::ReadASTBlock()`), 
>> then stored into `ASTReader::DeclsToCheckForDeferredDiags`, then goes into 
>> `Sema::DeclsToCheckForDeferredDiags` in 
>> `ASTReader::ReadDeclsToCheckForDeferredDiags()`. Doing dedup at the early 
>> stage when the decls were just read in `ASTReader` is more effective at 
>> reducing RSS.
>
> What if you use SmallSetVector for both Sema::DeclsToCheckForDeferredDiags 
> and ASTReader::DeclsToCheckForDeferredDiags? Does it cause extra memory usage 
> compared to using it only for ASTReader::DeclsToCheckForDeferredDiags? Thanks.

There would be a slight increase in memory usage since SmallSetVector requires 
more memory than just SmallVector internally, but given the majority the RSS 
comes from duplicated decls, I don't think it's an issue by making both 
SmallSetVector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

Tried to make `Sema::DeclsToCheckForDeferredDiags` `llvm::SmallSetVector`. The 
heap RSS did drop significantly (from peak 100GB to 59GB) , but not as good as 
the current fix (peak 26GB), which makes 
`ASTReader::DeclsToCheckForDeferredDiags` `llvm::SmallSetVector`.

I think the reason is that the duplicated decls are read from multiple module 
file sources (`ASTReader::ReadAST()` -> `ASTReader::ReadASTBlock()`), then 
stored into `ASTReader::DeclsToCheckForDeferredDiags`, then goes into 
`Sema::DeclsToCheckForDeferredDiags` in 
`ASTReader::ReadDeclsToCheckForDeferredDiags()`. Doing dedup at the early stage 
when the decls were just read in `ASTReader` is more effective at reducing RSS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D101793#2760639 , @yaxunl wrote:

> I think the root cause might be duplicated decls are added to 
> Sema::DeclsToCheckForDeferredDiags defined in
>
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/Sema.h#L1789
>
> When compiling source codes, a decl is added only once. However if modules 
> are imported, duplicate decls may be added.
>
> We need to avoid adding duplicate decls to 
> Sema::DeclsToCheckForDeferredDiags. However we cannot simply change it to a 
> set since the order is important, otherwise the error message for later code 
> may show up earlier, causing confusion for users. I would suggest to change 
> its type to SetVector, which keeps the order and also avoids duplicates.

Thanks for the suggestion! It does make more sense to use SetVector here. I 
will try this and report back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

Finally dealt with the other issues I need to take care. Let's resume the 
discussion.

I printed out the decls that are duplicated. The list of very long, but the top 
ones all seem from glibc headers. For example (the number after 'c:' is the 
duplication count),

  static inline __uint16_t __bswap_16(__uint16_t __bsx) {
  return ((__uint16_t)__bsx) >> 8) & 255) | (((__bsx) & 255) << 8)));
  }
  c:8581627
  static inline __uint16_t __bswap_16(__uint16_t __bsx)
  c:8581627
  static inline __uint32_t __bswap_32(__uint32_t __bsx) {
  return __bsx) & 4278190080U) >> 24) | (((__bsx) & 16711680U) >> 8) | 
(((__bsx) & 65280U) << 8) | (((__bsx) & 255U) << 24));
  }
  c:8581627
  static inline __uint32_t __bswap_32(__uint32_t __bsx)
  c:8581627
  static inline __uint64_t __bswap_64(__uint64_t __bsx) {
  return __bsx) & 18374686479671623680ULL) >> 56) | (((__bsx) & 
71776119061217280ULL) >> 40) | (((__bsx) & 280375465082880ULL) >> 24) |   
(((__bsx) & 1095216660480ULL) >> 8) | (((__bsx) & 4278190080ULL) << 8) | 
(((__bsx) & 16711680ULL) << 24) | (((__bsx) & 65280ULL) << 40) |   
(((__bsx) & 255ULL) << 56));
  }
  c:8581627
  static inline __uint64_t __bswap_64(__uint64_t __bsx)
  c:8581627
  static inline __uint16_t __uint16_identity(__uint16_t __x) {
  return __x;
  }
  c:8581627
  static inline __uint32_t __uint32_identity(__uint32_t __x) {
  return __x;
  }
  c:8581627
  static inline __uint64_t __uint64_identity(__uint64_t __x) {
  return __x;
  }
  c:8581627
  inline int iscanonical(float __val) {
  return ((void)(typeof (__val))(__val) , 1);
  }
  c:8581627
  inline int iscanonical(double __val) {
  return ((void)(typeof (__val))(__val) , 1);
  }
  c:8581627
  inline int iscanonical(long double __val) {
  return __iscanonicall(__val);
  }
  c:8581627
  inline int issignaling(float __val) {
  return __issignalingf(__val);
  }
  c:8581627
  inline int issignaling(double __val) {
  return __issignaling(__val);
  }
  c:8581627
  inline int issignaling(long double __val) {
  return __issignalingl(__val);
  }
  c:8581627
  static int __call(float __x, float __y) throw() {
  return __iseqsigf(__x, __y);
  }
  c:8581627
  static int __call(double __x, double __y) throw() {
  return __iseqsig(__x, __y);
  }
  c:8581627
  static int __call(long double __x, long double __y) throw() {
  return __iseqsigl(__x, __y);
  }
  c:8581627

I think the source codebase is a mix of conventional headers and modules, I 
don't know if that makes any difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D101793#2735336 , @yaxunl wrote:

> Decls in Sema::DeclsToCheckForDeferredDiags is supposed to be unique. 
> Therefore the fact that '1,734,387,685 out of 1,734,404,000 elements are the 
> same' is surprising. Did this happen when you compile the source code and 
> write AST? What language was the source code? C++, OpenMP, or CUDA? What was 
> the decl that got duplicated? Thanks.

Sorry for the late reply.

This happens during a single compilation instance, and emits a module file from 
a big list of module map files and headers. I believe the code is pure C++. 
Because of the huge amount of duplications, the memory RSS shoots to over 50GB. 
I'll update once having more information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

We've seen a huge memory footprint from AST Reader/Writer in a single CU with 
module enabled. Upon further analysis, the content of vector 
`DeclsToCheckForDeferredDiags` seems mostly redundant. In one case, 
1,734,387,685 out of 1,734,404,000 elements are the same. While this may 
indicate something wrong with the source itself, it also suggests that compiler 
would be better to perform deduplication on this type of Decl ID.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101793

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: hoy, wenlei.
weiwang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Reduce memory footprint of AST Reader/Writer:

1. Adjust internal data containers' element type.
2. Switch to set for deduplication of deferred diags.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101793

Files:
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4733,9 +4733,9 @@
   }
 
   // Build a record containing all of the DeclsToCheckForDeferredDiags.
-  RecordData DeclsToCheckForDeferredDiags;
+  SmallVector DeclsToCheckForDeferredDiags;
   for (auto *D : SemaRef.DeclsToCheckForDeferredDiags)
-AddDeclRef(D, DeclsToCheckForDeferredDiags);
+DeclsToCheckForDeferredDiags.push_back(GetDeclRef(D));
 
   RecordData DeclUpdatesOffsetsRecord;
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3835,7 +3835,7 @@
 
 case DECLS_TO_CHECK_FOR_DEFERRED_DIAGS:
   for (unsigned I = 0, N = Record.size(); I != N; ++I)
-DeclsToCheckForDeferredDiags.push_back(getGlobalDeclID(F, Record[I]));
+DeclsToCheckForDeferredDiags.insert(getGlobalDeclID(F, Record[I]));
   break;
 }
   }
@@ -8359,10 +8359,8 @@
 
 void ASTReader::ReadDeclsToCheckForDeferredDiags(
 llvm::SmallVector ) {
-  for (unsigned I = 0, N = DeclsToCheckForDeferredDiags.size(); I != N;
-   ++I) {
-auto *D = dyn_cast_or_null(
-GetDecl(DeclsToCheckForDeferredDiags[I]));
+  for (auto I : DeclsToCheckForDeferredDiags) {
+auto *D = dyn_cast_or_null(GetDecl(I));
 if (D)
   Decls.push_back(D);
   }
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -402,8 +402,8 @@
   /// headers. The declarations themselves are stored as declaration
   /// IDs, since they will be written out to an EAGERLY_DESERIALIZED_DECLS
   /// record.
-  SmallVector EagerlyDeserializedDecls;
-  SmallVector ModularCodegenDecls;
+  SmallVector EagerlyDeserializedDecls;
+  SmallVector ModularCodegenDecls;
 
   /// DeclContexts that have received extensions since their serialized
   /// form.
Index: clang/include/clang/Serialization/ASTReader.h
===
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -767,21 +767,21 @@
   /// This contains the data loaded from all EAGERLY_DESERIALIZED_DECLS blocks
   /// in the chain. The referenced declarations are deserialized and passed to
   /// the consumer eagerly.
-  SmallVector EagerlyDeserializedDecls;
+  SmallVector EagerlyDeserializedDecls;
 
   /// The IDs of all tentative definitions stored in the chain.
   ///
   /// Sema keeps track of all tentative definitions in a TU because it has to
   /// complete them and pass them on to CodeGen. Thus, tentative definitions in
   /// the PCH chain must be eagerly deserialized.
-  SmallVector TentativeDefinitions;
+  SmallVector TentativeDefinitions;
 
   /// The IDs of all CXXRecordDecls stored in the chain whose VTables are
   /// used.
   ///
   /// CodeGen has to emit VTables for these records, so they have to be eagerly
   /// deserialized.
-  SmallVector VTableUses;
+  SmallVector VTableUses;
 
   /// A snapshot of the pending instantiations in the chain.
   ///
@@ -789,7 +789,7 @@
   /// end of the TU. It consists of a pair of values for every pending
   /// instantiation where the first value is the ID of the decl and the second
   /// is the instantiation location.
-  SmallVector PendingInstantiations;
+  SmallVector PendingInstantiations;
 
   //@}
 
@@ -799,24 +799,24 @@
 
   /// A snapshot of Sema's unused file-scoped variable tracking, for
   /// generating warnings.
-  SmallVector UnusedFileScopedDecls;
+  SmallVector UnusedFileScopedDecls;
 
   /// A list of all the delegating constructors we've seen, to diagnose
   /// cycles.
-  SmallVector DelegatingCtorDecls;
+  SmallVector DelegatingCtorDecls;
 
   /// Method selectors used in a @selector expression. Used for
   /// implementation of -Wselector.
-  SmallVector ReferencedSelectorsData;
+  SmallVector ReferencedSelectorsData;
 
   /// A snapshot of Sema's weak undeclared identifier tracking, for
   /// generating warnings.
-  SmallVector WeakUndeclaredIdentifiers;
+  SmallVector WeakUndeclaredIdentifiers;
 
   /// The IDs 

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

Ah, I am sorry. Thanks for fixing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang 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 rG93dc1b5b8cb2: [Remarks][2/2] Expand remarks hotness 
threshold option support in more tools (authored by weiwang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/opt-record.c
  clang/test/Frontend/Inputs/remarks-hotness.prof
  clang/test/Frontend/remarks-hotness.cpp
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll

Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- /dev/null
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -0,0 +1,96 @@
+;; This test verifies 'auto' hotness threshold when profile file is provided.
+;;
+;; new PM
+; RUN: rm -f %t.yaml %t.hot.yaml
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.yaml \
+; RUN: -pass-remarks-with-hotness --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.yaml
+; RUN: FileCheck %s -check-prefix=YAML-MISS < %t.yaml
+
+;; test 'auto' threshold
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.hot.yaml \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.hot.yaml
+; RUN: not FileCheck %s -check-prefix=YAML-MISS < %t.hot.yaml
+
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks=inline --pass-remarks-missed=inline --pass-remarks-analysis=inline \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+
+; YAML-PASS:  --- !Passed
+; YAML-PASS-NEXT: Pass:inline
+; YAML-PASS-NEXT: Name:Inlined
+; YAML-PASS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 10, Column: 10 }
+; YAML-PASS-NEXT: Function:_Z7caller1v
+; YAML-PASS-NEXT: Hotness: 401
+
+; YAML-MISS:  --- !Missed
+; YAML-MISS-NEXT: Pass:inline
+; YAML-MISS-NEXT: Name:NeverInline
+; YAML-MISS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 14, Column: 10 }
+; YAML-MISS-NEXT: Function:_Z7caller2v
+; YAML-MISS-NEXT: Hotness: 2
+
+; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1 (hotness: 401)
+; CHECK-RPASS-NOT: _Z7callee2v not inlined into _Z7caller2v because it should never be inlined (cost=never): noinline function attribute (hotness: 2)
+
+; ModuleID = 'remarks-hotness.cpp'
+source_filename = "remarks-hotness.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7callee1v() #0 !dbg !7 {
+  ret i32 1, !dbg !11
+}
+
+; Function Attrs: noinline nounwind uwtable use-sample-profile
+define dso_local i32 @_Z7callee2v() #1 !dbg !12 {
+  ret i32 2, !dbg !13
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller1v() #0 !dbg !14 {
+  %1 = call i32 @_Z7callee1v(), !dbg !15
+  ret i32 %1, !dbg !16
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller2v() #0 !dbg !17 {
+  %1 = call i32 @_Z7callee2v(), !dbg !18
+  ret i32 %1, !dbg !19
+}
+
+attributes #0 = { "use-sample-profile" }
+attributes #1 = { noinline nounwind uwtable "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "remarks-hotness.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, 

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.



In D85808#2424588 , @tejohnson wrote:

> lgtm with a couple of minor nits noted below that you can fix before 
> submitting

Thanks for pointing them out. Really appreciate it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 308536.
weiwang added a comment.

1. Fix typo.
2. Minor order adjustment in testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/opt-record.c
  clang/test/Frontend/Inputs/remarks-hotness.prof
  clang/test/Frontend/remarks-hotness.cpp
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll

Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- /dev/null
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -0,0 +1,96 @@
+;; This test verifies 'auto' hotness threshold when profile file is provided.
+;;
+;; new PM
+; RUN: rm -f %t.yaml %t.hot.yaml
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.yaml \
+; RUN: -pass-remarks-with-hotness --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.yaml
+; RUN: FileCheck %s -check-prefix=YAML-MISS < %t.yaml
+
+;; test 'auto' threshold
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.hot.yaml \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.hot.yaml
+; RUN: not FileCheck %s -check-prefix=YAML-MISS < %t.hot.yaml
+
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks=inline --pass-remarks-missed=inline --pass-remarks-analysis=inline \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+
+; YAML-PASS:  --- !Passed
+; YAML-PASS-NEXT: Pass:inline
+; YAML-PASS-NEXT: Name:Inlined
+; YAML-PASS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 10, Column: 10 }
+; YAML-PASS-NEXT: Function:_Z7caller1v
+; YAML-PASS-NEXT: Hotness: 401
+
+; YAML-MISS:  --- !Missed
+; YAML-MISS-NEXT: Pass:inline
+; YAML-MISS-NEXT: Name:NeverInline
+; YAML-MISS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 14, Column: 10 }
+; YAML-MISS-NEXT: Function:_Z7caller2v
+; YAML-MISS-NEXT: Hotness: 2
+
+; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1 (hotness: 401)
+; CHECK-RPASS-NOT: _Z7callee2v not inlined into _Z7caller2v because it should never be inlined (cost=never): noinline function attribute (hotness: 2)
+
+; ModuleID = 'remarks-hotness.cpp'
+source_filename = "remarks-hotness.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7callee1v() #0 !dbg !7 {
+  ret i32 1, !dbg !11
+}
+
+; Function Attrs: noinline nounwind uwtable use-sample-profile
+define dso_local i32 @_Z7callee2v() #1 !dbg !12 {
+  ret i32 2, !dbg !13
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller1v() #0 !dbg !14 {
+  %1 = call i32 @_Z7callee1v(), !dbg !15
+  ret i32 %1, !dbg !16
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller2v() #0 !dbg !17 {
+  %1 = call i32 @_Z7callee2v(), !dbg !18
+  ret i32 %1, !dbg !19
+}
+
+attributes #0 = { "use-sample-profile" }
+attributes #1 = { noinline nounwind uwtable "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "remarks-hotness.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 11.0.0"}
+!7 = distinct !DISubprogram(name: "callee1", linkageName: "_Z7callee1v", scope: !1, 

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 308433.
weiwang added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/opt-record.c
  clang/test/Frontend/Inputs/remarks-hotness.prof
  clang/test/Frontend/remarks-hotness.cpp
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll

Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- /dev/null
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -0,0 +1,96 @@
+;; This test verifies 'auto' hotness threshold when profile file is provided.
+;;
+;; new PM
+; RUN: rm -f %t.yaml %t.hot.yaml
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.yaml \
+; RUN: -pass-remarks-with-hotness --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.yaml
+; RUN: FileCheck %s -check-prefix=YAML-MISS < %t.yaml
+
+;; test 'auto' threshold
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.hot.yaml \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.hot.yaml
+; RUN: not FileCheck %s -check-prefix=YAML-MISS < %t.hot.yaml
+
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks=inline --pass-remarks-missed=inline --pass-remarks-analysis=inline \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+
+; YAML-PASS:  --- !Passed
+; YAML-PASS-NEXT: Pass:inline
+; YAML-PASS-NEXT: Name:Inlined
+; YAML-PASS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 10, Column: 10 }
+; YAML-PASS-NEXT: Function:_Z7caller1v
+; YAML-PASS-NEXT: Hotness: 401
+
+; YAML-MISS:  --- !Missed
+; YAML-MISS-NEXT: Pass:inline
+; YAML-MISS-NEXT: Name:NeverInline
+; YAML-MISS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 14, Column: 10 }
+; YAML-MISS-NEXT: Function:_Z7caller2v
+; YAML-MISS-NEXT: Hotness: 2
+
+; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1 (hotness: 401)
+; CHECK-RPASS-NOT: _Z7callee2v not inlined into _Z7caller2v because it should never be inlined (cost=never): noinline function attribute (hotness: 2)
+
+; ModuleID = 'remarks-hotness.cpp'
+source_filename = "remarks-hotness.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7callee1v() #0 !dbg !7 {
+  ret i32 1, !dbg !11
+}
+
+; Function Attrs: noinline nounwind uwtable use-sample-profile
+define dso_local i32 @_Z7callee2v() #1 !dbg !12 {
+  ret i32 2, !dbg !13
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller1v() #0 !dbg !14 {
+  %1 = call i32 @_Z7callee1v(), !dbg !15
+  ret i32 %1, !dbg !16
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller2v() #0 !dbg !17 {
+  %1 = call i32 @_Z7callee2v(), !dbg !18
+  ret i32 %1, !dbg !19
+}
+
+attributes #0 = { "use-sample-profile" }
+attributes #1 = { noinline nounwind uwtable "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "remarks-hotness.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 11.0.0"}
+!7 = distinct !DISubprogram(name: "callee1", linkageName: "_Z7callee1v", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, 

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.
Herald added a subscriber: hoy.

@tejohnson @MaskRay Do you have other comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D85808#2403587 , @tejohnson wrote:

> Thanks for adding the Driver test. I was thinking of something to test the 
> CompilerInvocation changes, similar to your test using opt, that ensures the 
> option has the desired behavior when invoked via clang. Looks like there is 
> an existing test clang/test/Frontend/optimization-remark-with-hotness.c that 
> perhaps could be extended or leveraged?

Thanks for the suggestion. I have added a new test case for clang. The source 
file used is the same one in the opt test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 306256.
weiwang added a comment.

1. Add clang test with remarks output.
2. Fix a missing dependency on PSI in legacy pass manager.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/opt-record.c
  clang/test/Frontend/Inputs/remarks-hotness.prof
  clang/test/Frontend/remarks-hotness.cpp
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll

Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- /dev/null
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -0,0 +1,96 @@
+;; This test verifies 'auto' hotness threshold when profile file is provided.
+;;
+;; new PM
+; RUN: rm -f %t.yaml %t.hot.yaml
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.yaml \
+; RUN: -pass-remarks-with-hotness --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.yaml
+; RUN: FileCheck %s -check-prefix=YAML-MISS < %t.yaml
+
+;; test 'auto' threshold
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.hot.yaml \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.hot.yaml
+; RUN: not FileCheck %s -check-prefix=YAML-MISS < %t.hot.yaml
+
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks=inline --pass-remarks-missed=inline --pass-remarks-analysis=inline \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+
+; YAML-PASS:  --- !Passed
+; YAML-PASS-NEXT: Pass:inline
+; YAML-PASS-NEXT: Name:Inlined
+; YAML-PASS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 10, Column: 10 }
+; YAML-PASS-NEXT: Function:_Z7caller1v
+; YAML-PASS-NEXT: Hotness: 401
+
+; YAML-MISS:  --- !Missed
+; YAML-MISS-NEXT: Pass:inline
+; YAML-MISS-NEXT: Name:NeverInline
+; YAML-MISS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 14, Column: 10 }
+; YAML-MISS-NEXT: Function:_Z7caller2v
+; YAML-MISS-NEXT: Hotness: 2
+
+; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1 (hotness: 401)
+; CHECK-RPASS-NOT: _Z7callee2v not inlined into _Z7caller2v because it should never be inlined (cost=never): noinline function attribute (hotness: 2)
+
+; ModuleID = 'remarks-hotness.cpp'
+source_filename = "remarks-hotness.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7callee1v() #0 !dbg !7 {
+  ret i32 1, !dbg !11
+}
+
+; Function Attrs: noinline nounwind uwtable use-sample-profile
+define dso_local i32 @_Z7callee2v() #1 !dbg !12 {
+  ret i32 2, !dbg !13
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller1v() #0 !dbg !14 {
+  %1 = call i32 @_Z7callee1v(), !dbg !15
+  ret i32 %1, !dbg !16
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller2v() #0 !dbg !17 {
+  %1 = call i32 @_Z7callee2v(), !dbg !18
+  ret i32 %1, !dbg !19
+}
+
+attributes #0 = { "use-sample-profile" }
+attributes #1 = { noinline nounwind uwtable "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "remarks-hotness.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 11.0.0"}
+!7 = distinct !DISubprogram(name: 

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 305891.
weiwang added a comment.

update test case for clang option pass-through


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/opt-record.c
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll

Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- /dev/null
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -0,0 +1,96 @@
+;; This test verifies 'auto' hotness threshold when profile file is provided.
+;;
+;; new PM
+; RUN: rm -f %t.yaml %t.hot.yaml
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.yaml \
+; RUN: -pass-remarks-with-hotness --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.yaml
+; RUN: FileCheck %s -check-prefix=YAML-MISS < %t.yaml
+
+;; test 'auto' threshold
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.hot.yaml \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.hot.yaml
+; RUN: not FileCheck %s -check-prefix=YAML-MISS < %t.hot.yaml
+
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks=inline --pass-remarks-missed=inline --pass-remarks-analysis=inline \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+
+; YAML-PASS:  --- !Passed
+; YAML-PASS-NEXT: Pass:inline
+; YAML-PASS-NEXT: Name:Inlined
+; YAML-PASS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 10, Column: 10 }
+; YAML-PASS-NEXT: Function:_Z7caller1v
+; YAML-PASS-NEXT: Hotness: 401
+
+; YAML-MISS:  --- !Missed
+; YAML-MISS-NEXT: Pass:inline
+; YAML-MISS-NEXT: Name:NeverInline
+; YAML-MISS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 14, Column: 10 }
+; YAML-MISS-NEXT: Function:_Z7caller2v
+; YAML-MISS-NEXT: Hotness: 2
+
+; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1 (hotness: 401)
+; CHECK-RPASS-NOT: _Z7callee2v not inlined into _Z7caller2v because it should never be inlined (cost=never): noinline function attribute (hotness: 2)
+
+; ModuleID = 'remarks-hotness.cpp'
+source_filename = "remarks-hotness.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7callee1v() #0 !dbg !7 {
+  ret i32 1, !dbg !11
+}
+
+; Function Attrs: noinline nounwind uwtable use-sample-profile
+define dso_local i32 @_Z7callee2v() #1 !dbg !12 {
+  ret i32 2, !dbg !13
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller1v() #0 !dbg !14 {
+  %1 = call i32 @_Z7callee1v(), !dbg !15
+  ret i32 %1, !dbg !16
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller2v() #0 !dbg !17 {
+  %1 = call i32 @_Z7callee2v(), !dbg !18
+  ret i32 %1, !dbg !19
+}
+
+attributes #0 = { "use-sample-profile" }
+attributes #1 = { noinline nounwind uwtable "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "remarks-hotness.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 11.0.0"}
+!7 = distinct !DISubprogram(name: "callee1", linkageName: "_Z7callee1v", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, 

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang closed this revision.
weiwang added a comment.

Diff was committed, but did not close automatically. Manual close it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 301140.
weiwang added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -41,3 +41,37 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64 -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux -### -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-A
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-filename="
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-A:  "--plugin-opt=opt-remarks-filename=a.out.opt.ld.yaml"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS:  "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM:  "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM-SAME: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS:  "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,62 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A)
+F = A->getValue();
+  else if (Output.isFilename())
+F = Output.getFilename();
+
+  assert(!F.empty() && "Cannot determine remarks output name.");
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ Twine(".opt.ld.") + Format));
+
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_foptimization_record_passes_EQ))
+

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

@tejohnson @MaskRay Do you have other comments to the change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 293296.
weiwang added a comment.

minor update to test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -41,3 +41,37 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64 -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux -### -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-A
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-filename="
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-A:  "--plugin-opt=opt-remarks-filename=a.out.opt.ld.yaml"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS:  "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM:  "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM-SAME: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS:  "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,62 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A)
+F = A->getValue();
+  else if (Output.isFilename())
+F = Output.getFilename();
+
+  assert(F.empty() && "Cannot determine remarks output name.");
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ Twine(".opt.ld.") + Format));
+
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_foptimization_record_passes_EQ))
+

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 293276.
weiwang added a comment.

1. remove unreachable code
2. udpate test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -41,3 +41,37 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64 -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | FileCheck %s -check-prefix=CHECK-NOPASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux -### -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-A
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-NOPASS-NOT: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-A:  "--plugin-opt=opt-remarks-filename=a.out.opt.ld.yaml"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS-A-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS:  "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM:  "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM-SAME: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS:  "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,62 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A)
+F = A->getValue();
+  else if (Output.isFilename())
+F = Output.getFilename();
+
+  assert(F.empty() && "Cannot determine remarks output name.");
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ Twine(".opt.ld.") + Format));
+
+  if (const Arg *A =
+  

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang marked an inline comment as done.
weiwang added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:90
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();

MaskRay wrote:
> The output selection logic is untested.
I am not sure how to trigger a case of `Output` not being a filename in test 
case. 



Comment at: clang/test/Driver/opt-record.c:24
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO 
-fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not 
FileCheck %s -check-prefix=CHECK-PASS
+

MaskRay wrote:
> x86_64-linux-gnu can usually be simplied as x86_64.
> 
> There is no test without `-o FOO`.
> 
> Consider moving the RUN lines below, immediately above CHECK-PASS: lines
`x86_64` somehow gives error for the toolchain to generate linker command. I've 
updated the triple to be `x86_64-linux`.

Without `-o FOO`, it would just use the default output name `a.out`.

As suggested, changed lines are moved together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 292676.
weiwang added a comment.

update test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -41,3 +41,27 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64 -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s -check-prefix=CHECK-PASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
+// CHECK-PASS:  "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM:  "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM-SAME: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS:  "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS-SAME: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,66 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+F = A->getValue();
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();
+
+// Use the input filename.
+if (F.empty())
+  F = llvm::sys::path::stem(Input.getBaseInput());
+  }
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ Twine(".opt.ld.") + Format));
+
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_foptimization_record_passes_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=opt-remarks-passes=") + A->getValue()));
+
+  CmdArgs.push_back(Args.MakeArgString(
+  Twine("--plugin-opt=opt-remarks-format=") + Format.data()));
+}
+
+static void renderRemarksHotnessOptions(const ArgList ,
+ArgStringList ) {
+  if (Args.hasFlag(options::OPT_fdiagnostics_show_hotness,
+   options::OPT_fno_diagnostics_show_hotness, false))
+

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 288447.
weiwang added a comment.

remove redundant check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -18,7 +18,17 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format -fno-save-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-FOPT-DISABLE-FORMAT
-//
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s -check-prefix=CHECK-PASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
 
@@ -41,3 +51,17 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// CHECK-PASS: "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,66 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+F = A->getValue();
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();
+
+// Use the input filename.
+if (F.empty())
+  F = llvm::sys::path::stem(Input.getBaseInput());
+  }
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ Twine(".opt.ld.") + Format));
+
+  if (const Arg *A =
+  

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:596
+  // Handle remark diagnostics on screen options: '-Rpass-*'.
+  if (usesRpassOptions(Args))
+renderRpassOptions(Args, CmdArgs);

tejohnson wrote:
> Is this guard needed? It looks like renderRpassOptions will do the right 
> thing if none are specified (won't add anything). Right now you end up 
> checking each one a second time if any are enabled.
You are right! I will remove the redundant check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Wei Wang via Phabricator via cfe-commits]

weiwang added inline comments.



Comment at: llvm/lib/Analysis/OptimizationRemarkEmitter.cpp:103
 BFI = ().getBFI();
-  else
+// Get hotness threshold from PSI. This should only happen once.
+if (Context.isDiagnosticsHotnessThresholdSetFromPSI()) {

tejohnson wrote:
> Can you clarify what you mean by this only happening once?
The check of `Context.isDiagnosticsHotnessThresholdSetFromPSI()` guarantees 
that `ProfileSummaryInfoWrapperPass` and `setDiagnosticsHotnessThreshold` would 
only be called once. Once the threshold is set from PSI, the check always 
returns false. But strictly speaking, since PSI is immutable, it only happens 
once with/without the check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 286468.
weiwang marked an inline comment as done.
weiwang added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -18,7 +18,17 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format -fno-save-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-FOPT-DISABLE-FORMAT
-//
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s -check-prefix=CHECK-PASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
 
@@ -41,3 +51,17 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// CHECK-PASS: "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,66 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+F = A->getValue();
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();
+
+// Use the input filename.
+if (F.empty())
+  F = llvm::sys::path::stem(Input.getBaseInput());
+  }
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ Twine(".opt.ld.") 

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang marked 5 inline comments as done.
weiwang added inline comments.



Comment at: clang/include/clang/Driver/Driver.h:638
+/// This checks for clang specific R-value ('-Rpass-*') group.
+bool hasRpassOptions(const llvm::opt::ArgList );
+

bruno wrote:
> Nitpicking here, how about `usesRpassOptions`?
Thanks! Suggested name makes more sense. fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 286467.
weiwang added a comment.

coding style change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -18,7 +18,17 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format -fno-save-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-FOPT-DISABLE-FORMAT
-//
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s -check-prefix=CHECK-PASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
 
@@ -41,3 +51,17 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// CHECK-PASS: "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,66 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+F = A->getValue();
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();
+
+// Use the input filename.
+if (F.empty())
+  F = llvm::sys::path::stem(Input.getBaseInput());
+  }
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ Twine(".opt.ld.") + Format));
+
+  if (const Arg 

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 286204.
weiwang added a comment.

move some unrelated change into parent diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c

Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -18,7 +18,17 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format -fno-save-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-FOPT-DISABLE-FORMAT
-//
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s -check-prefix=CHECK-PASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
 
@@ -41,3 +51,17 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// CHECK-PASS: "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,70 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ)) {
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ)) {
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-missed=") + A->getValue()));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ)) {
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--plugin-opt=-pass-remarks-analysis=") + A->getValue()));
+  }
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+F = A->getValue();
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();
+
+if (F.empty()) {
+  // Use the input filename.
+  F = llvm::sys::path::stem(Input.getBaseInput());
+}
+  }
+  // Append "opt.ld." to the end of the file name.
+  CmdArgs.push_back(
+  Args.MakeArgString(Twine("--plugin-opt=opt-remarks-filename=") + F +
+ 

[PATCH] D85808: [remarks] Optimization remarks hotness filtering from profile summary

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 286203.
weiwang added a comment.

update:

1. reorganize code splitting with parent diff.
2. format and style change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll

Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- /dev/null
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -0,0 +1,96 @@
+;; This test verifies 'auto' hotness threshold when profile file is provided.
+;;
+;; new PM
+; RUN: rm -f %t.yaml %t.hot.yaml
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.yaml \
+; RUN: -pass-remarks-with-hotness --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.yaml
+; RUN: FileCheck %s -check-prefix=YAML-MISS < %t.yaml
+
+;; test 'auto' threshold
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.hot.yaml \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.hot.yaml
+; RUN: not FileCheck %s -check-prefix=YAML-MISS < %t.hot.yaml
+
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks=inline --pass-remarks-missed=inline --pass-remarks-analysis=inline \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+
+; YAML-PASS:  --- !Passed
+; YAML-PASS-NEXT: Pass:inline
+; YAML-PASS-NEXT: Name:Inlined
+; YAML-PASS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 10, Column: 10 }
+; YAML-PASS-NEXT: Function:_Z7caller1v
+; YAML-PASS-NEXT: Hotness: 401
+
+; YAML-MISS:  --- !Missed
+; YAML-MISS-NEXT: Pass:inline
+; YAML-MISS-NEXT: Name:NeverInline
+; YAML-MISS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 14, Column: 10 }
+; YAML-MISS-NEXT: Function:_Z7caller2v
+; YAML-MISS-NEXT: Hotness: 2
+
+; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1 (hotness: 401)
+; CHECK-RPASS-NOT: _Z7callee2v not inlined into _Z7caller2v because it should never be inlined (cost=never): noinline function attribute (hotness: 2)
+
+; ModuleID = 'remarks-hotness.cpp'
+source_filename = "remarks-hotness.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7callee1v() #0 !dbg !7 {
+  ret i32 1, !dbg !11
+}
+
+; Function Attrs: noinline nounwind uwtable use-sample-profile
+define dso_local i32 @_Z7callee2v() #1 !dbg !12 {
+  ret i32 2, !dbg !13
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller1v() #0 !dbg !14 {
+  %1 = call i32 @_Z7callee1v(), !dbg !15
+  ret i32 %1, !dbg !16
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller2v() #0 !dbg !17 {
+  %1 = call i32 @_Z7callee2v(), !dbg !18
+  ret i32 %1, !dbg !19
+}
+
+attributes #0 = { "use-sample-profile" }
+attributes #1 = { noinline nounwind uwtable "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "remarks-hotness.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 11.0.0"}
+!7 = distinct !DISubprogram(name: "callee1", linkageName: "_Z7callee1v", scope: !1, file: !1, line: 1, type: !8, scopeLine: 1, flags: DIFlagPrototyped, spFlags: 

[PATCH] D85810: [clang] Pass-through remarks options to lld

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 286124.
weiwang added a comment.
Herald added subscribers: dang, arichardson, emaste.
Herald added a reviewer: espindola.

update:

1. add `--plugin-opt` alias for remarks in lld
2. handle both lld and ld.gold pass-through
3. simplify target checking logic
4. code format update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/opt-record.c
  lld/ELF/Options.td

Index: lld/ELF/Options.td
===
--- lld/ELF/Options.td
+++ lld/ELF/Options.td
@@ -592,6 +592,21 @@
 def: J<"plugin-opt=obj-path=">,
   Alias,
   HelpText<"Alias for --lto-obj-path=">;
+def: J<"plugin-opt=opt-remarks-filename=">,
+  Alias,
+  HelpText<"Alias for --opt-remarks-filename">;
+def: J<"plugin-opt=opt-remarks-passes=">,
+  Alias,
+  HelpText<"Alias for --opt-remarks-passes">;
+def: J<"plugin-opt=opt-remarks-format=">,
+  Alias,
+  HelpText<"Alias for --opt-remarks-format">;
+def: F<"plugin-opt=opt-remarks-with-hotness">,
+  Alias,
+  HelpText<"Alias for --opt-remarks-with_hotness">;
+def: J<"plugin-opt=opt-remarks-hotness-threshold=">,
+  Alias,
+  HelpText<"Alias for --opt-remarks-hotness-threshold">;
 def: J<"plugin-opt=sample-profile=">,
   Alias, HelpText<"Alias for --lto-sample-profile">;
 def: F<"plugin-opt=save-temps">, Alias, HelpText<"Alias for --save-temps">;
Index: clang/test/Driver/opt-record.c
===
--- clang/test/Driver/opt-record.c
+++ clang/test/Driver/opt-record.c
@@ -18,7 +18,17 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format -fno-save-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-FOPT-DISABLE-FORMAT
-//
+
+// Test remarks options pass-through
+// No pass-through: lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s -check-prefix=CHECK-PASS
+
+// Pass-through:
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=gold -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-CUSTOM
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-PASS-RPASS
+
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
 
@@ -41,3 +51,17 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// CHECK-PASS: "--plugin-opt=opt-remarks-filename=FOO.opt.ld.yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-passes=inline"
+// CHECK-PASS: "--plugin-opt=opt-remarks-format=yaml"
+// CHECK-PASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-filename=FOO.txt.opt.ld.some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-format=some-format"
+// CHECK-PASS-CUSTOM: "--plugin-opt=opt-remarks-hotness-threshold=100"
+
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-missed=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=-pass-remarks-analysis=inline"
+// CHECK-PASS-RPASS: "--plugin-opt=opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,70 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ)) {
+CmdArgs.push_back(Args.MakeArgString(Twine("--plugin-opt=-pass-remarks=") +
+ A->getValue()));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ)) {
+

[PATCH] D85808: [remarks] Optimization remarks hotness filtering from profile summary

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

In D85808#2214272 , @tejohnson wrote:

> In D85808#2212297 , @weiwang wrote:
>
>> This is the 3rd of 3 dependent patches:
>>
>> 1. [lld] Enable remarks hotness filtering in lld: 
>> https://reviews.llvm.org/D85809
>> 2. [clang] Pass-through remarks options to lld: 
>> https://reviews.llvm.org/D85810
>> 3. [remarks] Optimization remarks hotness filtering from profile summary: 
>> https://reviews.llvm.org/D85808
>
> You can relate these as Parent/Child patches in Phabricator. See "Edit 
> Related Revisions" on the top right. It helps reviewers to see the 
> relationship and keep track of what patches are still open.
>
> Note these relationships get set up automatically when you upload a patch 
> with "Depends on Dx." in the description.

Thanks for the tip.




Comment at: lld/ELF/Config.h:280
+  // If threshold option is not specified, it is disabled by default.
+  llvm::Optional optRemarksHotnessThreshold = 0;
 

tejohnson wrote:
> Since this field is being added in patch D85809 as an unsigned, why not add 
> it as llvm::Optional<> there to start with instead of adding and then 
> changing?
> 
> Ditto for a number of other places in this patch.
Thanks for the feedback! It does seem awkward. When doing the splitting, I 
tried to make the 3 patches look more self-contained from each other. The 
`Optional` type change seems unrelated with the first patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D85810: [clang] Pass-through remarks options to lld

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 285205.
weiwang added a comment.

update test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/remarks-pass-through.c

Index: clang/test/Driver/remarks-pass-through.c
===
--- /dev/null
+++ clang/test/Driver/remarks-pass-through.c
@@ -0,0 +1,28 @@
+// This test verifies remarks options pass-through into linker(lld)
+
+// no pass-through if lto is disabled
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s
+
+// no pass-through if linker is not lld
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=gold -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s
+
+// pass-through cases
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-CUSTOM
+//
+// RUN: %clang -target x86_64-linux-gnu -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+//
+// CHECK: "--opt-remarks-filename" "FOO.opt.ld.yaml"
+// CHECK: "--opt-remarks-passes" "inline"
+// CHECK: "--opt-remarks-format" "yaml"
+// CHECK: "--opt-remarks-hotness-threshold=100"
+//
+// CHECK-CUSTOM: "--opt-remarks-filename" "FOO.txt.opt.ld.some-format"
+// CHECK-CUSTOM: "--opt-remarks-format" "some-format"
+// CHECK-CUSTOM: "--opt-remarks-hotness-threshold=100"
+//
+// CHECK-RPASS: "-mllvm" "-pass-remarks=inline"
+// CHECK-RPASS: "-mllvm" "-pass-remarks-missed=inline"
+// CHECK-RPASS: "-mllvm" "-pass-remarks-analysis=inline"
+// CHECK-RPASS: "--opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,95 @@
 using namespace clang;
 using namespace llvm::opt;
 
+// Remarks option pass-through only happens when
+// 1). single arch target
+// 2). linker is lld
+static bool checkRemarksOptions(StringRef LinkerPath, const ArgList ,
+const llvm::Triple ) {
+  bool hasMultipleArchs =
+  Triple.isOSDarwin() && Args.getAllArgValues(options::OPT_arch).size() > 1;
+
+  bool isLLD = llvm::sys::path::filename(LinkerPath) == "ld.lld" ||
+   llvm::sys::path::stem(LinkerPath) != "ld.lld";
+  if (hasMultipleArchs || !isLLD)
+return false;
+  return true;
+}
+
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks-missed=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks-analysis=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo ,
+ const InputInfo ) {
+  StringRef Format = "yaml";
+  if (const Arg *A = Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Format = A->getValue();
+
+  CmdArgs.push_back("--opt-remarks-filename");
+
+  SmallString<128> F;
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+F = A->getValue();
+  } else {
+if (Output.isFilename())
+  F = Output.getFilename();
+
+if (F.empty()) {
+  // Use the input filename.
+  F = llvm::sys::path::stem(Input.getBaseInput());
+}
+  }
+  // Append "opt.ld." to the end of the file name.
+  SmallString<32> Extension;
+  Extension += ".opt.ld.";
+  Extension += Format;
+
+  CmdArgs.push_back(Args.MakeArgString(F + Extension));
+
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_foptimization_record_passes_EQ)) {
+

[PATCH] D84246: [clang][LTO] Pass-through remarks options and set auto hotness threshold

[Wei Wang via Phabricator via cfe-commits]

weiwang abandoned this revision.
weiwang added a comment.

Diff has been split into 3 smaller ones:

1. [lld] Enable remarks hotness filtering in lld: 
https://reviews.llvm.org/D85809
2. [clang] Pass-through remarks options to lld: https://reviews.llvm.org/D85810
3. [remarks] Optimization remarks hotness filtering from profile summary: 
https://reviews.llvm.org/D85808


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84246

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

[PATCH] D85808: [remarks] Optimization remarks hotness filetering from profile summary

[Wei Wang via Phabricator via cfe-commits]

weiwang added reviewers: wenlei, hoyFB.
weiwang added a comment.

This is the the 3rd of 3 dependent patches:

1. [lld] Enable remarks hotness filtering in lld: 
https://reviews.llvm.org/D85809
2. [clang] Pass-through remarks options to lld: https://reviews.llvm.org/D85810
3. [remarks] Optimization remarks hotness filetering from profile summary: 
https://reviews.llvm.org/D85808


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D85810: [clang] Pass-through remarks options to lld

[Wei Wang via Phabricator via cfe-commits]

weiwang added reviewers: wenlei, hoyFB.
weiwang added a comment.

This is the the 2nd of 3 dependent patches:

1. [lld] Enable remarks hotness filtering in lld: 
https://reviews.llvm.org/D85809
2. [clang] Pass-through remarks options to lld: https://reviews.llvm.org/D85810
3. [remarks] Optimization remarks hotness filetering from profile summary: 
https://reviews.llvm.org/D85808


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D85810: [clang] Pass-through remarks options to lld

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.
weiwang requested review of this revision.

Propagate driver commandline remarks options to link. Pass-through happens when:

1. LTO is enabled;
2. Single arch target is specified;
3. The linker is lld;

This gives novice user a convenient way to collect and filter remarks throughout
a typical toolchain invocation with sample profile and LTO using single switch
from the clang driver.

A typical use of this option from clang command-line:

- Using -Rpass* options to print remarks to screen:

`clang -fuse-ld=lld -flto=thin -fprofile-sample-use=foo_sample.txt

  -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline
  -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=100 -o foo foo.cpp`

Remarks will be dumped to screen from both pre-lto and lto compilation.

- Using serialized remarks options

`clang -fuse-ld=lld -flto=thin -fprofile-sample-use=foo_sample.txt

  -fsave-optimization-record
  -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=100 -o foo foo.cpp`

This will produce multiple yaml files containing optimization remarks:

1. foo.opt.yaml : remarks from pre-lto
2. foo.opt.ld.yaml.thin.1.yaml: remark during lto


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85810

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/remarks-pass-through.c

Index: clang/test/Driver/remarks-pass-through.c
===
--- /dev/null
+++ clang/test/Driver/remarks-pass-through.c
@@ -0,0 +1,28 @@
+// This test verifies remarks options pass-through into linker(lld)
+
+// no pass-through if lto is disabled
+// RUN: %clang -### -o FOO -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s
+
+// no pass-through if linker is not lld
+// RUN: %clang -### -o FOO -fuse-ld=gold -fdiagnostics-hotness-threshold=100 -fsave-optimization-record %s 2>&1 | not FileCheck %s
+
+// pass-through cases
+// RUN: %clang -### -o FOO -fuse-ld=lld -flto -fdiagnostics-hotness-threshold=100 -fsave-optimization-record -foptimization-record-passes=inline %s 2>&1 | FileCheck %s
+
+// RUN: %clang -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -fsave-optimization-record=some-format -foptimization-record-file=FOO.txt %s 2>&1 | FileCheck %s -check-prefix=CHECK-CUSTOM
+//
+// RUN: %clang -### -o FOO -fuse-ld=lld -flto=thin -fdiagnostics-hotness-threshold=100 -Rpass=inline -Rpass-missed=inline -Rpass-analysis=inline %s 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+//
+// CHECK: "--opt-remarks-filename" "FOO.opt.ld.yaml"
+// CHECK: "--opt-remarks-passes" "inline"
+// CHECK: "--opt-remarks-format" "yaml"
+// CHECK: "--opt-remarks-hotness-threshold=100"
+//
+// CHECK-CUSTOM: "--opt-remarks-filename" "FOO.txt.opt.ld.some-format"
+// CHECK-CUSTOM: "--opt-remarks-format" "some-format"
+// CHECK-CUSTOM: "--opt-remarks-hotness-threshold=100"
+//
+// CHECK-RPASS: "-mllvm" "-pass-remarks=inline"
+// CHECK-RPASS: "-mllvm" "-pass-remarks-missed=inline"
+// CHECK-RPASS: "-mllvm" "-pass-remarks-analysis=inline"
+// CHECK-RPASS: "--opt-remarks-hotness-threshold=100"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -60,6 +60,95 @@
 using namespace clang;
 using namespace llvm::opt;
 
+// Remarks option pass-through only happens when
+// 1). single arch target
+// 2). linker is lld
+static bool checkRemarksOptions(StringRef LinkerPath, const ArgList ,
+const llvm::Triple ) {
+  bool hasMultipleArchs =
+  Triple.isOSDarwin() && Args.getAllArgValues(options::OPT_arch).size() > 1;
+
+  bool isLLD = llvm::sys::path::filename(LinkerPath) == "ld.lld" ||
+   llvm::sys::path::stem(LinkerPath) != "ld.lld";
+  if (hasMultipleArchs || !isLLD)
+return false;
+  return true;
+}
+
+static void renderRpassOptions(const ArgList , ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks-missed=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ)) {
+CmdArgs.push_back("-mllvm");
+std::string Passes = std::string("-pass-remarks-analysis=") + A->getValue();
+CmdArgs.push_back(Args.MakeArgString(Passes));
+  }
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+

[PATCH] D85808: [remarks] Optimization remarks hotness filetering from profile summary

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dang, dexonsmith, 
steven_wu, hiraditya, eraman, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: MaskRay.
Herald added projects: clang, LLVM.
weiwang requested review of this revision.

Expand remarks hotness threshold option `-fdiagnostics-hotness-threshold` in
clang driver command-line to utilize hotness threshold from profile summary.

Remarks hotness filtering relies on several driver options. Table below lists
how different options are correlated and affect final remarks outputs:

  | profile | hotness | threshold | remarks printed |
  |-|-|---|-|
  | No  | No  | No| All |
  | No  | No  | Yes   | None|
  | No  | Yes | No| All |
  | No  | Yes | Yes   | None|
  | Yes | No  | No| All |
  | Yes | No  | Yes   | None|
  | Yes | Yes | No| All |
  | Yes | Yes | Yes   | >=threshold |

In the presence of profile summary, it is often more desirable to directly use
the hotness threshold from profile summary. The new argument value
`-fdiagnostics-hotness-threshold=auto` indicates threshold will be synced with
hotness threshold from profile summary during compilation. The "auto" threshold
relies on the availability of profile summary. In case of missing such
information, no remarks will be generated.

In order to make the option consistent across various tools, the support for the
new 'auto' argument is also available in the following tools:

  lld:`-opt-remarks-with-hotness=auto`
  llvm-lto:   `-lto-pass-remarks-hotness-threshold=auto`
  opt/llc:`--pass-remarks-hotness-threshold=auto`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85808

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/Options.td
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/LLVMRemarkStreamer.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/LTO/Config.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Remarks/HotnessThresholdParser.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll
  llvm/tools/llc/llc.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -38,6 +38,7 @@
 #include "llvm/LinkAllIR.h"
 #include "llvm/LinkAllPasses.h"
 #include "llvm/MC/SubtargetFeature.h"
+#include "llvm/Remarks/HotnessThresholdParser.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
@@ -272,11 +273,13 @@
 cl::desc("With PGO, include profile count in optimization remarks"),
 cl::Hidden);
 
-static cl::opt
-RemarksHotnessThreshold("pass-remarks-hotness-threshold",
-cl::desc("Minimum profile count required for "
- "an optimization remark to be output"),
-cl::Hidden);
+static cl::opt, false, remarks::HotnessThresholdParser>
+RemarksHotnessThreshold(
+"pass-remarks-hotness-threshold",
+cl::desc("Minimum profile count required for "
+ "an optimization remark to be output. "
+ "Use 'auto' to apply the threshold from profile summary."),
+cl::value_desc("N or 'auto'"), cl::init(0), cl::Hidden);
 
 static cl::opt
 RemarksFilename("pass-remarks-output",
Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -37,6 +37,7 @@
 #include "llvm/InitializePasses.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Pass.h"
+#include "llvm/Remarks/HotnessThresholdParser.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
@@ -142,11 +143,13 @@
 cl::desc("With PGO, include profile count in optimization remarks"),
 cl::Hidden);
 
-static cl::opt
-RemarksHotnessThreshold("pass-remarks-hotness-threshold",
-cl::desc("Minimum profile 

[PATCH] D84246: [clang][LTO] Pass-through remarks options and set auto hotness threshold

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

still waiting for inputs. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84246

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

[PATCH] D84246: [clang][LTO] Pass-through remarks options and set auto hotness threshold

[Wei Wang via Phabricator via cfe-commits]

weiwang updated this revision to Diff 281077.
weiwang added a comment.

Fix msvc build failure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84246

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/remarks-pass-through.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/opt-remarks.ll
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/LLVMRemarkStreamer.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/LTO/Config.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Remarks/HotnessThresholdParser.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll
  llvm/tools/llc/llc.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -38,6 +38,7 @@
 #include "llvm/LinkAllIR.h"
 #include "llvm/LinkAllPasses.h"
 #include "llvm/MC/SubtargetFeature.h"
+#include "llvm/Remarks/HotnessThresholdParser.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
@@ -272,11 +273,13 @@
 cl::desc("With PGO, include profile count in optimization remarks"),
 cl::Hidden);
 
-static cl::opt
-RemarksHotnessThreshold("pass-remarks-hotness-threshold",
-cl::desc("Minimum profile count required for "
- "an optimization remark to be output"),
-cl::Hidden);
+static cl::opt, false, remarks::HotnessThresholdParser>
+RemarksHotnessThreshold(
+"pass-remarks-hotness-threshold",
+cl::desc("Minimum profile count required for "
+ "an optimization remark to be output. "
+ "Use 'auto' to apply the threshold from profile summary."),
+cl::value_desc("N or 'auto'"), cl::init(0), cl::Hidden);
 
 static cl::opt
 RemarksFilename("pass-remarks-output",
Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -37,6 +37,7 @@
 #include "llvm/InitializePasses.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Pass.h"
+#include "llvm/Remarks/HotnessThresholdParser.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
@@ -142,11 +143,13 @@
 cl::desc("With PGO, include profile count in optimization remarks"),
 cl::Hidden);
 
-static cl::opt
-RemarksHotnessThreshold("pass-remarks-hotness-threshold",
-cl::desc("Minimum profile count required for "
- "an optimization remark to be output"),
-cl::Hidden);
+static cl::opt, false, remarks::HotnessThresholdParser>
+RemarksHotnessThreshold(
+"pass-remarks-hotness-threshold",
+cl::desc("Minimum profile count required for "
+ "an optimization remark to be output. "
+ "Use 'auto' to apply the threshold from profile summary."),
+cl::value_desc("N or 'auto'"), cl::init(0), cl::Hidden);
 
 static cl::opt
 RemarksFilename("pass-remarks-output",
Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- /dev/null
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -0,0 +1,96 @@
+;; This test verifies 'auto' hotness threshold when profile file is provided.
+;;
+;; new PM
+; RUN: rm -f %t.yaml %t.hot.yaml
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.yaml \
+; RUN: -pass-remarks-with-hotness --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.yaml
+; RUN: FileCheck %s -check-prefix=YAML-MISS < %t.yaml
+
+;; test 'auto' threshold
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: 

[PATCH] D84246: [clang][LTO] Pass-through remarks options and set auto hotness threshold

[Wei Wang via Phabricator via cfe-commits]

weiwang added a comment.

I put everything together in a single diff so that it is easier to get the 
whole idea. The change itself is too big to go in as a single diff, and I'd 
like to get inputs on how to split it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84246



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

[PATCH] D84246: [clang][LTO] Pass-through remarks options and set auto hotness threshold

[Wei Wang via Phabricator via cfe-commits]

weiwang created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dang, dexonsmith, 
steven_wu, MaskRay, hiraditya, eraman, arichardson, inglorion, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: MaskRay.
Herald added projects: clang, LLVM.

Expand remarks hotness threshold option -fdiagnostics-hotness-threshold in clang
driver command-line to both filtering remarks by hotness threshold from profile
summary and automatic remarks options pass-through to linker.

Remarks hotness filtering relies on several driver options. Table below lists
how different options are correlated and affect final remarks outputs:

| profile | hotness | threshold | remarks printed |
| --- | --- | - | --- |
| No  | No  | No| All |
| No  | No  | Yes   | None|
| No  | Yes | No| All |
| No  | Yes | Yes   | None|
| Yes | No  | No| All |
| Yes | No  | Yes   | None|
| Yes | Yes | No| All |
| Yes | Yes | Yes   | >=threshold |
|

The new argument value -fdiagnostics-hotness-threshold=auto indicates threshold
will be synced with hotness threshold from profile summary during compilation.
In addition, when the following conditions are met, remarks related options are
passed into linker as well:

1. LTO is enabled;
2. Single arch target is specified;
3. The linker is lld;

The "auto" threshold relies on the availability of profile summary. In case of
missing such information, no remarks will be generated.

This gives novice user a convenient way to collect and filter remarks throughout
a typical toolchain invocation with sample profile and LTO using single switch
from the clang driver.

A typical use of this option from clang command-line:

- Using -Rpass* options to print remarks to screen:

clang -fuse-ld=lld -flto=thin

  -fprofile-sample-use=foo_sample.txt
  -Rpass=inline
  -Rpass-missed=inline -Rpass-analysis=inline
  -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=auto
  -o foo foo.cpp

Remarks will be dumped to screen from both pre-lto and lto compilation.

- Using serialized remarks options:

clang -fuse-ld=lld -flto=thin

  -fprofile-sample-use=foo_sample.txt -fsave-optimization-record
  -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=auto
  -o foo foo.cpp

This will produce multiple yaml files containing optimization remarks:

- foo.opt.yaml : remarks from pre-lto
- foo.opt.ld.yaml.thin.1.yaml: remark during lto

Both types of options can be used together.

Given its restricted use cases, this shouldn't be viewed as a complete
replacement of current remarks option handling for the linker.

In order to make the option consistent across various tools, the support for the
new 'auto' argument is also available in the following tools:

- lld:  -opt-remarks-with-hotness=auto
- llvm-lto: -lto-pass-remarks-hotness-threshold=auto
- opt/llc:  --pass-remarks-hotness-threshold=auto


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84246

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/remarks-pass-through.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/opt-remarks.ll
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/LLVMRemarkStreamer.h
  llvm/include/llvm/IR/Module.h
  llvm/include/llvm/LTO/Config.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Remarks/HotnessThresholdParser.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll
  llvm/tools/llc/llc.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -40,6 +40,7 @@
 #include "llvm/LinkAllIR.h"
 #include "llvm/LinkAllPasses.h"
 #include "llvm/MC/SubtargetFeature.h"
+#include "llvm/Remarks/HotnessThresholdParser.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
@@ -274,11 +275,13