[PATCH] Add warning capabilities in LLVM (backend part)

[Hal Finkel]

----- Original Message -----
> From: "Quentin Colombet" <qcolombet at apple.com>
> To: dblaikie at gmail.com, echristo at gmail.com, "renato golin" <renato.golin at linaro.org>, chandlerc at gmail.com,
> qcolombet at apple.com
> Cc: llvm-commits at cs.uiuc.edu, hfinkel at anl.gov
> Sent: Friday, November 15, 2013 1:18:22 PM
> Subject: Re: [PATCH] Add warning capabilities in LLVM (backend part)
> 
> 
>   Hi Hal,
> 
>   Thanks for the feedback.
>   I suggest that you read the RFC regarding the choices for the
>   interface.
>   What you are proposing has IIRC already been discussed.

In part, yes, but not exactly, and I think it makes sense to have a more-targeted discussion now that we have an implementation to play with.

> 
>   Let me try to summarize why we did not choose what you are
>   proposing:
>   1. srcloc metadata.
>   We cannot anticipate what would be reported and in particular, a
>   source location may not be adapted. Assume you want to report why
>   the vectorizer failed to optimize a given loop. Having only one
>   source location will not be enough. So why should we be bound to
>   one source location?

No, my point is:

 1. We need the srcloc data to deal with inline asm messages (this is an existing requirement).

 2. Because the mechanism is completely general (although currently used only for inline asm), it makes sense for it to be a first-class part of the interface. Might we want to extend this to deal with more general situation later? Maybe. But we need at least one srcloc for inline asm messages, and we can discuss how to deal with multiple-location messages as we have concrete use cases (although if we have a value array, we can also put them in there).

> 
>   2. StringRef vs. ArrayRef<Value*>
>   First I will quote David Blaikie and Chris Lattner on StringRef vs.
>   ArrayRef<StringRef>:
>   David: “Presumably there are diagnostics that are going to have
>   more than one parameter, though. ArrayRef<StringRef>?”
>   Chris: “It's possible, but the full generality of any diagnostic
>   can be folded (at some loss of generality) into a single string.
>    To balance complexity and generality vs simplicity, "1" is a
>   pretty decent number.”

Fair enough, and in terms of ArrayRef<StringRef> I completely agree, but:

 1. We already have a message string (and I don't think that we need more).

 2. Value* also gives us constants, metadata, and essentially everything else. Even the backend often has values from things in the MMO structure.

 3. The backend is the wrong place to do string formatting for user consumption. Should it demangle function names too? ;)

> 
>   Now, why using StringRef instead of Value, I see two reasons for
>   that:
>   - I believe we do not want the diagnostic system of the front-end
>   to have to deal with the IR.

 - The IR is the interface between the frontend and the backend. Will the frontend know what to do with some arbitrary Value*? Probably not. That's not the point. That Value* can be a ConstantInt, or a MDNode (maybe a srcloc, maybe a loop id, some debug metadata, etc.), a global variable, and many other things. Only the frontend knows how to format those things reasonably in the context of the source language. 

>   - What happens for a report coming for the MC level (e.g.,
>   AsmParser), IIRC, there is no Value involved here.\

 - The backends often do have values (from MMO), and can certainly use IR interfaces to create constants, metadata, etc. as necessary. I'd rather pass constants as constants, than having to parse them again later out of a string.

> 
>   3. Default message printer.
>   The idea of the default message printer is to provide a directly
>   usable message.

 - I think that we should provide a utility function that will take the string + the value array and produce a default serialization of the message. Any frontend that does not really care too much (or opt, for example), can use that.

> I believe we do not what to push in every
>   front-end a parsing of the default message.

 - I agree, but we really should give them that option.

 Moreover, like I said,
>   I think we may not be able to produce Value objects every time.

 - I disagree, we can always produce value objects -- value objects can be metadata strings, and are strictly more expressive than strings.

> 
>   What do you think?

There you have it ;)

Thanks again,
Hal

> 
>   Cheers,
>   Quentin
> 
> http://llvm-reviews.chandlerc.com/D2190
>