[llvm] [InstCombine] Fold `(icmp pred (trunc nuw/nsw X), C)` -> `(icmp pred X, (zext/sext C))` (PR #87935)

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Sun May 26 14:13:41 PDT 2024


vitalybuka wrote:

#93163 didn't help.

> I understand you want a clean MSAN run, but tools like this are meant to be helpful, not impeding. If there is a bug in the tool, it doesn't seem so helpful in this case.

I see it in a different way. 
Clang is a software product, msan is an established feature of the product, and users rely on it. New optimization triggers a pre-existed bug, which regresses existing features.  Even if the optimization is correct on it's own, it's still regression of the product. Usually it's up to the author to make sure there is no regression to existing features (though it can be just reverting and asking  maintainers for help). In rare cases it's impossible to make both features work correctly and trade-off is needed, but there is no evidence that this is the case. 

With a reproducer it would be totally fair to revert this patch.  However it's often better to revert early with just a promise to publish a reproducer slightly later.

I guess I will have time in the next 24h to investigate. I am still open to 3 possibilities:
1. Patch is wrong
2.  Msan is wrong (most likely)
3. User's code has UB

I will get back with the result soon.



 



https://github.com/llvm/llvm-project/pull/87935


More information about the llvm-commits mailing list