[PATCH] D17154: Bug fix: use dyn_cast_or_null instead of dyn_cast

[Chad Rosier via llvm-commits]

mcrosier added a comment.

Can you please cleanup the test case to be more human readable?

Is the test just verifying that we don't get a compiler ICE due to dereferencing a nullptr?  If so, please add a comment to the test case expressing this.


================
Comment at: lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp:829
@@ -829,2 +828,3 @@
+  GetElementPtrInst *SecondGEP = dyn_cast_or_null<GetElementPtrInst>(ResultPtr);
   if (isSwapCandidate && isLegalToSwapOperand(FirstGEP, SecondGEP, L))
     swapGEPOperand(FirstGEP, SecondGEP);
----------------
hulx2000 wrote:
> Thanks, Chad:
> 
> That's not needed because I checked in isLegalToSwapOperand().
Okay, that's fine.

================
Comment at: test/CodeGen/AArch64/gep-nullptr.ll:10
@@ +9,3 @@
+; Function Attrs: argmemonly nounwind
+declare void @llvm.lifetime.end(i64, i8* nocapture)
+
----------------
Is the lifetime intrinsic relevant to the test?

================
Comment at: test/CodeGen/AArch64/gep-nullptr.ll:17
@@ +16,3 @@
+
+; CHECK:  .text
+for.body13.us:                                    ; preds = %entry
----------------
You should probably use a CHECK-LABEL directive like

; CHECK-LABEL: test


Repository:
  rL LLVM

http://reviews.llvm.org/D17154