Re: [PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-07-01 Thread Rong Xu via cfe-commits 
On Fri, Jun 28, 2019 at 5:28 PM Xinliang David Li 
wrote:

> I agree that the  test coverage needs to be improved eg better check etc.
>
PGO is rarely run under -O0.  But I agree that we should improve the test.

I sent out https://reviews.llvm.org/D64029. Chanlder: could you take a look?

Thanks,

-Rong


>
> David
>
>
> On Fri, Jun 28, 2019 at 5:21 PM Chandler Carruth via Phabricator via
> llvm-commits  wrote:
>
>> chandlerc added a comment.
>>
>> In D63155#1563275 , @xur wrote:
>>
>> > In D63155#1563266 ,
>> @chandlerc wrote:
>> >
>> > > I just think we also need to understand why *no other test failed*,
>> and why the only test we have for doing PGO at O0 is this one and it could
>> be "fixed" but such a deeply unrelated change
>> >
>> >
>> > We have special code to do PGO at O0 in old pass manager. This is not
>> done in the new pass manager.
>>
>>
>> I'm not sure how this addresses my question about test coverage.
>>
>>
>> Repository:
>>   rL LLVM
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D63155/new/
>>
>> https://reviews.llvm.org/D63155
>>
>>
>>
>> ___
>> llvm-commits mailing list
>> llvm-comm...@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Xinliang David Li via cfe-commits 
I agree that the  test coverage needs to be improved eg better check etc.

David


On Fri, Jun 28, 2019 at 5:21 PM Chandler Carruth via Phabricator via
llvm-commits  wrote:

> chandlerc added a comment.
>
> In D63155#1563275 , @xur wrote:
>
> > In D63155#1563266 , @chandlerc
> wrote:
> >
> > > I just think we also need to understand why *no other test failed*,
> and why the only test we have for doing PGO at O0 is this one and it could
> be "fixed" but such a deeply unrelated change
> >
> >
> > We have special code to do PGO at O0 in old pass manager. This is not
> done in the new pass manager.
>
>
> I'm not sure how this addresses my question about test coverage.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D63155/new/
>
> https://reviews.llvm.org/D63155
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Chandler Carruth via Phabricator via cfe-commits 
chandlerc added a comment.

In D63155#1563275 , @xur wrote:

> In D63155#1563266 , @chandlerc wrote:
>
> > I just think we also need to understand why *no other test failed*, and why 
> > the only test we have for doing PGO at O0 is this one and it could be 
> > "fixed" but such a deeply unrelated change
>
>
> We have special code to do PGO at O0 in old pass manager. This is not done in 
> the new pass manager.


I'm not sure how this addresses my question about test coverage.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Leonard Chan via Phabricator via cfe-commits 
leonardchan added a comment.

Reverted in r364692


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Rong Xu via Phabricator via cfe-commits 
xur added a comment.

In D63155#1563266 , @chandlerc wrote:

> In D63155#1563240 , @leonardchan 
> wrote:
>
> > In D63155#1563229 , @xur wrote:
> >
> > > This patch does not make sense to me.
> > >
> > > The reason of failing with -fexperimental-new-pass-manager is because we 
> > > don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> > > instrumentation/use passes are under 
> > > PassBuilder::buildPerModuleDefaultPipeline.
> > >
> > > This patch add a pass
> > >
> > >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> > >
> > > which only gives you the wrong signal  of instrumentation is done.
> > >
> > > I wrote pass PGOInstrumentationGenCreateVar only for some trick 
> > > interactions for thinlto under ldd for CSPGO.
> > >  Regular FDO should not use it.
> > >
> > > The right fix is to enable PGO instrumentation and use in pass builder 
> > > for O0.
> > >
> > > I would like to request to revert this patch.
> >
> >
> > As an alternative, could I instead request that we remove the BackendUtil 
> > changes and just mark the runs in gcc-flag-compatibility.c with 
> > `-fno-experimental-new-pass-manager`. This way we could clarify that under 
> > the new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as 
> > is.
>
>
> No, I think we should be doing PGO at O0 in the new PM if we do so in the old 
> PM.
>
> I think Rong is saying that the *way* you're enabling PGO at O0 isn't correct 
> to fix this test case. That seems plausible to me at least, and I think 
> reverting and figuring out what the right way to do it is a fine approach.


Just check the IR, we still not doing instrumentation at O0. This patch just 
mask the error.

> I just think we also need to understand why *no other test failed*, and why 
> the only test we have for doing PGO at O0 is this one and it could be "fixed" 
> but such a deeply unrelated change

We have special code to do PGO at O0 in old pass manager. This is not done in 
the new pass manager.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Rong Xu via Phabricator via cfe-commits 
xur added a comment.

they are not doing the exacly the same thing for old pass manager and new pass 
manger: old pass manger is doing instrumentation, while the new pass manager 
with this change is NOT.
the test is not check instrumentation, (it only check the variables that 
generates by InstroProfiling pass). 
In this sense, the test is not well written.

I can draft a patch for this.

In D63155#1563239 , @chandlerc wrote:

> In D63155#1563229 , @xur wrote:
>
> > This patch does not make sense to me.
> >
> > The reason of failing with -fexperimental-new-pass-manager is because we 
> > don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> > instrumentation/use passes are under 
> > PassBuilder::buildPerModuleDefaultPipeline.
> >
> > This patch add a pass
> >
> >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> >
> > which only gives you the wrong signal  of instrumentation is done.
> >
> > I wrote pass PGOInstrumentationGenCreateVar only for some trick 
> > interactions for thinlto under ldd for CSPGO.
> >  Regular FDO should not use it.
> >
> > The right fix is to enable PGO instrumentation and use in pass builder for 
> > O0.
> >
> > I would like to request to revert this patch.
>
>
> I mean, I'm happy for the patch to be reverted, but I still really don't 
> understand why this fixes the test to work *exactly* the same as w/ the old 
> pass manager and doesn't break any other tests if it is completely wrong? It 
> seems like there must be something strange with the test coverage if this is 
> so far off of correct without any failures...
>
> I also don't understand what fix you are suggesting instead, but maybe you 
> can show a patch?




In D63155#1563266 , @chandlerc wrote:

> In D63155#1563240 , @leonardchan 
> wrote:
>
> > In D63155#1563229 , @xur wrote:
> >
> > > This patch does not make sense to me.
> > >
> > > The reason of failing with -fexperimental-new-pass-manager is because we 
> > > don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> > > instrumentation/use passes are under 
> > > PassBuilder::buildPerModuleDefaultPipeline.
> > >
> > > This patch add a pass
> > >
> > >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> > >
> > > which only gives you the wrong signal  of instrumentation is done.
> > >
> > > I wrote pass PGOInstrumentationGenCreateVar only for some trick 
> > > interactions for thinlto under ldd for CSPGO.
> > >  Regular FDO should not use it.
> > >
> > > The right fix is to enable PGO instrumentation and use in pass builder 
> > > for O0.
> > >
> > > I would like to request to revert this patch.
> >
> >
> > As an alternative, could I instead request that we remove the BackendUtil 
> > changes and just mark the runs in gcc-flag-compatibility.c with 
> > `-fno-experimental-new-pass-manager`. This way we could clarify that under 
> > the new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as 
> > is.
>
>
> No, I think we should be doing PGO at O0 in the new PM if we do so in the old 
> PM.
>
> I think Rong is saying that the *way* you're enabling PGO at O0 isn't correct 
> to fix this test case. That seems plausible to me at least, and I think 
> reverting and figuring out what the right way to do it is a fine approach.
>
> I just think we also need to understand why *no other test failed*, and why 
> the only test we have for doing PGO at O0 is this one and it could be "fixed" 
> but such a deeply unrelated change





Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Leonard Chan via Phabricator via cfe-commits 
leonardchan added a comment.

Understood. I'll revert this patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Chandler Carruth via Phabricator via cfe-commits 
chandlerc added a comment.

In D63155#1563240 , @leonardchan wrote:

> In D63155#1563229 , @xur wrote:
>
> > This patch does not make sense to me.
> >
> > The reason of failing with -fexperimental-new-pass-manager is because we 
> > don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> > instrumentation/use passes are under 
> > PassBuilder::buildPerModuleDefaultPipeline.
> >
> > This patch add a pass
> >
> >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> >
> > which only gives you the wrong signal  of instrumentation is done.
> >
> > I wrote pass PGOInstrumentationGenCreateVar only for some trick 
> > interactions for thinlto under ldd for CSPGO.
> >  Regular FDO should not use it.
> >
> > The right fix is to enable PGO instrumentation and use in pass builder for 
> > O0.
> >
> > I would like to request to revert this patch.
>
>
> As an alternative, could I instead request that we remove the BackendUtil 
> changes and just mark the runs in gcc-flag-compatibility.c with 
> `-fno-experimental-new-pass-manager`. This way we could clarify that under 
> the new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as is.


No, I think we should be doing PGO at O0 in the new PM if we do so in the old 
PM.

I think Rong is saying that the *way* you're enabling PGO at O0 isn't correct 
to fix this test case. That seems plausible to me at least, and I think 
reverting and figuring out what the right way to do it is a fine approach.

I just think we also need to understand why *no other test failed*, and why the 
only test we have for doing PGO at O0 is this one and it could be "fixed" but 
such a deeply unrelated change


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

Re: [PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Rong Xu via cfe-commits 
they are not doing the exacly the same thing for old pass manager and new
pass manger: old pass manger is doing instrumentation, while the new pass
manager with this change is NOT.
the test is not check instrumentation, (it only check the variables that
generates by InstroProfiling pass).
In this sense, the test is not well written.

I can draft a patch for this.

On Fri, Jun 28, 2019 at 4:53 PM Chandler Carruth via Phabricator <
revi...@reviews.llvm.org> wrote:

> chandlerc added a comment.
>
> In D63155#1563229 , @xur wrote:
>
> > This patch does not make sense to me.
> >
> > The reason of failing with -fexperimental-new-pass-manager is because we
> don't do PGO instrumentation at -O0. This is due to the fact that PGO
> instrumentation/use passes are under
> PassBuilder::buildPerModuleDefaultPipeline.
> >
> > This patch add a pass
> >
> >   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
> >
> > which only gives you the wrong signal  of instrumentation is done.
> >
> > I wrote pass PGOInstrumentationGenCreateVar only for some trick
> interactions for thinlto under ldd for CSPGO.
> >  Regular FDO should not use it.
> >
> > The right fix is to enable PGO instrumentation and use in pass builder
> for O0.
> >
> > I would like to request to revert this patch.
>
>
> I mean, I'm happy for the patch to be reverted, but I still really don't
> understand why this fixes the test to work *exactly* the same as w/ the old
> pass manager and doesn't break any other tests if it is completely wrong?
> It seems like there must be something strange with the test coverage if
> this is so far off of correct without any failures...
>
> I also don't understand what fix you are suggesting instead, but maybe you
> can show a patch?
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D63155/new/
>
> https://reviews.llvm.org/D63155
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Leonard Chan via Phabricator via cfe-commits 
leonardchan added a comment.

> I mean, I'm happy for the patch to be reverted, but I still really don't 
> understand why this fixes the test to work *exactly* the same as w/ the old 
> pass manager and doesn't break any other tests if it is completely wrong? It 
> seems like there must be something strange with the test coverage if this is 
> so far off of correct without any failures...
> 
> I also don't understand what fix you are suggesting instead, but maybe you 
> can show a patch?

This is also the fix I'm suggesting.

  diff --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
  index 5d66473e7b9..f924ecbd8c6 100644
  --- a/clang/lib/CodeGen/BackendUtil.cpp
  +++ b/clang/lib/CodeGen/BackendUtil.cpp
  @@ -1220,12 +1220,13 @@ void 
EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
 }
   }
   
  -if (CodeGenOpts.OptimizationLevel == 0)
  +if (CodeGenOpts.OptimizationLevel == 0) {
 addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
   
  -if (CodeGenOpts.hasProfileIRInstr()) {
  -  // This file is stored as the ProfileFile.
  -  MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
  +  if (CodeGenOpts.hasProfileIRInstr()) {
  +// This file is stored as the ProfileFile.
  +MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
  +  }
   }
 }


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Leonard Chan via Phabricator via cfe-commits 
leonardchan added a comment.

