[PATCH] Add diagnostic capabilities in LLVM (frontend part)
Hal Finkel
hfinkel at anl.gov
Tue Dec 17 15:53:45 PST 2013
Hi Quentin,
Any reason you did not use phabricator for this patch?
+def BackendInlineAsm : DiagGroup<"backend-inline-asm">;
+def BackendStackSize : DiagGroup<"backend-stack-size">;
+def BackendPlugin : DiagGroup<"backend-plugin">;
Are these group names user-visible? If they are, do we want "backend" in the name?
+ // FIXME: We should demangle the function name.
+ // FIXME: Is there a way to get a location for that function?
+ FullSourceLoc Loc;
+ Diags.Report(Loc, diag::warn_fe_backend_stack_size)
+ << D.getStackSize() << D.getFunction().getName();
+ return true;
+}
+
We should figure this out ;) -- Furthermore, since you put in the work to have the diagnostic printers, we should use that to handle this stack-size warning instead of using this special class.
Looking through CodeGenModule.cpp, it does not look like there is such a map currently, although it looks like we could maintain such a map in the CodeGenModule::GetOrCreateLLVMFunction function.
-Hal
----- Original Message -----
> From: "Quentin Colombet" <qcolombet at apple.com>
> To: "Dmitri Gribenko" <gribozavr at gmail.com>
> Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>
> Sent: Tuesday, December 17, 2013 5:03:36 PM
> Subject: Re: [PATCH] Add diagnostic capabilities in LLVM (frontend part)
>
>
>
> Hi Dimitri,
>
> Thanks for the quick reply!
>
> New patch attached.
>
> On Dec 17, 2013, at 11:58 AM, Dmitri Gribenko <gribozavr at gmail.com>
> wrote:
>
> > On Tue, Dec 17, 2013 at 11:22 AM, Quentin Colombet
> > <qcolombet at apple.com> wrote:
> >> - Tests.
> >> I made some tests locally, but the patch does not add any test
> >> case. The
> >> question is how can we add tests that require both a backend and a
> >> frontend
> >> in either the frontend or the backend?
> >
> > A lot of tests in Clang have "REQUIRES: x86-registered-target" --
> > please take a look at them.
> Thanks, using your advice, I have added a test in
> test/Frontend/backend-diagnostic.c.
>
> >
> > About the patch:
> >
> > 80 columns.
> Oh sorry about that, I was sure I had run clang-format… apparently
> not!
> Fixed.
>
> >
> > +/// DiagnosticHandler2 - This function is invoked when the backend
> > needs
> > +/// to report something to the user.
> >
> > Please don't duplicate function name in comments.
> Fixed.
>
> >
> > +/// The \p DI interface provides two basic information:
> > +/// getKind() helps to determines what it is reporting and
> > +/// getSeverity() decribes how bad this is.
> >
> > This does not really belong to the documentation for
> > DiagnosticHandler2(), I think.
> Removed.
>
> >
> > +#define SeveritySwitch(Severity, GroupName, DiagID)
> > \
> > + switch (Severity) {
> > \
> >
> > Please wrap this in do {...} while(false), so that the trailing
> > semicolon in the macro invocation gets correctly attached to the
> > expansion.
> Done.
>
> >
> > Also, this macro could have a better name -- maybe ComputeSeverity?
> Renamed into ComputeDiagID.
>
> Thanks,
> -Quentin
>
>
>
>
> >
> > Dmitri
> >
> > --
> > main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> > (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the cfe-commits
mailing list