[JSC] Lower switch statements at LLVM IR level when using FastISel.
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 ***