[PATCH] D64616: [ARM][LowOverheadLoops] Fix branch target codegen

[Sam Parker via Phabricator via llvm-commits]

samparker marked 3 inline comments as done.
samparker added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13105
+  } else {
+    CC = cast<CondCodeSDNode>(N->getOperand(1))->get();
+    Cond = N->getOperand(2);
----------------
SjoerdMeijer wrote:
> perhaps an assert we're expecting a BR_CC here?
sure.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13139
+
+  assert((IsTrueIfZero(CC, Imm) || IsFalseIfZero(CC, Imm)) &&
+         "unsupported condition");
----------------
SjoerdMeijer wrote:
> I was wondering if we can return here; not sure though if that is better/more robust.
At this point we know we have a loop intrinsic that we need to handle it.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13172
+    } else
+      llvm_unreachable("unhandled condition");
+    DAG.ReplaceAllUsesOfValueWith(Int.getValue(1), Int.getOperand(0));
----------------
SjoerdMeijer wrote:
> but in that case we don't need this unreachable.
good point.


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

https://reviews.llvm.org/D64616