[PATCH] D29400: [ImplicitNullCheck] Extend Implicit Null Check scope by using stores

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

Thank you, for review. I will update a change.



================
Comment at: include/llvm/CodeGen/FaultMaps.h:29
 public:
-  enum FaultKind { FaultingLoad = 1, FaultKindMax };
+  enum FaultKind { FaultingLoad = 1, FaultingStore, FaultKindMax };
 
----------------
sanjoy wrote:
> I found the earlier classification system better, with a third `FaultingLoadStore` category.  Right now we don't have a place to put `addq (%rax), %rbx` type instructions, which both load and store.
I considered that for that kind of instruction we first read and failed on this :) But ok, I will return it back.


================
Comment at: lib/CodeGen/ImplicitNullChecks.cpp:308
+
+  SeenLoad = SeenLoad || MI.mayLoad();
+
----------------
sanjoy wrote:
> How about `SeenLoad |= MI.mayLoad()`?
Sanjoy, as I understand in my case mayLoad will be einvoked only if SeenLoad == false, in |= form mayLoad will be invoked always and there is no ||= operator.


https://reviews.llvm.org/D29400