Re: Instrumentation passes and -O0 optimization level

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Instrumentation passes and -O0 optimization level

Alexander Potapenko-2
>
>> + EP_EnabledOnOptLevel0
>
> I'd rename this as EP_AlwaysEnabled
>
Renamed, see the attachment.

But note that one needs to add his pass at two extension points: at O0
and wherever else he wanted to add it.
Won't such a name confuse the user?
E.g. he may think that just adding a pass as "EP_AlwaysEnabled" should
be enough to have it at any optimization level.

PS. Should we move the discussion to cfe-commits or it's ok to
continue the review process here?

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

clang.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Instrumentation passes and -O0 optimization level

Chandler Carruth-2
On Tue, Nov 29, 2011 at 8:56 AM, Alexander Potapenko <[hidden email]> wrote:
PS. Should we move the discussion to cfe-commits or it's ok to
continue the review process here?

For future reference, please send patches which touch both LLVM and Clang to llvm-commits and cfe-commits. However, looking at the Clang side of the patch, it is totally fine. =D

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Instrumentation passes and -O0 optimization level

Kostya Serebryany


On Tue, Nov 29, 2011 at 4:31 PM, Chandler Carruth <[hidden email]> wrote:
On Tue, Nov 29, 2011 at 8:56 AM, Alexander Potapenko <[hidden email]> wrote:
PS. Should we move the discussion to cfe-commits or it's ok to
continue the review process here?

For future reference, please send patches which touch both LLVM and Clang to llvm-commits and cfe-commits. However, looking at the Clang side of the patch, it is totally fine. =D

Alex, 
Now, the patch is actually a bit confusing to me. 
EP_AlwaysEnabled should mean "works with O0 after inliner and with >= O1 somewhere late", but it doesn't look like it works this way (otherwise, you wouldn't need to call PMBuilder.addExtension twice). 

--kcc 

 

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Instrumentation passes and -O0 optimization level

Alexander Potapenko
>
> Alex,
> Now, the patch is actually a bit confusing to me.
> EP_AlwaysEnabled should mean "works with O0 after inliner and with >= O1
> somewhere late", but it doesn't look like it works this way (otherwise, you
> wouldn't need to call PMBuilder.addExtension twice).
> ?
This was actually my question to Devang.
Any other suggestions for the EP name?

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Instrumentation passes and -O0 optimization level

Devang Patel

On Nov 29, 2011, at 11:26 PM, Alexander Potapenko wrote:

>>
>> Alex,
>> Now, the patch is actually a bit confusing to me.
>> EP_AlwaysEnabled should mean "works with O0 after inliner and with >= O1
>> somewhere late", but it doesn't look like it works this way (otherwise, you
>> wouldn't need to call PMBuilder.addExtension twice).
>> ?
> This was actually my question to Devang.
> Any other suggestions for the EP name?

OK, I withdraw my suggestion :)

-
Devang

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Instrumentation passes and -O0 optimization level

Kostya Serebryany
+cfe-commits (as the patch touches both llvm and clang)

On Wed, Nov 30, 2011 at 9:33 AM, Devang Patel <[hidden email]> wrote:

On Nov 29, 2011, at 11:26 PM, Alexander Potapenko wrote:

>>
>> Alex,
>> Now, the patch is actually a bit confusing to me.
>> EP_AlwaysEnabled should mean "works with O0 after inliner and with >= O1
>> somewhere late", but it doesn't look like it works this way (otherwise, you
>> wouldn't need to call PMBuilder.addExtension twice).
>> ?
> This was actually my question to Devang.
> Any other suggestions for the EP name?

OK, I withdraw my suggestion :)

Does anyone else have comments to the original patch (attached)?
Thanks, 

--kcc 



 

-
Devang



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

clang.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Instrumentation passes and -O0 optimization level

Chandler Carruth-2
On Wed, Nov 30, 2011 at 11:04 AM, Kostya Serebryany <[hidden email]> wrote:
+cfe-commits (as the patch touches both llvm and clang)

On Wed, Nov 30, 2011 at 9:33 AM, Devang Patel <[hidden email]> wrote:

On Nov 29, 2011, at 11:26 PM, Alexander Potapenko wrote:

>>
>> Alex,
>> Now, the patch is actually a bit confusing to me.
>> EP_AlwaysEnabled should mean "works with O0 after inliner and with >= O1
>> somewhere late", but it doesn't look like it works this way (otherwise, you
>> wouldn't need to call PMBuilder.addExtension twice).
>> ?
> This was actually my question to Devang.
> Any other suggestions for the EP name?

OK, I withdraw my suggestion :)

Does anyone else have comments to the original patch (attached)?
Thanks, 

I already commented (and gave a reminder to send to cfe-commits as well for patches to both).

it LGTM. =] 


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Instrumentation passes and -O0 optimization level

Kostya Serebryany
landed as r145530 (llvm) and r145531 (clang)
Thanks! 

On Wed, Nov 30, 2011 at 12:13 PM, Chandler Carruth <[hidden email]> wrote:
On Wed, Nov 30, 2011 at 11:04 AM, Kostya Serebryany <[hidden email]> wrote:
+cfe-commits (as the patch touches both llvm and clang)

On Wed, Nov 30, 2011 at 9:33 AM, Devang Patel <[hidden email]> wrote:

On Nov 29, 2011, at 11:26 PM, Alexander Potapenko wrote:

>>
>> Alex,
>> Now, the patch is actually a bit confusing to me.
>> EP_AlwaysEnabled should mean "works with O0 after inliner and with >= O1
>> somewhere late", but it doesn't look like it works this way (otherwise, you
>> wouldn't need to call PMBuilder.addExtension twice).
>> ?
> This was actually my question to Devang.
> Any other suggestions for the EP name?

OK, I withdraw my suggestion :)

Does anyone else have comments to the original patch (attached)?
Thanks, 

I already commented (and gave a reminder to send to cfe-commits as well for patches to both).

it LGTM. =] 



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev