[PATCH] D76332: Fix MSan false positive due to select folding.

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 18 15:13:44 PDT 2020


spatel added a comment.

In D76332#1929773 <https://reviews.llvm.org/D76332#1929773>, @efriedma wrote:

> > Is there some reason that we didn't make the select semantics for poison explicit in the LangRef?
>
> The exact semantics were evolving for a while, and nobody actually wrote up where we landed with Alive2.


Thanks - I pushed the text from the bug report here:
rGfaba1d034a04 <https://reviews.llvm.org/rGfaba1d034a046305a3a7902e1753bf5dfc4f686e>
If we need to amend/extend that in some way, let me know.

>> Should I remove TryToUnfoldSelectInCurrBB completely? I don't see how it can be saved.
> 
> See the documentation comment on TryToUnfoldSelectInCurrBB.  The ultimate transform we're actually hoping to perform, which is jump threading to simplify the select, is legal.  So it's probably worth trying to do that, still, even if the intermediate step TryToUnfoldSelectInCurrBB actually implements isn't legal.
> 
> We already have some other code that tries to handle selects; I'm not sure how much of an optimization gap would remain if we just delete TryToUnfoldSelectInCurrBB.

I added the reduced example from this report as a regression test, so we can adjust this patch to just delete the whole function without having to add an MSan-specific test:
rG22c66c1a28c0 <https://reviews.llvm.org/rG22c66c1a28c042c51ee7059b954eff5db589ca92>

(can use 'utils/update_test_checks.py --function=TryToUnfoldSelectInCurrBB' to auto-gen the new CHECK lines for that test only)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76332/new/

https://reviews.llvm.org/D76332





More information about the llvm-commits mailing list