[PATCH] D32564: AArch64: compress jump tables to minimum size needed to reach destinations

[JF Bastien via Phabricator via llvm-commits]

jfb added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:466
+
+    for (unsigned ii = 0, ee = JTBBs.size(); ii != ee; ++ii)
+      emitJumpTableEntry(MJTI, JTBBs[ii], JTI);
----------------
Range-based for loop here.


================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:489
+    Value = MCBinaryExpr::createDiv(
+        Value, MCConstantExpr::create(4, OutContext), OutContext);
+  }
----------------
Make it a `shr` here, since that's what you'll want to emit anyways?


================
Comment at: llvm/lib/Target/AArch64/AArch64CompressJumpTables.cpp:115
+  auto AFI = MF->getInfo<AArch64FunctionInfo>();
+  if (isInt<8>(MaxOffset / 4) && isInt<8>(MinOffset / 4)) {
+    AFI->setJumpTableEntryInfo(JTIdx, 1, getJumpTablePICSymbol(JTIdx));
----------------
Are there ever cases where min is large, but the range [min, max) would fit within 8 or 16 bits? I'm wondering if you want another optimization where you pack the jump offsets, and always add min (here you're using a base of 0).


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:108
   case AArch64::TLSDESC_CALLSEQ:
     // This gets lowered to an instruction sequence which takes 16 bytes
     NumBytes = 16;
----------------
lol this comment


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:79
+
+
 static cl::opt<bool>
----------------
drop this


Repository:
  rL LLVM

https://reviews.llvm.org/D32564