Moving the discussion from llvm-dev here. This was Vivek's question.

Hi Adam,

I looked into code related to above feature request and perhaps I am not yet clear about this.
I have tried out following things:

1. allowExtraAnalysis function will have a string parameter which is pass name.

2. Find an appropriate entry point in Clang where we can make a shared_ptr for
   OptimizationRemarkPattern, OptimizationRemarkAnalysisPattern in LLVMContext class.

3. Use above changes so that allowExtraAnalysis can be used to check if specific
   remark is on or not.

However I am confused because there is one more class DiagnosticInfo in LLVM which handles same thing for opt's flags, but why those informations are not used in allowExtraAnalysis() ? Have I understood the purpose of allowExtraAnalysis or not ?

Please guide here.

Sincerely,
Vivek

Thanks for taking this on!

1. allowExtraAnalysis function will have a string parameter which is pass
   name.

I think it will also need the remark kind (e.g. DK_OptRemarkAnalysis) unless we want to return all three in a structure. The latter may be useful since passes may want to cache these together in order to have zero-overhead checking to see whether the remarks are off.

2. Find an appropriate entry point in Clang where we can make a shared_ptr
   for
   OptimizationRemarkPattern, OptimizationRemarkAnalysisPattern in LLVMContext
   class.

I am assuming you were thinking of doing this to call isEnabled on the remark; unfortunately that does not work. (I know that is what WholeProgramDevirt does too.) isEnabled is not used in clang in llvm (opt/llc):

https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CodeGenAction.cpp#L598

I think that the new design should move isEnabled to the DiagnosticHandler in the context. DiagnosticHandler is only a callback now so it should probably become a class. Then clients can ask whether a kind of remark (OptRemark, OptRemarkMissed, OptRemarkAnalysis) is enabled for a pass specified by the pass name.

3. Use above changes so that allowExtraAnalysis can be used to check if
   specific
   remark is on or not.

Yes.

However I am confused because there is one more` class DiagnosticInfo in LLVM
which handles same thing for opt's flags, but why those informations are not
used in allowExtraAnalysis() ? Have I understood the purpose of
allowExtraAnalysis or not ?

As explained above.

Thanks for taking this on!

1. allowExtraAnalysis function will have a string parameter which is pass
   name.

I think it will also need the remark kind (e.g. DK_OptRemarkAnalysis) unless
we want to return all three in a structure. The latter may be useful since
passes may want to cache these together in order to have zero-overhead
checking to see whether the remarks are off.

2. Find an appropriate entry point in Clang where we can make a shared_ptr
   for
   OptimizationRemarkPattern, OptimizationRemarkAnalysisPattern in LLVMContext
   class.

I am assuming you were thinking of doing this to call isEnabled on the
remark; unfortunately that does not work. (I know that is what
WholeProgramDevirt does too.) isEnabled is not used in clang in llvm
(opt/llc):

https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CodeGenAction.
cpp#L598
No actually I wanted to keep pointer for CodeGenOpts.OptimizationRemarkPattern and other similar objects in LLVMContext so that we can take decision based on clang's -Rpass options.
With that thought I am confused because as opt/llc and clang uses different options for same thing and user passes both things (particularly in clang though -mllvm) then it can creates problem. So I thought we can teach clang that drop opt/llc's options for ORE.

-Vivek

I think that the new design should move isEnabled to the DiagnosticHandler
in the context. DiagnosticHandler is only a callback now so it should
probably become a class. Then clients can ask whether a kind of remark
(OptRemark, OptRemarkMissed, OptRemarkAnalysis) is enabled for a pass
specified by the pass name.

3. Use above changes so that allowExtraAnalysis can be used to check if
   specific
   remark is on or not.

Yes.

However I am confused because there is one more` class DiagnosticInfo in LLVM
which handles same thing for opt's flags, but why those informations are not
used in allowExtraAnalysis() ? Have I understood the purpose of
allowExtraAnalysis or not ?

As explained above.

Thanks for taking this on!

1. allowExtraAnalysis function will have a string parameter which is pass
   name.

I think it will also need the remark kind (e.g. DK_OptRemarkAnalysis) unless
we want to return all three in a structure. The latter may be useful since
passes may want to cache these together in order to have zero-overhead
checking to see whether the remarks are off.

2. Find an appropriate entry point in Clang where we can make a shared_ptr
   for
   OptimizationRemarkPattern, OptimizationRemarkAnalysisPattern in LLVMContext
   class.

I am assuming you were thinking of doing this to call isEnabled on the
remark; unfortunately that does not work. (I know that is what
WholeProgramDevirt does too.) isEnabled is not used in clang in llvm
(opt/llc):

https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CodeGenAction.
cpp#L598
No actually I wanted to keep pointer for
CodeGenOpts.OptimizationRemarkPattern and other similar objects in
LLVMContext so that we can take decision based on clang's -Rpass options.
With that thought I am confused because as opt/llc and clang uses different
options for same thing and user passes both things (particularly in clang
though -mllvm) then it can creates problem. So I thought we can teach clang
that drop opt/llc's options for ORE.
I am not sure why currently it is not done but when -fsave-optimization-record is present then clang should covert -Rpass to -mllvm equivalent opt/llc's option.

-Vivek

I think that the new design should move isEnabled to the DiagnosticHandler
in the context. DiagnosticHandler is only a callback now so it should
probably become a class. Then clients can ask whether a kind of remark
(OptRemark, OptRemarkMissed, OptRemarkAnalysis) is enabled for a pass
specified by the pass name.

3. Use above changes so that allowExtraAnalysis can be used to check if
   specific
   remark is on or not.

Yes.

However I am confused because there is one more` class DiagnosticInfo in LLVM
which handles same thing for opt's flags, but why those informations are not
used in allowExtraAnalysis() ? Have I understood the purpose of
allowExtraAnalysis or not ?

As explained above.

I am not sure why currently it is not done but when -fsave-optimization-record is present then clang should covert -Rpass to -mllvm equivalent opt/llc's option.

-mllvm options are for compiler development purposes only; they're not part of the user interface. Moreover, using command-line options to pass information between the frontend and the backend is highly discouraged.

Work in progress patch is https://reviews.llvm.org/D33514

Vivek fixed this in r313382. Thanks Vivek!