[PATCH] [LLVM - ARM/AArch64] Add ACLE special register intrinsics (10.1)

Renato Golin renato.golin at linaro.org
Tue May 12 09:16:30 PDT 2015


Hi Luke,

Thanks for the changes. I think the two main functions to lower read/write register calls need some cleaning (see inline), but the tests are good.

cheers,
--renato


================
Comment at: lib/Target/ARM/ARMISelDAGToDAG.cpp:3360
@@ +3359,3 @@
+  return StringSwitch<int>(RegString.lower())
+           .Case("r8_usr", 0x00)
+           .Case("r9_usr", 0x01)
----------------
Aren't there ARM::* register enums for those? You seem to be using a similar technique for VFP registers below, why can't you do the same for these?

================
Comment at: lib/Target/ARM/ARMISelDAGToDAG.cpp:3505
@@ +3504,3 @@
+
+  if (SpecialReg == "apsr" || SpecialReg == "cpsr") {
+    Ops = { getAL(CurDAG, DL), CurDAG->getRegister(0, MVT::i32) };
----------------
This block seems lost... and it's parsing the string again, when you already had it to enum up there. Maybe also add "cpsr" into the string switch and add a new switch to continue on specific cases and emit the call on default:


================
Comment at: lib/Target/ARM/ARMISelDAGToDAG.cpp:3522
@@ +3521,3 @@
+
+  if (SpecialReg == "spsr") {
+    Ops = { getAL(CurDAG, DL), CurDAG->getRegister(0, MVT::i32) };
----------------
This one as well.

Would be good if you could use ARM:: enum values for the register names here, too.

================
Comment at: lib/Target/ARM/ARMISelDAGToDAG.cpp:3534
@@ +3533,3 @@
+// and the registers/masks to use in the nodes
+SDNode *ARMDAGToDAGISel::SelectWriteRegister(SDNode *N){
+  const MDNodeSDNode *MD = dyn_cast<MDNodeSDNode>(N->getOperand(1));
----------------
Same things can be said about this function, as the one above.

In addition, there are too many nested blocks here, with a lot of magic constants. Would be good to split these into smaller functions, static if necessary, that deal with the gory details of masks and register names/IDs.

http://reviews.llvm.org/D9699

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list