llvm dev - May 2016 - [RFC] New diagnostic handler for llc

On 12 May 2016 at 16:19, Krzysztof Parzyszek via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> The problem in PR24071 seemed to be that clang proceeded with compilation
> even though the inline asm was not valid.  I'm not sure that there is
value
> in trying to make the backend continue compiling code that most likely has
> no meaning.
I'm not 100% convinced that is the case here. The inline asm errors
are reported via LLVMContext::emitError, which has this documentation:

  /// emitError - Emit an error message to the currently installed error handler
  /// with optional location information.  This function returns, so code should
  /// be prepared to drop the erroneous construct on the floor and "not
crash".
  /// The generated code need not be correct.

Since the backend is the one calling emitError, the backend is the one
that shouldn't crash, regardless of what diagnostic handler is
installed. I think llc should help us test that this is the case. At
least that's my interpretation of things.

On Thu, May 12, 2016 at 10:16 AM, Diana Picus via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> On 12 May 2016 at 16:19, Krzysztof Parzyszek via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> The problem in PR24071 seemed to be that clang proceeded with
compilation
>> even though the inline asm was not valid.  I'm not sure that there
is value
>> in trying to make the backend continue compiling code that most likely
has
>> no meaning.
>
> I'm not 100% convinced that is the case here. The inline asm errors
> are reported via LLVMContext::emitError, which has this documentation:
>
>   /// emitError - Emit an error message to the currently installed error
handler
>   /// with optional location information.  This function returns, so code
should
>   /// be prepared to drop the erroneous construct on the floor and
"not crash".
>   /// The generated code need not be correct.
>
> Since the backend is the one calling emitError, the backend is the one
> that shouldn't crash, regardless of what diagnostic handler is
> installed. I think llc should help us test that this is the case. At
> least that's my interpretation of things.
I agree with your interpretation. We should give llc a diagnostic
handler so that 'emitError' doesn't exit immediately when we're
testing with llc.

On 5/12/2016 12:29 PM, Reid Kleckner wrote:
>
> I agree with your interpretation. We should give llc a diagnostic
> handler so that 'emitError' doesn't exit immediately when
we're
> testing with llc.
I don't think we should be calling emitError from the backend, except 
for the MachineVerifier.  The verifier could ascertain the validity of 
the input IR, and could report as many problems as it is able to find.

After that point I'd rather have an ICE when a problem is encountered.

-Krzysztof

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation

On 5/12/2016 12:16 PM, Diana Picus wrote:
> Since the backend is the one calling emitError, the backend is the one
> that shouldn't crash, regardless of what diagnostic handler is
> installed.
Ok, I agree that emitError should not cause llc to crash.

-Krzysztof

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation