Summary: | [JSC] Lower switch statements at LLVM IR level when using FastISel. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Juergen Ributzka <juergen> | ||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED DUPLICATE | ||||||
Severity: | Normal | CC: | fpizlo | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Juergen Ributzka
2014-11-07 15:04:19 PST
Created attachment 241210 [details]
Patch
Comment on attachment 241210 [details]
Patch
r=me
Is there any downside to lowering early? Does this prevent lookup-table-based switch?
Comment on attachment 241210 [details]
Patch
....Hmmm... looks like a missing #include?:
/Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/library/LLVMExports.cpp:103:32: error: use of undeclared identifier 'LLVMAddLowerSwitchPass'; did you mean 'LLVMAddLoopUnswitchPass'?
FOR_EACH_LLVM_API_FUNCTION(LLVM_API_FUNCTION_ASSIGNMENT);
^
In file included from /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/library/LLVMExports.cpp:30:
In file included from /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMAPI.h:31:
/Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:636:5: note: expanded from macro 'FOR_EACH_LLVM_API_FUNCTION'
macro(void, AddLowerSwitchPass, (LLVMPassManagerRef PM)) \
^
/Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/library/LLVMExports.cpp:102:20: note: expanded from macro 'LLVM_API_FUNCTION_ASSIGNMENT'
result->name = LLVM##name;
^
<scratch space>:10:1: note: expanded from here
LLVMAddLowerSwitchPass
^
In file included from /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/library/LLVMExports.cpp:30:
In file included from /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMAPI.h:31:
In file included from /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:29:
In file included from /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMHeaders.h:56:
/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/LLVMForJavaScriptCore/include/llvm-c/Transforms/Scalar.h:78:6: note: 'LLVMAddLoopUnswitchPass' declared here
(In reply to comment #2) > Comment on attachment 241210 [details] > Patch > > r=me > > Is there any downside to lowering early? Does this prevent > lookup-table-based switch? Yes, it prevents lookup-table-based switches, but FastISel isn't supporting them anyways. I haven't seen a performance regression yet, but we can revisit this if it should become a problem. The important thing here is that you don't want to fall-back to SelectionDAG, which would increase compile time. It works on my machines with the latest headers. I guess the llvm src code is not up-to-date on that machine. From where do you get the code - from the LLVM drops? (In reply to comment #3) > Comment on attachment 241210 [details] > Patch > > ....Hmmm... looks like a missing #include?: > > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/library/LLVMExports.cpp: > 103:32: error: use of undeclared identifier 'LLVMAddLowerSwitchPass'; did > you mean 'LLVMAddLoopUnswitchPass'? > FOR_EACH_LLVM_API_FUNCTION(LLVM_API_FUNCTION_ASSIGNMENT); > ^ > In file included from > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/library/LLVMExports.cpp: > 30: > In file included from > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMAPI.h:31: > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:636:5: > note: expanded from macro 'FOR_EACH_LLVM_API_FUNCTION' > macro(void, AddLowerSwitchPass, (LLVMPassManagerRef PM)) \ > ^ > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/library/LLVMExports.cpp: > 102:20: note: expanded from macro 'LLVM_API_FUNCTION_ASSIGNMENT' > result->name = LLVM##name; > ^ > <scratch space>:10:1: note: expanded from here > LLVMAddLowerSwitchPass > ^ > In file included from > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/library/LLVMExports.cpp: > 30: > In file included from > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMAPI.h:31: > In file included from > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:29: > In file included from > /Volumes/Data/EWS/WebKit/Source/JavaScriptCore/llvm/LLVMHeaders.h:56: > /Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/LLVMForJavaScriptCore/ > include/llvm-c/Transforms/Scalar.h:78:6: note: 'LLVMAddLoopUnswitchPass' > declared here Comment on attachment 241210 [details]
Patch
R=me if it builds. ;-)
(In reply to comment #6) > Comment on attachment 241210 [details] > Patch > > R=me if it builds. ;-) It does build with LLVM trunk. The C API for the switch lowering pass was added after the LLVM drops where made, so that is why this is failing on the buildbots. Comment on attachment 241210 [details]
Patch
Let's come up with a way to adopt this API conditionally that doesn't break the build or harm performance when using older versions of LLVM.
*** This bug has been marked as a duplicate of bug 143489 *** |