[PATCH] D84886: Create LoopNestPass

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added a comment.

Examples of existing transformations that can be benefited from LoopNestPass:  
LoopInterchange, LoopUnrollAndJam, LoopFusion, LoopIdiom, LoopDistribution, ...
And their corresponding cost model analyses can be benefited from 
LoopNestAnalysis, e.g. LoopCacheCost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84886

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4702
+
+  // Add or replace -ftime-trace` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {

dongjunduo wrote:
> Whitney wrote:
> > Do you mean Add or replace the modified `-ftime-trace=` to all clang 
> > jobs?
> Right
ic, can you please have the comment updated?



Comment at: clang/lib/Driver/Driver.cpp:4739
+
+  // replace `-ftime-trace=`
+  auto &JArgs = J.getArguments();

dongjunduo wrote:
> Whitney wrote:
> > should we also replace `-ftime-trace`?
> The work before here is to infer the correct path to store the time-trace 
> file.
> 
> After that, the  in `-ftime-trace=` should be replaced by the 
> infered correct path.
> 
> We do not need to replace `-ftime-trace` then.
What happens when `-ftime-trace` is given by the user? Do you have both 
`-ftime-trace=` and `-ftime-trace` as arguments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4702
+
+  // Add or replace -ftime-trace` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {

dongjunduo wrote:
> Whitney wrote:
> > dongjunduo wrote:
> > > Whitney wrote:
> > > > Do you mean Add or replace the modified `-ftime-trace=` to all 
> > > > clang jobs?
> > > Right
> > ic, can you please have the comment updated?
> Done at line:4703
Let me clarify...can you please add `=` in between `-ftime-trace` and `` 
for the comment on line 4703?



Comment at: clang/lib/Driver/Driver.cpp:4739
+
+  // replace `-ftime-trace=`
+  auto &JArgs = J.getArguments();

dongjunduo wrote:
> Whitney wrote:
> > dongjunduo wrote:
> > > Whitney wrote:
> > > > should we also replace `-ftime-trace`?
> > > The work before here is to infer the correct path to store the time-trace 
> > > file.
> > > 
> > > After that, the  in `-ftime-trace=` should be replaced by the 
> > > infered correct path.
> > > 
> > > We do not need to replace `-ftime-trace` then.
> > What happens when `-ftime-trace` is given by the user? Do you have both 
> > `-ftime-trace=` and `-ftime-trace` as arguments?
> It doesn't matter that either "-ftime-trace" or "-ftime-trace=" or both 
> of them are given by the user.
> 
> The TimeProfiler is switched on when either "-ftime-trace" and 
> "-ftime-trace="  is specified. Then, 
> 
> * If "-ftime-trace=" is specified, the driver use .
> * If only "-ftime-trace" is specified, the  can be infered, then a new 
> option "-ftime-trace=" may be added to the clang.
In the case of only "-ftime-trace" is specified, isn't it better to only pass  
"-ftime-trace=" to the compile step instead of both  
"-ftime-trace=" and "-ftime-trace"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/test/Driver/check-time-trace.cpp:9
+// RUN:   | FileCheck %s
+// RUN: %clangxx -S -ftime-trace=%T -ftime-trace-granularity=0 -o 
%T/check-time-trace %s
+// RUN: cat %T/check-time-trace.json \

By default, the JSON file is stored in %T, as it is the directory for the 
output object file. Can you make a different directory, to ensure it is not 
just the default behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D128048: Add a new clang option "-ftime-trace="

[Whitney Tsang via Phabricator via cfe-commits]

Whitney accepted this revision.
Whitney added inline comments.



Comment at: clang/tools/driver/cc1_main.cpp:265
+  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+  if (llvm::sys::fs::is_directory(TracePath)) {
+llvm::sys::path::append(TracePath, FileName);

No need braces for single instruction block. 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4739
 
+  // set data storing path of the options `-ftime-trace`, `-ftime-trace=`
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;

It would be cleaner if all these logic can be moved to a separate function with 
a description.



Comment at: clang/lib/Driver/Driver.cpp:4753
+else
+  LinkingResParentPath = ".";
+  }

Do you expect link only show up once? If so, you can add a break here.



