[PATCH] Add a jumptable attribute and support for creating jump-instruction tables

[Nick Lewycky]

On 14 May 2014 11:57, Tom Roeder <tmroeder at google.com> wrote:

> This diff fixes the problems noted in the review. PTAL.
>
> For the for-loop invalidation problem, I didn't see any way to increment
> the implicit iterator early in C++11 style, so I've gone back to a standard
> iterator for loop to match the style of other invalidation-sensitive
> Use-handling loops.
>
> Also, as I was working on these fixes, I found a problem with GlobalAlias:
> it doesn't support replaceUsesOfWithOnConstant, and it cannot target
> declaration-only functions. So, I've added some special-case handling for
> GlobalAlias statements that target jumptable functions. I've also added a
> new test case for "alias".
>

Rafael, could you take a look at how this interacts with global aliases?

+//===- JumpInstrTableInfo.h: Info for Jump-Instruction Tables -*- C++
-*---===//
...
+//===----- JumpInstrTables.h: Jump-Instruction Tables -*- C++
-*-----------===//

Please right-align the emacs major mode marker.

+  virtual ~JumpInstrTables() { }

Move the implementation into a .cpp file. In this case, the destructor is
the key method which means that it controls which TUs will contain the
functions and debug info. Putting it in the header file means that all
inclusions of it will contain copies, but if you move it to the .cpp then
we get one copy in that .cpp.

+JumpInstrTableInfo::JumpInstrTableInfo()
+  : ImmutablePass(ID),
+  Tables() {

Formatting? Tables() should be on the previous line? In fact, doesn't
  JumpInstrTableInfo::JumpInstrTableInfo() : ImmutablePass(ID), Tables() {
fit on one line?

+//===- JumpInstrTables.cpp: Jump-Instruction Tables -*- C++
-*-------------===//

Right-align the -*- again.

+  Value::use_iterator I, E;
+  for (I = GV->use_begin(), E = GV->use_end(); I != E;) {

Any reason that isn't:
  for (Value::use_iterator I = GV->use_begin(), E = GV->use_end(); I != E;)
{
?

+JumpInstrTables::JumpInstrTables()
+  : ModulePass(ID),
+  Metadata(),
+  JITI(nullptr),
+  TableCount(0) {

Again, should be packed formatting here. You might want to try clang-format
--style=LLVM?


This is missing a change to include/llvm-c/Core.h in the commented out
section of LLVMAttribute.

I'd also appreciate someone from the backend side looking at this.

Nick


> http://reviews.llvm.org/D3361
>
> Files:
>   docs/LangRef.rst
>   include/llvm/Analysis/JumpInstrTableInfo.h
>   include/llvm/Analysis/Passes.h
>   include/llvm/Bitcode/LLVMBitCodes.h
>   include/llvm/CodeGen/JumpInstrTables.h
>   include/llvm/CodeGen/Passes.h
>   include/llvm/IR/Attributes.h
>   include/llvm/InitializePasses.h
>   include/llvm/LinkAllPasses.h
>   include/llvm/Target/TargetInstrInfo.h
>   lib/Analysis/Analysis.cpp
>   lib/Analysis/CMakeLists.txt
>   lib/Analysis/JumpInstrTableInfo.cpp
>   lib/AsmParser/LLLexer.cpp
>   lib/AsmParser/LLParser.cpp
>   lib/AsmParser/LLToken.h
>   lib/Bitcode/Reader/BitcodeReader.cpp
>   lib/Bitcode/Writer/BitcodeWriter.cpp
>   lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>   lib/CodeGen/CMakeLists.txt
>   lib/CodeGen/JumpInstrTables.cpp
>   lib/CodeGen/LLVMTargetMachine.cpp
>   lib/IR/Attributes.cpp
>   lib/IR/Verifier.cpp
>   lib/LTO/LTOCodeGenerator.cpp
>   lib/Target/ARM/ARMBaseInstrInfo.cpp
>   lib/Target/ARM/ARMBaseInstrInfo.h
>   lib/Target/X86/X86InstrInfo.cpp
>   lib/Target/X86/X86InstrInfo.h
>   lib/Transforms/IPO/IPO.cpp
>   test/Bitcode/attributes.ll
>   test/CodeGen/ARM/jump_tables.ll
>   test/CodeGen/X86/jump_table_alias.ll
>   test/CodeGen/X86/jump_table_bitcast.ll
>   test/CodeGen/X86/jump_tables.ll
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140520/573e631e/attachment.html>