[PATCH] D84940: [JumpThreading] Conditionally freeze its condition when unfolding select

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:2809
+  if (!InsertFreezeWhenUnfoldingSelect &&
+      BB->getParent()->hasFnAttribute(Attribute::SanitizeMemory))
     return false;
----------------
eugenis wrote:
> efriedma wrote:
> > I suspect we don't want to change this.  Even if "freeze" version of the transform is legal with msan, it probably reduces the quality of msan diagnostics.
> Actually, I think this would reintroduce https://bugs.llvm.org/show_bug.cgi?id=45220, because the current handling of freeze instruction in MSan is to check (and report) the argument and mark the result as initialized / defined. 
> 
> Is this wrong? Should we instead ignore the input shadow in freeze? In that case, I agree, this change will make MSan miss some issues.
If msan diagnoses uninitialized arguments to freeze, that defeats the point of freeze: the behavior should be defined even if the operand is undef/poison.

Probably we want to avoid performing transforms that require introducing freeze before msan instrumentation is inserted.  But I think if msan does see a freeze, it's probably better to respect it, as opposed to producing a false-positive diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84940