[PATCH] ARM: allow ConstantIslands to handle jump tables

Pete Cooper peter_cooper at apple.com
Fri May 15 11:01:21 PDT 2015


Hey Tim.

A bunch of nitpicking, but nothing that should block committing.  LGTM.

Cheers,
Pete


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:1014
@@ +1013,3 @@
+
+  for (unsigned i = 0, e = JTBBs.size(); i != e; ++i) {
+    MachineBasicBlock *MBB = JTBBs[i];
----------------
This can be a foreach.

================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:516
@@ -497,3 +515,3 @@
 
-/// doInitialPlacement - Perform the initial placement of the constant pool
-/// entries.  To start with, we put them all at the end of the function.
+/// doInitialConstPlacement - Perform the initial placement of the regular
+/// constant pool entries.  To start with, we put them all at the end of the
----------------
Can you change this to \brief as you're changing these lines anyway.

================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:578
@@ -558,1 +577,3 @@
 
+/// Do initial placement of the jump tables. Because Thumb2's TBB and TBH
+/// instructions can be made more efficient if the jump table immediately
----------------
And use \brief here too.

================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:617
@@ +616,3 @@
+      MI->getOperand(NumOps - (MI->isPredicable() ? 2 : 1));
+    int JTI = JTOp.getIndex();
+    unsigned Size = JT[JTI].MBBs.size() * sizeof(uint32_t);
----------------
You do unsigned for JTI earlier in the patch.  Would unsigned be better here?

================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:2028
@@ -1919,1 +2027,3 @@
 
+/// Returns whether CPEMI is the first instruction in the block immediately
+/// following JTMI (assumed to be a TBB or TBH terminator). If so, we can switch
----------------
\brief here too.

http://reviews.llvm.org/D9796

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






More information about the llvm-commits mailing list