[PATCH] D46860: [IRCE] Fix miscompile with range checks against negative values
Max Kazantsev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 14 19:25:35 PDT 2018
mkazantsev created this revision.
mkazantsev added reviewers: samparker, skatkov, reames.
In the patch https://reviews.llvm.org/rL329547, we have lifted the over-restrictive limitation on collected range
checks, allowing to work with range checks with the end of their range not being
provably non-negative. However it appeared that the non-negativity of this value was
assumed in the utility function `ClampedSubtract`. In particular, its reasoning is based
on the fact that `0 <= SINT_MAX - X`, which is not true if `X` is negative.
The function `ClampedSubtract` is only called twice, once with `X = 0` (which is OK)
and the second time with `X = IRC.getEnd()`, where we may now see the problem if
the end is actually a negative value. In this case, we may sometimes miscompile.
This patch is the conservative fix of the miscompile problem. Rather than rejecting
negative `getEnd()` values, we will check it for non-negativity in runtime. For this, we
use function `smax(smin(X, 0), -1) + 1` that is equal to `1` if `X` is non-negative and
is equal to 0 if `X` is negative. If we multiply `Begin, End` of safe iteration space by this
function calculated for `X = IRC.getEnd()`, we will get the original `[Begin, End)` if
`IRC.getEnd()` was non-negative (and, thus, `ClampedSubtract` worked correctly) and
the empty range `[0, 0)` in case if ` IRC.getEnd()` was negative.
So we in fact prohibit execution of the main loop if at least one of range checks was
made against a negative value (and we figured it out in runtime). It is still better than
what we have before (non-negativity had to be proved in compile time) and prevents
us from miscompile, however it is sometiles too restrictive for unsigned range checks
against a negative value (which in fact can be eliminated).
Once we re-implement `ClampedSubtract` in a way that it handles negative `X` correctly,
this limitation can be lifted, too.
https://reviews.llvm.org/D46860
Files:
lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
test/Transforms/IRCE/rc-negative-bound.ll
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46860.146740.patch
Type: text/x-patch
Size: 31497 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180515/4d458e05/attachment-0001.bin>
More information about the llvm-commits
mailing list