Comment at: clang/tools/driver/cc1_main.cpp:260
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
 if (auto profilerOutput = Clang->createOutputFile(
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,

Should we assert that TimeTracePath is not empty?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4786
+const std::string::size_type size = arg.size();
+char *buffer = new char[size + 1];
+memcpy(buffer, arg.c_str(), size + 1);

Assert size not 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D106815: Update: clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/test/Profile/gcc-flag-compatibility.c:10
 
+// On AIX, -flto is required with -fprofile-generate
+// XFAIL: aix

Add in the comment that gcc-flag-compatibility-aix.c is used to do the testing 
on AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106815

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

[PATCH] D84886: Create LoopNestPass

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added a comment.

> I was trying to convert the LoopInterchange pass into a loop-nest pass. 
> However, there seems to be no corresponding loop pass for the NPM.
> Any particular reason for this?

That's true, LoopInterchange pass is not already ported to the NPM, likely 
because no one needed it in the NPM. Maybe you want to convert another pass to 
loop-nest pass as the first loop-nest pass, or you will have to port it to NPM 
first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84886

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

[PATCH] D104803: [AIX] Emitting diagnostics error for profile options

[Whitney Tsang via Phabricator via cfe-commits]

Whitney created this revision.
Whitney added reviewers: hubert.reinterpretcast, etiotto, bmahjour, kbarton, 
daltenty.
Whitney added a project: LLVM.
Whitney requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Only LLVM-based instrumentation profile is supported on AIX.
And it currently must be used with full LTO.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104803

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/unsupported-option.c


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,7 +1,20 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
+// CHECK: error: unsupported option '--hedonism'
+
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-
-// CHECK: error: unsupported option '--hedonism'
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+
+// RUN: not %clang -fprofile-instr-generate -fprofile-sample-use=code.prof 
--target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
+// INVALID-AIX-PROFILE-DAG: error: unsupported option 
'-fprofile-instr-generate' for target
+// INVALID-AIX-PROFILE-DAG: error: unsupported option '-fprofile-sample-use=' 
for target
+
+// RUN: not %clang -fprofile-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-LTO
+// AIX-PROFILE-LTO: error: invalid argument '-fprofile-generate' only allowed 
with '-flto'
+
+// RUN: not %clang -fprofile-generate -flto=thin --target=powerpc-ibm-aix %s 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-THINLTO
+// AIX-PROFILE-THINLTO: error: invalid argument '-fprofile-generate' only 
allowed with '-flto'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -776,6 +776,21 @@
   options::OPT_fprofile_instr_generate,
   options::OPT_fprofile_instr_generate_EQ,
   options::OPT_fno_profile_instr_generate);
+
+  if (TC.getTriple().isOSAIX()) {
+if (PGOGenerateArg)
+  if (!D.isUsingLTO(false /*IsDeviceOffloadAction */) ||
+  D.getLTOMode() != LTOK_Full)
+D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< PGOGenerateArg->getSpelling() << "-flto";
+if (ProfileGenerateArg)
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileGenerateArg->getSpelling() << TC.getTriple().str();
+if (Arg *ProfileSampleUseArg = getLastProfileSampleUseArg(Args))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileSampleUseArg->getSpelling() << TC.getTriple().str();
+  } 
+
   if (ProfileGenerateArg &&
   ProfileGenerateArg->getOption().matches(
   options::OPT_fno_profile_instr_generate))


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,7 +1,20 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
+// CHECK: error: unsupported option '--hedonism'
+
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-
-// CHECK: error: unsupported option '--hedonism'
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+
+// RUN: not %clang -fprofile-instr-generate -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
+// INVALID-AIX-PROFILE-DAG: error: unsupported option '-fprofile-instr-generate' for target
+// INVALID-AIX-PROFILE-DAG: error: unsupported option '-fprofile-sample-use=' for target
+
+// RUN: not %clang -fprofile-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-LTO
+// AIX-PROFILE-LTO: error: invalid argument '-fprofile-generate' only allowed with '-flto'
+
+// RUN: not %clang -fprofile-generate -flto=thin --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-THINLTO
+// AIX-PROFILE-THINLTO: error: invalid argument '-fprofile-generate' only allowed with '-flto'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -776,6 +776,21 @@
   options::OPT_fprofile_instr_generate,
   options::OPT_fprofile_instr_generate_EQ,
   options::OPT_fno_profile_instr_generate);
