[LLVMdev] patch for llc/ARM: added mechanism to move switch tables from .text -> .data; also cleanup and documentation
bob.wilson at apple.com
Mon Jul 27 11:24:54 PDT 2009
On Jul 23, 2009, at 12:10 PM, robert muth wrote:
> Thanks for cleaning this up. I like the new patch much better than
> the old one.
> Teaching the (abstract) ConstantValue class about jumptable indices
> is a little
> bit ugly but I do not see any better solution without massive
> I have added TODOs here and elsewhere and plan to address them in
> patches. I also added a test and pcleaned up a few things.
Maybe I'm missing something but it looks like this is basically the
same as the sample code that I sent back to you. Aside from the TODO
comments, it looks like all you changed was to move the new code in
BranchFolding into a new UpdateLiveSetUsingConstantPoolAndMapIndices
function, and I don't particularly like that change -- the new code is
not that big and it is essentially the same code as in the preceding
loop. If anything, maybe it would be possible to refactor those two
loops to share some code? I don't see the point in just moving one of
them out to a separate function with a big name.
As I wrote before, there are a few things that need to be addressed
before we can take the patch. I'll repeat them here for clarity:
* It needs to work on Thumb and Thumb2. I'm not even sure if the
"brind" pattern I added is the right thing for ARM mode.
* The JIT code emitter is definitely broken with this. Unless there
is a fundamental reason why this is hard, it should be fixed.
* The testcase should use the new FileCheck format. (I think I
phrased this as a suggestion before, but upon looking more closely at
the test, it seems like the sort of test that is not well-suited to
You didn't respond to my comment about the command-line option name.
What do you think of "no-inline-jumptables"?
How do you plan to test this? The simple testcase is a good start.
Beyond that, I'd like to see you (temporarily) enable this by default
and run through the llvm testsuite.
I also see you added a comment: "TODO: this does not currently work
for PIC mode: the code works correctly but the table still ends up in
the .text section." I don't remember seeing that behavior, but maybe
I missed it. Please investigate why this is happening.
More information about the llvm-dev