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

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 23 06:29:40 PDT 2019


SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

looks good to me to, just some nits inline.



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


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:13139
+
+  assert((IsTrueIfZero(CC, Imm) || IsFalseIfZero(CC, Imm)) &&
+         "unsupported condition");
----------------
I was wondering if we can return here; not sure though if that is better/more robust.


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


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

https://reviews.llvm.org/D64616





More information about the llvm-commits mailing list