[LLVMdev] patch for llc/ARM: added mechanism to move switch tables from .text -> .data; also cleanup and documentation
robert at muth.org
Thu Jul 23 12:10:11 PDT 2009
On Tue, Jul 14, 2009 at 6:48 PM, Bob Wilson <bob.wilson at apple.com> wrote:
> On Jul 2, 2009, at 10:48 AM, robert muth wrote:
> I spend over a day trying to follow your suggestion. In the end I was not
> successful. Here is what Iearned:
> After setting
> ARMJITInfo::hasCustomJumpTables -> true
> setOperationAction for ISD::BR_JT -> Expand
> I needed to add a "brind" definition to ARMInstrInfo.td
> I picked "bx" but to do a proper job one would have to take older
> into account..
> The next thing was to to set
> setOperationAction for ISD::JumpTable -> Custom
> and implement the corresponding code.
> This code is very similar to my "outline" code as it has to materialize
> the table start address.
> I never quite got this to work, though.
> Sorry for the slow response. Since it wasn't clear what problems you ran
> into, and since I wasn't very familiar with LLVM's handling of jump tables,
> I wanted to have a look at it myself to see what the issues were.
> I was able to make it work after fixing one problem: the branch folder
> removes "dead" jump tables and it doesn't scan the constant pool for jump
> table references. I had to add some code to the branch folder and the
> MachineConstantPoolValue class to fix that. I haven't yet done much testing
> of this but it looks like it's basically working.
> Since I had to write the code to try this out, I'm sending my revised patch
> back to you. There are still a few issues to sort out before committing
> * I followed your lead and added a "brind" pattern using "bx". I think
> this works on older architectures, too. What issue were you thinking of
> with regard to supporting older architectures? I didn't even look at Thumb
> or Thumb2, but we'll need something for them.
> * I saw some earlier discussion about the command-line option name. You
> had "outline-jumptables" and Evan had suggested "OutOfLine". I don't
> particularly like either one (sorry, Evan). How about
> "no-inline-jumptables"? I prefer that because it captures the sense that
> this option is disabling an ARM-specific feature (the inline jumptables) and
> changing back to the default.
> * I didn't look at the JIT code emitter at all. That will need to be
> * It would be great to change the testcase to use the new FileCheck format.
> I've attached my revised patch to show you how I made this work. I've also
> attached a copy of your last patch with embedded comments (search for
> "bob>"). Let me know what you think and if you can take a look at
> addressing some of the open issues above.
> Presumably what the generic code generator
> would to is to use this table start multiple the "switch index" by 4 load
> the target address and jump there. Not a very big saving in my mind.
> And it is offset by having to touch ARMInstrInfo.td.
> Also note, that the generic code generator must make some assumptions
> about the format of each jumptable entry (e.g. 32bit absolute addresses) so
> this cannot be changed easily afterwards.
> Having the default jumtables and the out of line version diverge too much
> and have different code path may also not be such a good idea.
> I'm not so much concerned about savings in the quantity of code as I am in
> keeping the ARM backend from diverging too far from the other LLVM backends.
> Sometimes we have to do that but it is an ongoing maintenance
> burden, and in this case, it seems like we can easily avoid it.
> It also helps leverage the existing code. For example, I didn't look too closely, but with my patch, compiling with -relocation-model=pic seemed to do the right thing.
>> Why did you change the default value for JumpTableDataSection? Jump
>> tables really should not go into .data. That is a security hole. They
>> should be read-only.
> I changed that too so the data goes into the default .rodata now.
> I am not sure where .rodata lives. if it lives in the text *segment*
> you get better protection but the code wont be PIC if .rodata
> goes into the data segment it is the other way round.
> If you compile with -relocation-model=pic, then the jump table contains
> offsets relative to the start of the table. The tables live in .rodata (or
> even in .text itself, but not inline in the code) which is in the same
> segment as .text and thus the code is still PIC.
Thanks for cleaning this up. I like the new patch much better than the old
Teaching the (abstract) ConstantValue class about jumptable indices is a
bit ugly but I do not see any better solution without massive refactoring.
I have added TODOs here and elsewhere and plan to address them in future
patches. I also added a test and pcleaned up a few things.
The new patch is attached.
I also uploaded it to
which is more convenient for browsing.
(you will have to register with an arbitrary pair
of email address and password --if you have a gmail
account you can use that)
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
-------------- next part --------------
An HTML attachment was scrubbed...
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 16583 bytes
Desc: not available
More information about the llvm-dev