[PATCH] D87148: [ImplicitNullCheck] Handle Nonzero faulting pages and complex addressing

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: llvm/lib/CodeGen/ImplicitNullChecks.cpp:421
+      Scale != 0 && IndexRegUsedInConstAddr.first &&
+      IndexRegUsedInConstAddr.first != PointerReg;
 
----------------
dantrushin wrote:
> Do you really need to check that `BaseRegUsedInConstAddr.first` is not zero?
> Would simple `BaseRegUsedInConstAddr.first != PointerReg` work here?
> 
> I also found positive names more readable, e.g. `bool BaseRegIsNullChecked = BaseRegUsedInConstAddr.first == PointerReg`
> 
> I think it is reasonable to assert `Scale !=0 || IndexRegUsedInConstAddr.first == 0` and remove Scale check.
Changed the names and hopefully more readable. 
We need to check that the register exists before checking if it is equal to the null checked register because we don't want to fail when we don't have the register (in that case, we wont be using it in the addres calculation). Here we will state that the memory op is unsuitable and then fail. I realized this through some failing test case `imp_null_check_load` in implicit-null-checks.ll. There, we do not use the index register, so it is zero. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87148/new/

https://reviews.llvm.org/D87148