+
+  if (TC.getTriple().isOSAIX()) {
+if (PGOGenerateArg)
+  if (!D.isUsingLTO(false /*IsDeviceOffloadAction */) ||
+  D.getLTOMode() != LTOK_Full)
+D.Diag(clang::d

[PATCH] D104803: [AIX] Emitting diagnostics error for profile options

[Whitney Tsang via Phabricator via cfe-commits]

Whitney updated this revision to Diff 354096.
Whitney marked an inline comment as done.
Whitney added a comment.

Updated one of the test cases to use `--target=powerpc64-ibm-aix`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104803

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/unsupported-option.c


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,7 +1,20 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
+// CHECK: error: unsupported option '--hedonism'
+
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-
-// CHECK: error: unsupported option '--hedonism'
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+
+// RUN: not %clang -fprofile-instr-generate -fprofile-sample-use=code.prof 
--target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
+// INVALID-AIX-PROFILE-DAG: error: unsupported option 
'-fprofile-instr-generate' for target
+// INVALID-AIX-PROFILE-DAG: error: unsupported option '-fprofile-sample-use=' 
for target
+
+// RUN: not %clang -fprofile-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-LTO
+// AIX-PROFILE-LTO: error: invalid argument '-fprofile-generate' only allowed 
with '-flto'
+
+// RUN: not %clang -fprofile-generate -flto=thin --target=powerpc64-ibm-aix %s 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-THINLTO
+// AIX-PROFILE-THINLTO: error: invalid argument '-fprofile-generate' only 
allowed with '-flto'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -776,6 +776,21 @@
   options::OPT_fprofile_instr_generate,
   options::OPT_fprofile_instr_generate_EQ,
   options::OPT_fno_profile_instr_generate);
+
+  if (TC.getTriple().isOSAIX()) {
+if (PGOGenerateArg)
+  if (!D.isUsingLTO(false /*IsDeviceOffloadAction */) ||
+  D.getLTOMode() != LTOK_Full)
+D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< PGOGenerateArg->getSpelling() << "-flto";
+if (ProfileGenerateArg)
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileGenerateArg->getSpelling() << TC.getTriple().str();
+if (Arg *ProfileSampleUseArg = getLastProfileSampleUseArg(Args))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileSampleUseArg->getSpelling() << TC.getTriple().str();
+  } 
+
   if (ProfileGenerateArg &&
   ProfileGenerateArg->getOption().matches(
   options::OPT_fno_profile_instr_generate))


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,7 +1,20 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
+// CHECK: error: unsupported option '--hedonism'
+
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-
-// CHECK: error: unsupported option '--hedonism'
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+
+// RUN: not %clang -fprofile-instr-generate -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
+// INVALID-AIX-PROFILE-DAG: error: unsupported option '-fprofile-instr-generate' for target
+// INVALID-AIX-PROFILE-DAG: error: unsupported option '-fprofile-sample-use=' for target
+
+// RUN: not %clang -fprofile-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-LTO
+// AIX-PROFILE-LTO: error: invalid argument '-fprofile-generate' only allowed with '-flto'
+
+// RUN: not %clang -fprofile-generate -flto=thin --target=powerpc64-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-THINLTO
+// AIX-PROFILE-THINLTO: error: invalid argument '-fprofile-generate' only allowed with '-flto'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -776,6 +776,21 @@
   options::OPT_fprofile_instr_generate,
   options::OPT_fprofile_instr_generate_EQ,
   options::OPT_fno_profile_instr_generate);
