[PATCH] D53776: [DAGCombiner] Fix for big endian in ForwardStoreValueToDirectLoad

[Nirav Dave via Phabricator via llvm-commits]

niravd accepted this revision.
niravd added a comment.
This revision is now accepted and ready to land.

Modulo removing the unnecessary condition you commented on, this looks good to me. Thanks.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12870
+      (Offset >= 0) &&
       (Offset * 8 <= LDMemType.getSizeInBits()) &&
       (Offset * 8 + LDMemType.getSizeInBits() <= STMemType.getSizeInBits());
----------------
bjope wrote:
> @niravd do you remember why we have the check `(Offset * 8 <= LDMemType.getSizeInBits())` here?
> 
> From my point of view it looks wrong. Maybe it is supposed to be `(Offset * 8 <= STMemType.getSizeInBits())`, i.e. checking that the load starts before the last bit written by the store. But then I guess it is enough to check
> `(Offset >= 0) && (Offset * 8 + LDMemType.getSizeInBits() <= STMemType.getSizeInBits())` or are we trying to catch some special case when we get overflow in the int64_t?
Huh. I have no idea how that got there. We should only need the other 2 checks. 

Can you take it out?


Repository:
  rL LLVM

https://reviews.llvm.org/D53776