[PATCH] D29404: [ValueTracking] emit a warning when we detect a conflicting assumption (PR31809)

[Adam Nemet via Phabricator via llvm-commits]

anemet added inline comments.


================
Comment at: lib/Analysis/ValueTracking.cpp:800-804
+
+    Q.CxtI->getContext().diagnose(DiagnosticInfoOptimizationFailure(
+        *Q.CxtI->getFunction(), Q.CxtI->getDebugLoc(),
+        "Detected conflicting code assumptions. Program has undefined behavior "
+        "or internal compiler error."));
----------------
davide wrote:
> I'm struggling here.  It's true we're emitting a diagnostic related to the fact we can't optimize, but it's also true we're not optimizing because the IR to which we lowered exhibits undefined behaviour (so, there's a correctness issue). 
> Also, I thought we could use OptimizationRemarkEmitter for this kind of tasks? (+ Adam) @anemet 
I am not sure this should be an optimization failure.  I think (@hfinkel, correct me if I am wrong), we use failure for cases where the user has explicitly asked us to optimization the code in a way that we fail to do.  I.e. it's a pretty strong diagnostics -- almost like a warning.

In this case I am not sure how severe this message should be.  We can emit a missed optimization remark or an analysis  remark, the latter being the catch-all and usually the most noisy.

In terms of the API used, this is the legacy API that we're trying to move away from.  It would be great to convert instsimplify to require OptimizationRemarkEmitter analysis and then stash that somewhere (in Q perhaps?!) and then use the new ORE->emit(remark) interface.



https://reviews.llvm.org/D29404