r213112 - Handle diagnostic warnings in Frontend diagnostic handler.

Alp Toker alp at nuanti.com
Thu Jul 17 05:44:09 PDT 2014


On 16/07/2014 04:20, Alp Toker wrote:
>
> On 16/07/2014 03:40, Tyler Nowicki wrote:
>> Author: tnowicki
>> Date: Tue Jul 15 19:40:42 2014
>> New Revision: 213112
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=213112&view=rev
>> Log:
>> Handle diagnostic warnings in Frontend diagnostic handler.
>>
>> Clang uses a diagnostic handler to grab diagnostic messages so it can 
>> print them
>> with the line of source code they refer to. This patch extends this 
>> to handle
>> diagnostic warnings that were added to llvm to produce a warning when
>> loop vectorization is explicitly specified (using a pragma clang loop 
>> directive)
>> but fails.
>>
>> Reviewed by: Aaron Ballman
>>
>> Modified:
>>      cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
>>      cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>>      cfe/trunk/lib/CodeGen/CodeGenAction.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=213112&r1=213111&r2=213112&view=diff
>> ============================================================================== 
>>
>> --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Tue Jul 
>> 15 19:40:42 2014
>> @@ -41,6 +41,8 @@ def remark_fe_backend_optimization_remar
>>       InGroup<BackendOptimizationRemarkMissed>, DefaultRemark;
>>   def remark_fe_backend_optimization_remark_analysis : Remark<"%0">, 
>> BackendInfo,
>>       InGroup<BackendOptimizationRemarkAnalysis>, DefaultRemark;
>> +def warn_fe_backend_optimization_warning : Warning<"%0">, BackendInfo,
>> +    InGroup<BackendOptimizationWarnings>, DefaultWarn;
>>   def note_fe_backend_optimization_remark_invalid_loc : Note<"could "
>>     "not determine the original source location for %0:%1:%2">;
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=213112&r1=213111&r2=213112&view=diff
>> ============================================================================== 
>>
>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jul 15 
>> 19:40:42 2014
>> @@ -704,6 +704,7 @@ def RemarkBackendPlugin : DiagGroup<"rem
>>   def BackendOptimizationRemark : DiagGroup<"pass">;
>>   def BackendOptimizationRemarkMissed : DiagGroup<"pass-missed">;
>>   def BackendOptimizationRemarkAnalysis : DiagGroup<"pass-analysis">;
>> +def BackendOptimizationWarnings : DiagGroup<"optimization-warning">;
>
> The diagnostic group name shouldn't contain the severity. It should be 
> something along the lines of DiagGroup<"optimization"> or 
> DiagGroup<"missed-optimization"> because the usage will be 
> -Woptimization, Werror=optimization and so on.
>
>>     // Instrumentation based profiling warnings.
>>   def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=213112&r1=213111&r2=213112&view=diff
>> ============================================================================== 
>>
>> --- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Tue Jul 15 19:40:42 2014
>> @@ -238,15 +238,16 @@ namespace clang {
>>       /// \brief Specialized handlers for optimization remarks.
>>       /// Note that these handlers only accept remarks and they 
>> always handle
>>       /// them.
>> -    void
>> -    EmitOptimizationRemark(const 
>> llvm::DiagnosticInfoOptimizationRemarkBase &D,
>> -                           unsigned DiagID);
>> +    void EmitOptimizationMessage(const 
>> llvm::DiagnosticInfoOptimizationBase &D,
>> +                                 unsigned DiagID);
>>       void
>>       OptimizationRemarkHandler(const 
>> llvm::DiagnosticInfoOptimizationRemark &D);
>>       void OptimizationRemarkHandler(
>>           const llvm::DiagnosticInfoOptimizationRemarkMissed &D);
>>       void OptimizationRemarkHandler(
>>           const llvm::DiagnosticInfoOptimizationRemarkAnalysis &D);
>> +    void OptimizationWarningHandler(
>> +        const llvm::DiagnosticInfoOptimizationWarning &D);
>
> I don't get this. The frontend maps backend remarks to frontend 
> diagnostics so why the special case?
>
>>     };
>>         void BackendConsumer::anchor() {}
>> @@ -416,10 +417,11 @@ BackendConsumer::StackSizeDiagHandler(co
>>     return false;
>>   }
>>   -void BackendConsumer::EmitOptimizationRemark(
>> -    const llvm::DiagnosticInfoOptimizationRemarkBase &D, unsigned 
>> DiagID) {
>> -  // We only support remarks.
>> -  assert(D.getSeverity() == llvm::DS_Remark);
>> +void BackendConsumer::EmitOptimizationMessage(
>> +    const llvm::DiagnosticInfoOptimizationBase &D, unsigned DiagID) {
>> +  // We only support warnings and remarks.
>> +  assert(D.getSeverity() == llvm::DS_Remark ||
>> +         D.getSeverity() == llvm::DS_Warning);
>>       SourceManager &SourceMgr = Context->getSourceManager();
>>     FileManager &FileMgr = SourceMgr.getFileManager();
>> @@ -442,8 +444,12 @@ void BackendConsumer::EmitOptimizationRe
>>       if (const Decl *FD = 
>> Gen->GetDeclForMangledName(D.getFunction().getName()))
>>         Loc = FD->getASTContext().getFullLoc(FD->getBodyRBrace());
>>   -  Diags.Report(Loc, DiagID) << AddFlagValue(D.getPassName())
>> -                            << D.getMsg().str();
>> +  // Flag value not used by all optimization messages.
>> +  if (D.getPassName())
>> +    Diags.Report(Loc, DiagID) << AddFlagValue(D.getPassName())
>> +                              << D.getMsg().str();
>> +  else
>> +    Diags.Report(Loc, DiagID) << D.getMsg().str();
>
> Is this change necessary? The existing code had the same functionality 
> without the need for an if/else here AFAICT.
>
>
>>       if (DILoc.isInvalid())
>>       // If we were not able to translate the file:line:col information
>> @@ -460,7 +466,7 @@ void BackendConsumer::OptimizationRemark
>>     // expression that matches the name of the pass name in \p D.
>>     if (CodeGenOpts.OptimizationRemarkPattern &&
>> CodeGenOpts.OptimizationRemarkPattern->match(D.getPassName()))
>> -    EmitOptimizationRemark(D, 
>> diag::remark_fe_backend_optimization_remark);
>> +    EmitOptimizationMessage(D, 
>> diag::remark_fe_backend_optimization_remark);
>>   }
>>     void BackendConsumer::OptimizationRemarkHandler(
>> @@ -470,8 +476,8 @@ void BackendConsumer::OptimizationRemark
>>     // name in \p D.
>>     if (CodeGenOpts.OptimizationRemarkMissedPattern &&
>> CodeGenOpts.OptimizationRemarkMissedPattern->match(D.getPassName()))
>> -    EmitOptimizationRemark(D,
>> - diag::remark_fe_backend_optimization_remark_missed);
>> +    EmitOptimizationMessage(D,
>> + diag::remark_fe_backend_optimization_remark_missed);
>>   }
>>     void BackendConsumer::OptimizationRemarkHandler(
>> @@ -481,10 +487,15 @@ void BackendConsumer::OptimizationRemark
>>     // name in \p D.
>>     if (CodeGenOpts.OptimizationRemarkAnalysisPattern &&
>> CodeGenOpts.OptimizationRemarkAnalysisPattern->match(D.getPassName()))
>> -    EmitOptimizationRemark(
>> +    EmitOptimizationMessage(
>>           D, diag::remark_fe_backend_optimization_remark_analysis);
>>   }
>>   +void BackendConsumer::OptimizationWarningHandler(
>> +    const llvm::DiagnosticInfoOptimizationWarning &D) {
>> +  EmitOptimizationMessage(D, 
>> diag::warn_fe_backend_optimization_warning);
>> +}
>> +
>>   /// \brief This function is invoked when the backend needs
>>   /// to report something to the user.
>>   void BackendConsumer::DiagnosticHandlerImpl(const DiagnosticInfo 
>> &DI) {
>> @@ -518,6 +529,11 @@ void BackendConsumer::DiagnosticHandlerI
>>       OptimizationRemarkHandler(
>> cast<DiagnosticInfoOptimizationRemarkAnalysis>(DI));
>>       return;
>> +  case llvm::DK_OptimizationWarning:
>> +    // Optimization warnings are always handled completely by this
>> +    // handler.
>> + 
>> OptimizationWarningHandler(cast<DiagnosticInfoOptimizationWarning>(DI));
>> +    return;
>>     default:
>>       // Plugin IDs are not bound to any value as they are set 
>> dynamically.
>>       ComputeDiagRemarkID(Severity, backend_plugin, DiagID);
>
> It's not clear how the warning is enabled -- was there meant to be a 
> separate driver patch?
>
> Tyler, could you back out this commit for now and repost with some tests? 

We definitely need tests for a user-facing feature like this and the 
review comments also need to be addressed.

I'm guessing you're away so I've reverted this for you in r213260, 
pending your feedback.

Thanks
Alp.


> It's best for someone who's been working on the frontend diagnostics 
> to review this -- I think the plan was to handle this with 
> user-defined diagnostic mappings rather than what's being done here.
>
>
>
>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>

-- 
http://www.nuanti.com
the browser experts




More information about the cfe-commits mailing list