[llvm] [InstCombine] Fold `(icmp pred (trunc nuw/nsw X), C)` -> `(icmp pred X, (zext/sext C))` (PR #87935)
via llvm-commits
llvm-commits at lists.llvm.org
Fri May 24 09:55:21 PDT 2024
goldsteinn wrote:
> > > This patch causing false MSAN reports. It's likely a bug in MSAN, but I didn't figured out the fix yet.
> >
> >
> > As in breaks MSAN, or MSAN now complains about LLVM?
>
> Msan reports false bugs in users code.
>
> > > Would it be OK to revert the patch while I am investigating, to avoid false reports for us and other MSAN users?
> >
> >
> > I guess preferably not. Can you wait until there have some actual reports / an issue justifying.
>
> I have multiple reports on our internal codebase, I just need to minimize them. Inconveniently, I am traveling this week. Also as soon I have reduced reproducer I will likely have a msan patch. Usually it's just over aggressive handling of some instruction which need to be relaxed.
>
> Also if we wait, it can be branched into clang RC, which will need cherry-picks, potentially with conflicts.
The thing is, we want this patch (from a codegen POV). It is necessary to cleanup regressions otherwise assosiated w/ #90436.
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.
Is there any way to disable MSAN for this region (or just disable this fold if msan is enabled) as a stopgap?
If not, ill defer to @nikic, if he thinks the regressions w.o this are acceptable, then revert, but please keep me updated on the progress of the fix.
https://github.com/llvm/llvm-project/pull/87935
More information about the llvm-commits
mailing list