In D63155#1563229 , @xur wrote:

> This patch does not make sense to me.
>
> The reason of failing with -fexperimental-new-pass-manager is because we 
> don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> instrumentation/use passes are under 
> PassBuilder::buildPerModuleDefaultPipeline.
>
> This patch add a pass
>
>   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
>
> which only gives you the wrong signal  of instrumentation is done.
>
> I wrote pass PGOInstrumentationGenCreateVar only for some trick interactions 
> for thinlto under ldd for CSPGO.
>  Regular FDO should not use it.
>
> The right fix is to enable PGO instrumentation and use in pass builder for O0.
>
> I would like to request to revert this patch.


As an alternative, could I instead request that we remove the BackendUtil 
changes and just mark the runs in gcc-flag-compatibility.c with 
`-fno-experimental-new-pass-manager`. This way we could clarify that under the 
new PM, we shouldn't run PGO at -O0? If not, I'll revert this patch as is.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Chandler Carruth via Phabricator via cfe-commits 
chandlerc added a comment.

In D63155#1563229 , @xur wrote:

> This patch does not make sense to me.
>
> The reason of failing with -fexperimental-new-pass-manager is because we 
> don't do PGO instrumentation at -O0. This is due to the fact that PGO 
> instrumentation/use passes are under 
> PassBuilder::buildPerModuleDefaultPipeline.
>
> This patch add a pass
>
>   MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
>
> which only gives you the wrong signal  of instrumentation is done.
>
> I wrote pass PGOInstrumentationGenCreateVar only for some trick interactions 
> for thinlto under ldd for CSPGO.
>  Regular FDO should not use it.
>
> The right fix is to enable PGO instrumentation and use in pass builder for O0.
>
> I would like to request to revert this patch.


