[PATCH] D21308: AArch64: Fix end iterator dereference

[John Brawn via llvm-commits]

john.brawn added a subscriber: john.brawn.
john.brawn added a comment.

It looks like this currently happens to not crash because ilist::end() points to a sentinel value which can be validly accessed. Relying on that implementation detail seems like a bad idea though, so this looks like a good idea.


================
Comment at: lib/Target/AArch64/AArch64BranchRelaxation.cpp:265
@@ -264,3 +264,3 @@
 /// specific BB can fit in MI's displacement field.
-bool AArch64BranchRelaxation::isBlockInRange(MachineInstr *MI,
+bool AArch64BranchRelaxation::isBlockInRange(MachineInstr &MI,
                                              MachineBasicBlock *DestBB,
----------------
Is there a particular reason you're changing this to take a reference instead of a pointer (and also getDestBlock below)? If you're doing it to emphasize "this pointer cannot be null" then it would make sense to do the same thing everywhere (i.e. in fixupConditionalBranch, invertBccCondition, etc.).

================
Comment at: lib/Target/AArch64/AArch64BranchRelaxation.cpp:472
@@ +471,3 @@
+
+    MachineInstr &MI = *J;
+    if (isConditionalBranch(MI.getOpcode()) &&
----------------
Making a reference only to then take the address of it when calling fixupConditionalBranch seems a little odd. I'd just stick with a pointer.


http://reviews.llvm.org/D21308