+
+  if (TC.getTriple().isOSAIX()) {
+if (PGOGenerateArg)
+  if (!D.isUsingLTO(false /*IsDeviceOffloadAction */) ||
+  D.getLTOMode() != LTOK_Full)
+D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< PGOGenerateArg->getSpelling() << "-flto";
+if (ProfileGene

[PATCH] D104803: [AIX] Emitting diagnostics error for profile options

[Whitney Tsang via Phabricator via cfe-commits]

Whitney updated this revision to Diff 354108.
Whitney added a comment.

move code later


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104803

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/unsupported-option.c


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,7 +1,23 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
+// CHECK: error: unsupported option '--hedonism'
+
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-
-// CHECK: error: unsupported option '--hedonism'
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+
+// RUN: not %clang -fprofile-instr-generate --target=powerpc-ibm-aix %s 2>&1 | 
\
+// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
+// INVALID-AIX-PROFILE: error: unsupported option '-fprofile-instr-generate' 
for target
+
+// RUN: not %clang -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-SAMPLE
+// AIX-PROFILE-SAMPLE: error: unsupported option '-fprofile-sample-use=' for 
target
+
+// RUN: not %clang -fprofile-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-LTO
+// AIX-PROFILE-LTO: error: invalid argument '-fprofile-generate' only allowed 
with '-flto'
+
+// RUN: not %clang -fprofile-generate -flto=thin --target=powerpc64-ibm-aix %s 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-THINLTO
+// AIX-PROFILE-THINLTO: error: invalid argument '-fprofile-generate' only 
allowed with '-flto'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -801,6 +801,20 @@
 PGOGenerateArg = nullptr;
   }
 
+  if (TC.getTriple().isOSAIX()) {
+if (PGOGenerateArg)
+  if (!D.isUsingLTO(false /*IsDeviceOffloadAction */) ||
+  D.getLTOMode() != LTOK_Full)
+D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< PGOGenerateArg->getSpelling() << "-flto";
+if (ProfileGenerateArg)
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileGenerateArg->getSpelling() << TC.getTriple().str();
+if (Arg *ProfileSampleUseArg = getLastProfileSampleUseArg(Args))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileSampleUseArg->getSpelling() << TC.getTriple().str();
+  }
+
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(
 options::OPT_fprofile_instr_generate_EQ))


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,7 +1,23 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
+// CHECK: error: unsupported option '--hedonism'
+
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-
-// CHECK: error: unsupported option '--hedonism'
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+
+// RUN: not %clang -fprofile-instr-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
+// INVALID-AIX-PROFILE: error: unsupported option '-fprofile-instr-generate' for target
+
+// RUN: not %clang -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-SAMPLE
+// AIX-PROFILE-SAMPLE: error: unsupported option '-fprofile-sample-use=' for target
+
+// RUN: not %clang -fprofile-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-LTO
+// AIX-PROFILE-LTO: error: invalid argument '-fprofile-generate' only allowed with '-flto'
+
+// RUN: not %clang -fprofile-generate -flto=thin --target=powerpc64-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-THINLTO
+// AIX-PROFILE-THINLTO: error: invalid argument '-fprofile-generate' only allowed with '-flto'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -801,6 +801,20 @@
 PGOGenerateArg = nullptr;
   }
 
+  if (TC.getTriple().isOSAIX()) {
+if (PGOGenerateArg)
+  if (!D.isUsingLTO(false /*IsDeviceOffloadAction */) ||
+  D.getLTOMode() != LTOK_Full)
+D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< PGOGenerateArg->getSpelling() << "-flto";
+if (ProfileGenerateArg)
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileGen

[PATCH] D104803: [AIX] Emitting diagnostics error for profile options

[Whitney Tsang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGab244db1fa0b: [AIX] Emitting diagnostics error for profile 
options (authored by Whitney).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104803

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/unsupported-option.c


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,7 +1,23 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
+// CHECK: error: unsupported option '--hedonism'
+
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-
-// CHECK: error: unsupported option '--hedonism'
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+
+// RUN: not %clang -fprofile-instr-generate --target=powerpc-ibm-aix %s 2>&1 | 
\
+// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
+// INVALID-AIX-PROFILE: error: unsupported option '-fprofile-instr-generate' 
for target
+
+// RUN: not %clang -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-SAMPLE
+// AIX-PROFILE-SAMPLE: error: unsupported option '-fprofile-sample-use=' for 
target
+
+// RUN: not %clang -fprofile-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-LTO
+// AIX-PROFILE-LTO: error: invalid argument '-fprofile-generate' only allowed 
with '-flto'
+
+// RUN: not %clang -fprofile-generate -flto=thin --target=powerpc64-ibm-aix %s 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-THINLTO
+// AIX-PROFILE-THINLTO: error: invalid argument '-fprofile-generate' only 
allowed with '-flto'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -801,6 +801,20 @@
 PGOGenerateArg = nullptr;
   }
 
+  if (TC.getTriple().isOSAIX()) {
+if (PGOGenerateArg)
+  if (!D.isUsingLTO(false /*IsDeviceOffloadAction */) ||
+  D.getLTOMode() != LTOK_Full)
+D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< PGOGenerateArg->getSpelling() << "-flto";
+if (ProfileGenerateArg)
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileGenerateArg->getSpelling() << TC.getTriple().str();
+if (Arg *ProfileSampleUseArg = getLastProfileSampleUseArg(Args))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileSampleUseArg->getSpelling() << TC.getTriple().str();
+  }
+
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(
 options::OPT_fprofile_instr_generate_EQ))


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,7 +1,23 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
+// CHECK: error: unsupported option '--hedonism'
+
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-
-// CHECK: error: unsupported option '--hedonism'
 // DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+
