warning from -instcombine

Hi,

I noticed this code in lib/Transforms/Scalar/InstructionCombining.cpp:

      cerr << "WARNING: While resolving call to function '"
           << Callee->getName() << "' arguments were dropped!\n";

If you're using LLVM as a static compiler, this warning is a bit
incongruous, because it's not formatted in the same way as warnings
from the front end, and it isn't suppressed by -w.

If you're using LLVM as a JIT, it seems completely wrong for any
optimisation pass to use cerr like this.

So should it be removed, or downgraded to DOUT, or something?

There are a few other instances of cerr << "WARNING: ..." in other LLVM passes.

Thanks,
Jay.

Hi,

I noticed this code in lib/Transforms/Scalar/InstructionCombining.cpp:

     cerr << "WARNING: While resolving call to function '"
          << Callee->getName() << "' arguments were dropped!\n";

If you're using LLVM as a static compiler, this warning is a bit
incongruous, because it's not formatted in the same way as warnings
from the front end, and it isn't suppressed by -w.

If you're using LLVM as a JIT, it seems completely wrong for any
optimisation pass to use cerr like this.

So should it be removed, or downgraded to DOUT, or something?

We should not remove them. These indicate a bug in the source, frontend, or earlier optimization passes. It's pretty useful. It would be nice to have a backend diagnosis facility then we can control them. Patches welcome.

Evan

Hi,

I noticed this code in lib/Transforms/Scalar/InstructionCombining.cpp:

    cerr << "WARNING: While resolving call to function '"
         << Callee->getName() << "' arguments were dropped!\n";

If you're using LLVM as a static compiler, this warning is a bit
incongruous, because it's not formatted in the same way as warnings
from the front end, and it isn't suppressed by -w.

If you're using LLVM as a JIT, it seems completely wrong for any
optimisation pass to use cerr like this.

So should it be removed, or downgraded to DOUT, or something?

We should not remove them. These indicate a bug in the source,
frontend, or earlier optimization passes. It's pretty useful.

Why not use assert() like mechanism to catch such bugs?

The compiler should not crash on invalid or poorly formed code.

-Chris

How about setting an error flag, and error message in the Module, then
let the caller know the Module is broken?
Sort of like what should happen on invalid inline assembly instead of
abort(), as we already talked about.

Best regards,
--Edwin

How about setting an error flag, and error message in the Module, then
let the caller know the Module is broken?

I'm not sure what you mean by "broken". In the case of this "arguments
were dropped" warning, the module just contains some code that will
give undefined behaviour if it gets executed. It would be reasonable
for an optimiser to turn that code into a call to abort(). In the
static compilation case it would also be reasonable to warn the user;
it's just that dumping this message to cerr is a very crude way of
doing that, compared to the sort of warnings that a user would expect
to see from a GCC-like compiler.

Jay.

How about setting an error flag, and error message in the Module, then
let the caller know the Module is broken?
    
I'm not sure what you mean by "broken". In the case of this "arguments
were dropped" warning, the module just contains some code that will
give undefined behaviour if it gets executed. It would be reasonable
for an optimiser to turn that code into a call to abort().

Sorry, if it is just a warning, then the module is not broken.

Maybe module can have a list of warnings, and the caller can check it.
Ideally these would be set by diagnostic passes.
llvm-gcc might print a warning, opt may continue to use cerr, etc.

I don't like warnings depending on optimizations being run, but if the
only way to catch this one
is by running instcombine, I think there is no other way.
If this warning could be caught by running an independent diagnostics
pass (LLVM doesn't have any currently),
then maybe this functionality should be moved into a pass of its own?

What do you think?

In the
static compilation case it would also be reasonable to warn the user;
it's just that dumping this message to cerr is a very crude way of
doing that, compared to the sort of warnings that a user would expect
to see from a GCC-like compiler.
  
Indeed.

Best regards,
--Edwin

Hi,

I noticed this code in lib/Transforms/Scalar/InstructionCombining.cpp:

     cerr << "WARNING: While resolving call to function '"
          << Callee->getName() << "' arguments were dropped!\n";

If you're using LLVM as a static compiler, this warning is a bit
incongruous, because it's not formatted in the same way as warnings
from the front end, and it isn't suppressed by -w.

I agree that this sucks. :frowning:

Unfortunately, there is no really good solution for this right now. We do not have great location information in the LLVM IR unless debug info is around, so it is hard to produce decent diagnostics.

OTOH, this specific warning is very useful, it can identify some serious issues that can only be detected at link-time.

Edwin says:

How about setting an error flag, and error message in the Module, then
let the caller know the Module is broken?
Sort of like what should happen on invalid inline assembly instead of
abort(), as we already talked about.

I agree. Giving optimizer/codegen a real diagnostics subsystem as Edwin proposes does sound like the right first step, even if it doesn't fix the "not having location info" problem. This would also handle the issues where the code generator will abort when presented with invalid inline asm, etc. If you were using a JIT you could recover from this instead of displaying it to the user.

-Chris