[PATCH] D87108: [ImplicitNullCheck] Handle instructions that do not modify null behaviour of null checked reg

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetInstrInfo.h:1276
+  /// NullValueReg = rax.
+  virtual bool isNullBehaviourUnchanged(const MachineInstr *MI,
+                                        const Register NullValueReg,
----------------
reames wrote:
> minor name change: preservesZeroReg(...)
> 
> Null is simply zero in MI land, and we don't want to imply there's anything special about the pointer interpretation here.
> 
> I'd also suggest being really explicit in the comment that the register can become null.  This function only returns whether a zero is guaranteed to remain a zero.    
I'd keep the name as `preservesZeroValueInReg` because "zero reg" could be confusing to mean "NoRegister" which is 0 for all targets.



================
Comment at: llvm/test/CodeGen/X86/implicit-null-check.ll:595
+; Same as imp_null_check_load_shift_addr but shift is by 3 and this is now
+; converted into an LEA.
+; TODO: Can be converted into implicit null check
----------------
reames wrote:
> not an LEA - I think you meant complex addressing?  Or maybe the LEA gets folded late?
> 
> Regardless, the comment is out of sync with tested output.
ah yes. I meant complex addressing :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87108