I mean, I'm happy for the patch to be reverted, but I still really don't 
understand why this fixes the test to work *exactly* the same as w/ the old 
pass manager and doesn't break any other tests if it is completely wrong? It 
seems like there must be something strange with the test coverage if this is so 
far off of correct without any failures...

I also don't understand what fix you are suggesting instead, but maybe you can 
show a patch?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread Rong Xu via Phabricator via cfe-commits 
xur added a comment.

This patch does not make sense to me.

The reason of failing with -fexperimental-new-pass-manager is because we don't 
do PGO instrumentation at -O0. This is due to the fact that PGO 
instrumentation/use passes are under PassBuilder::buildPerModuleDefaultPipeline.

This patch add a pass
 MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
which only gives you the wrong signal  of instrumentation is done.

I wrote pass PGOInstrumentationGenCreateVar only for some trick interactions 
for thinlto under ldd for CSPGO.
Regular FDO should not use it.

The right fix is to enable PGO instrumentation and use in pass builder for O0.

I would like to request to revert this patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-28 Thread David Li via Phabricator via cfe-commits 
davidxl added a comment.

Rong, can you take a look at this patch?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-13 Thread Leonard Chan via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363278: [clang][NewPM] Fix broken profile test (authored by 
leonardchan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63155?vs=204124=204574#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63155

Files:
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/test/Profile/gcc-flag-compatibility.c


Index: cfe/trunk/test/Profile/gcc-flag-compatibility.c
===
--- cfe/trunk/test/Profile/gcc-flag-compatibility.c
+++ cfe/trunk/test/Profile/gcc-flag-compatibility.c
@@ -7,25 +7,29 @@
 // -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 | FileCheck 
-check-prefix=PROFILE-GEN %s
+// 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: __llvm_profile_filename
 
 // Check that -fprofile-generate=/path/to generates /path/to/default.profraw
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to | 
FileCheck -check-prefix=PROFILE-GEN-EQ %s
+// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN-EQ %s
+// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN-EQ %s
 // PROFILE-GEN-EQ: constant [{{.*}} x i8] c"/path/to{{/|\\5C}}{{.*}}\00"
 
 // Check that -fprofile-use=some/path reads some/path/default.profdata
 // RUN: rm -rf %t.dir
 // RUN: mkdir -p %t.dir/some/path
 // RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o 
%t.dir/some/path/default.profdata
-// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path | FileCheck -check-prefix=PROFILE-USE-2 %s
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path -fno-experimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-USE-2 %s
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path -fexperimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-USE-2 %s
 // PROFILE-USE-2: = !{!"branch_weights", i32 101, i32 2}
 
 // Check that -fprofile-use=some/path/file.prof reads some/path/file.prof
 // RUN: rm -rf %t.dir
 // RUN: mkdir -p %t.dir/some/path
 // RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o 
%t.dir/some/path/file.prof
-// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path/file.prof | FileCheck 
-check-prefix=PROFILE-USE-3 %s
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path/file.prof -fno-experimental-new-pass-manager | 
FileCheck -check-prefix=PROFILE-USE-3 %s
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path/file.prof -fexperimental-new-pass-manager | 
FileCheck -check-prefix=PROFILE-USE-3 %s
 // PROFILE-USE-3: = !{!"branch_weights", i32 101, i32 2}
 
 int X = 0;
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -60,6 +60,7 @@
 #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h"
 #include "llvm/Transforms/Instrumentation/InstrProfiling.h"
 #include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
+#include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
 #include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar.h"
@@ -1216,6 +1217,11 @@
 
 if (CodeGenOpts.OptimizationLevel == 0)
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
+
+if (CodeGenOpts.hasProfileIRInstr()) {
+  // This file is stored as the ProfileFile.
+  MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
+}
   }
 
   // FIXME: We still use the legacy pass manager to do code generation. We