+// RUN: not %clang -fprofile-instr-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=INVALID-AIX-PROFILE
+// INVALID-AIX-PROFILE: error: unsupported option '-fprofile-instr-generate' for target
+
+// RUN: not %clang -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-SAMPLE
+// AIX-PROFILE-SAMPLE: error: unsupported option '-fprofile-sample-use=' for target
+
+// RUN: not %clang -fprofile-generate --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-LTO
+// AIX-PROFILE-LTO: error: invalid argument '-fprofile-generate' only allowed with '-flto'
+
+// RUN: not %clang -fprofile-generate -flto=thin --target=powerpc64-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-THINLTO
+// AIX-PROFILE-THINLTO: error: invalid argument '-fprofile-generate' only allowed with '-flto'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -801,6 +801,20 @@
 PGOGenerateArg = nullptr;
   }
 
+  if (TC.getTriple().isOSAIX()) {
+if (PGOGenerateArg)
+  if (!D.isUsingLTO(false /*IsDeviceOffloadAction */) ||
+  D.getLTOMode() != LTOK_Full)
+D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< PGOGenerateArg->getSpelling() << "-flto";
+if (Profi

[PATCH] D106815: Update: clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added a comment.

In D106815#2926363 , @madanial wrote:

> I need some help to commit this change as I do not have commit access as of 
> now.

I will commit it for you today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106815

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

[PATCH] D106815: Update: clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX

[Whitney Tsang 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 rG39ca3e5541d0: Update: 
clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX (authored by 
madanial, committed by Whitney).

Changed prior to commit:
  https://reviews.llvm.org/D106815?vs=362829&id=365185#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106815

Files:
  clang/test/Profile/gcc-flag-compatibility-aix.c
  clang/test/Profile/gcc-flag-compatibility.c


Index: clang/test/Profile/gcc-flag-compatibility.c
===
--- clang/test/Profile/gcc-flag-compatibility.c
+++ clang/test/Profile/gcc-flag-compatibility.c
@@ -7,6 +7,8 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
+// On AIX, -flto is required with -fprofile-generate. 
gcc-flag-compatibility-aix.c is used to do the testing on AIX with -flto
+// XFAIL: aix
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
 // PROFILE-GEN: @__profc_main = {{(private|internal)}} global [2 x i64] 
zeroinitializer, section
Index: clang/test/Profile/gcc-flag-compatibility-aix.c
===
--- clang/test/Profile/gcc-flag-compatibility-aix.c
+++ clang/test/Profile/gcc-flag-compatibility-aix.c
@@ -7,14 +7,16 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
+// On AIX, -flto is required with -fprofile-generate
+
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate -fno-experimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-GEN %s
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate -fexperimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-GEN %s
 // PROFILE-GEN: @__profc_main = {{(private|internal)}} global [2 x i64] 
zeroinitializer, section
 // PROFILE-GEN: @__profd_main =
 
 // Check that -fprofile-generate=/path/to generates /path/to/default.profraw
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fno-experimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
-// RxUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fexperimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate=/path/to -fno-experimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
+// RxUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate=/path/to -fexperimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
 // PROFILE-GEN-EQ: constant [{{.*}} x i8] c"/path/to{{/|}}{{.*}}\00"
 
 // Check that -fprofile-use=some/path reads some/path/default.profdata


Index: clang/test/Profile/gcc-flag-compatibility.c
===
--- clang/test/Profile/gcc-flag-compatibility.c
+++ clang/test/Profile/gcc-flag-compatibility.c
@@ -7,6 +7,8 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
+// On AIX, -flto is required with -fprofile-generate. gcc-flag-compatibility-aix.c is used to do the testing on AIX with -flto
+// XFAIL: aix
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate -fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate -fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
 // PROFILE-GEN: @__profc_main = {{(private|internal)}} global [2 x i64] zeroinitializer, section
Index: clang/test/Profile/gcc-flag-compatibility-aix.c
===
--- clang/test/Profile/gcc-flag-compatibility-aix.c
+++ clang/test/Profile/gcc-flag-compatibility-aix.c
@@ -7,14 +7,16 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate -fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
-// RUN: %clang %s -c -S -

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4786
+const std::string::size_type size = arg.size();
+char *buffer = new char[size + 1];
+memcpy(buffer, arg.c_str(), size + 1);

dongjunduo wrote:
> Whitney wrote:
> > Assert size not 0.
> The size of `arg` maynot be 0 because of its basic initialization in L:4690
> ```
> std::string arg = std::string("-ftime-trace=");
> ```
How about assert that it starts with `-ftime-trace=` and not ends with `=`.



Comment at: clang/tools/driver/cc1_main.cpp:260
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+if (TracePath.empty()) {
+  llvm::errs() << "-ftime-trace= is empty!\n";

As it is unexpected, it should be an assert: 
`assert(!TracePath.empty() && "...");`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/tools/driver/cc1_main.cpp:260
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+assert(!TracePath.empty());
 if (auto profilerOutput = Clang->createOutputFile(

According to llvm coding standards:
To further assist with debugging, make sure to put some kind of error message 
in the assertion statement (which is printed if the assertion is tripped).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4663
+void InferTimeTracePath(Compilation &C) {
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile =

Usually bool variable has name that starts with `Has` or something similar, 
e.g., `HasTimeTraceFile`.



Comment at: clang/lib/Driver/Driver.cpp:4702
+
+  // Add or replace -ftime-trace` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {

Do you mean Add or replace the modified `-ftime-trace=` to all clang jobs?



Comment at: clang/lib/Driver/Driver.cpp:4739
+
+  // replace `-ftime-trace=`
+  auto &JArgs = J.getArguments();

should we also replace `-ftime-trace`?



Comment at: clang/test/Driver/check-time-trace.cpp:4
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o 
%T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \

what may be between `check-time-trace` and `.json`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added a comment.

Can you please add some test cases?




Comment at: clang/tools/driver/cc1_main.cpp:257
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
+SmallString<128> Path(Clang->getFrontendOpts().TimeTracePath);
+
Path.append(llvm::sys::path::filename(Clang->getFrontendOpts().OutputFile));

What happens when TimeTracePath is not given? Ideally the originally behavior 
is not changed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2832
+def ftime_trace_path : Joined<["-"], "ftime-trace-path=">, Group,
+  HelpText<"Path which stores the output files of time profiler">,
+  Flags<[CC1Option, CoreOption]>,

jamieschmeiser wrote:
> which specifies the output files for -ftime-trace
What do you think of "which specifies the output directory for -ftime-trace"? 
As from your example, this option doesn't specify a file, instead it specifies 
a directory.



Comment at: clang/include/clang/Frontend/FrontendOptions.h:502
 
+  /// Path which stores the output files of time profiler.
+  std::string TimeTracePath;

Please update this comment according too. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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

[PATCH] D128048: Add a new clang option "-ftime-trace-path"

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added a comment.

In D128048#3601579 , @jamieschmeiser 
wrote:

> Can you please use git rebase -i to collapse all the changes into a single 
> change?  If this isn't done, it is difficult to know what is being reviewed 
> as the changes only show the differences since your last revision, not all of 
> the changes.

I can see all the changes, make sure your link is not suffix with "new", or 
check the history tab to see what is it comparing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

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