[PATCH] D32281: [ARM] ACLE Chapter 9 intrinsics

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I'm not convinced you're handling the Q and GE flags correctly for all of the intrinsics which set/use them.  If we allow users to access those flags, every instruction which might set them needs to be mapped to a SelectionDAG node with a chain, every instruction which sets or uses them needs to be marked hasSideEffects, and we can never turn arithmetic into an operation which sets Q or GE.  I'm not convinced you've managed that correctly everywhere.  We currently have an ARMISD::SSAT node which doesn't have a chain.  We have a pattern which generates SMLABB from an ISD::MUL.  And it's difficult to review the rest because you're adding them all at once.



================
Comment at: include/llvm/IR/IntrinsicsARM.td:87
+def int_arm_sel : GCCBuiltin<"__builtin_arm_sel">,
+    Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty], []>;
+
----------------
IntrReadMem?


================
Comment at: lib/Target/ARM/ARMInstrInfo.td:877
+    return Imm > 0 && Imm <= 16;
+  }],
     imm1_16_XFORM> {
----------------
ImmLeaf?


https://reviews.llvm.org/D32281