[PATCH] D33300: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased
Sanjoy Das via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 17 15:17:26 PDT 2017
sanjoy created this revision.
Herald added a subscriber: mcrosier.
Right now `areMemoryOpsAliased` has an assertion justified as:
// MMO1 should have a value due it comes from operation we'd like to use
// as implicit null check.
assert(MMO1->getValue() && "MMO1 should have a Value!");
However, it is possible for that invariant to not be upheld in the
following situation (conceptually):
Null check %RAX
NotNullSucc:
%RAX = LEA %RSP, 16 // I0
%RDX = MOV64rm %RAX // I1
With the current code, we will have an early exit from
`ImplicitNullChecks::isSuitableMemoryOp` on `I0` with `SR_Unsuitable`.
However, `I1` will look plausible (since it loads from `%RAX`) and
will go ahead and call `areMemoryOpsAliased(I1, I0)`. This will cause
us to fail the assert mentioned above since `I1` does not load from an
IR level value and thus is allowed to have a non-Value base address.
The fix is to return `SR_Impossible` whenever we see an unsuitable
instruction overwrite `PointerReg`. This would guarantee that when we
call `areMemoryOpsAliased`, we're guaranteed to be looking at an
instruction that loads from or stores to an IR level value.
https://reviews.llvm.org/D33300
Files:
lib/CodeGen/ImplicitNullChecks.cpp
test/CodeGen/X86/ImplicitNullChecks/non-value-mem-operand.mir
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33300.99356.patch
Type: text/x-patch
Size: 17500 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170517/775815fb/attachment-0001.bin>
More information about the llvm-commits
mailing list