Index: cfe/trunk/test/Profile/gcc-flag-compatibility.c
===
--- cfe/trunk/test/Profile/gcc-flag-compatibility.c
+++ cfe/trunk/test/Profile/gcc-flag-compatibility.c
@@ -7,25 +7,29 @@
 // -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 | FileCheck -check-prefix=PROFILE-GEN

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-12 Thread Chandler Carruth via Phabricator via cfe-commits 
chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

Let's update the test to explicitly run w/ both PMs to make sure this keeps 
working. LGTM with that change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63155



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

[PATCH] D63155: [clang][NewPM] Fix broken profile test

2019-06-11 Thread Leonard Chan via Phabricator via cfe-commits 
leonardchan created this revision.
leonardchan added reviewers: chandlerc, echristo, phosek, serge-sans-paille.
leonardchan added a project: clang.
leonardchan added a parent revision: D62225: [clang][NewPM] Fixing -O0 tests 
that are broken under new PM.

This contains the part of D62225  which fixes 
`Profile/gcc-flag-compatibility.c` by adding the pass that allows default 
profile generation to work under the new PM. It seems that `./default.profraw` 
was not being generated with new PM enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63155

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -60,6 +60,7 @@
 #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h"
 #include "llvm/Transforms/Instrumentation/InstrProfiling.h"
 #include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
+#include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
 #include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar.h"
@@ -1216,6 +1217,11 @@
 
 if (CodeGenOpts.OptimizationLevel == 0)
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
+
+if (CodeGenOpts.hasProfileIRInstr()) {
+  // This file is stored as the ProfileFile.
+  MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
+}
   }
 
   // FIXME: We still use the legacy pass manager to do code generation. We


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -60,6 +60,7 @@
 #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h"
 #include "llvm/Transforms/Instrumentation/InstrProfiling.h"
 #include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
+#include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
 #include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar.h"
@@ -1216,6 +1217,11 @@
 
 if (CodeGenOpts.OptimizationLevel == 0)
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
+
+if (CodeGenOpts.hasProfileIRInstr()) {
+  // This file is stored as the ProfileFile.
+  MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->ProfileFile));
+}
   }
 
   // FIXME: We still use the legacy pass manager to do code generation. We
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits