[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