[PATCH] D84940: [JumpThreading] Conditionally freeze its condition when unfolding select
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 30 15:05:03 PDT 2020
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
More information about the llvm-commits
mailing list