[PATCH] D154506: [ARM] in LowerConstantFP, make sure we cover armv6-m execute-only

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Thanks. Sounds good to me.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7073
   if (ST->genExecuteOnly()) {
+    // we shouldn't trigger this for v6m execute-only
+    if (ST->isThumb1Only() && !ST->hasV8MBaselineOps())
----------------
we -> We


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:7074-7075
+    // we shouldn't trigger this for v6m execute-only
+    if (ST->isThumb1Only() && !ST->hasV8MBaselineOps())
+      llvm_unreachable("Unexpected architecture");
+
----------------
Perhaps just use an assert instead of a llvm_unreachable? It's common to add the message to asserts in LLVM code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154506