On Mon, Dec 22, 2014 at 12:55 PM, Chris Bieneman <beanz at apple.com> wrote:> > On Dec 22, 2014, at 12:02 PM, Chandler Carruth <chandlerc at google.com> > wrote: > > > On Fri, Dec 19, 2014 at 3:39 PM, Chris Bieneman <beanz at apple.com> wrote: > >> More diffs to enjoy. >> >> I’ve updated my tablegen patches to work with the clang::impossible_enum >> attribute. With these changes I am seeing the same code size regressions >> that #ifdefs were showing — without all the ifdefs. >> >> That’s about a 2% decrease in our library for WebKit. >> >> See attached patches for llvm & clang. >> > > FWIW, I still strongly object to this entire approach. > > If we want to add support for this magical attribute to clang, fine, but > honestly I don't think it's that relevant for the issue at hand. > > The critical question to my mind is why are intrinsics taking up so much > space. I think it would be much more profitable to make intrinsics compact > than to add hacks to LLVM to exclude target intrinsics entirely. Especially > based on a fairly subtle and magical enum attribute. It changes too much of > the meaning of enums. > > > The vast majority of the space savings comes from reduced code size (not > constant data), which was somewhat surprising to me at first, but in > retrospect makes a lot of sense. In particular the TableGen-generated code > that parses intrinsic names to resolve strings->ids, is pretty massive. >Without having looked at the code in question, could we maybe just have a binary search on a table? (possibly an existing table?) -- Sean Silva> > > For example, what happens when an "impossible" enum is used to compute the > value of some other enum? > > > This could be un-affected. My clang changes only impact when impossible > enums are used in conditional statements and switch statements. That aside, > in our specific use I’d be pretty terrified if we’re using the value of > intrinsic ids to compute other intrinsic ids because I don’t believe we > have any guarantee of the ordering or values of intrinsic ids. From looking > at the TableGen backend it looks to me like the id is really just the index > into the records list, which is constructed in order of the parsing, and > those indexes are used all over the place and assumed to match. > > At what number do you start up numbering those enums listed after an > impossible enum? I see no easy, obvious, and unsurprising answers here. I > don't want to have to think this much about something that should be > *extremely* basic. > > > The way I handled numbering in my patch was to make the impossible enums > immediately follow the last possible enum, and it all worked fine. > > > If the target-independent optimizer is being bloated by code to handle > optimizing target-specific intrinsics, we should look at factoring such > code into an IR-level transform library in the target that can be inserted > into the optimizer pass pipeline like anything else. But I have my doubts > that this is the cause. > > If the target-specific intrinsics are carrying a huge amount of code in > the core IR library in order to support parsing, serializing, printing, > etc., we should really find a more compact way of doing this. I cannot > fathom how this could require 500kb of data (or text) to handle. We're > doing something deeply wrong with intrinsics, and I think it is a mistake > to pursue anything until we understand that. > > > I understand all your points here. Independent of whether or not this is > the right approach for handling intrinsics I’m going to clean-up and > propose my patches to clang to add the attribute because it does seem > useful to me. > > -Chris > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141222/6bd94bab/attachment.html>
A binary search on a table would be possible, but we’d need to generate a table that was sorted. Today the order of intrinsics is really just the order that TableGen parses them in. -Chris> On Dec 22, 2014, at 1:34 PM, Sean Silva <chisophugis at gmail.com> wrote: > > > > On Mon, Dec 22, 2014 at 12:55 PM, Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> wrote: > >> On Dec 22, 2014, at 12:02 PM, Chandler Carruth <chandlerc at google.com <mailto:chandlerc at google.com>> wrote: >> >> >> On Fri, Dec 19, 2014 at 3:39 PM, Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> wrote: >> More diffs to enjoy. >> >> I’ve updated my tablegen patches to work with the clang::impossible_enum attribute. With these changes I am seeing the same code size regressions that #ifdefs were showing — without all the ifdefs. >> >> That’s about a 2% decrease in our library for WebKit. >> >> See attached patches for llvm & clang. >> >> FWIW, I still strongly object to this entire approach. >> >> If we want to add support for this magical attribute to clang, fine, but honestly I don't think it's that relevant for the issue at hand. >> >> The critical question to my mind is why are intrinsics taking up so much space. I think it would be much more profitable to make intrinsics compact than to add hacks to LLVM to exclude target intrinsics entirely. Especially based on a fairly subtle and magical enum attribute. It changes too much of the meaning of enums. > > The vast majority of the space savings comes from reduced code size (not constant data), which was somewhat surprising to me at first, but in retrospect makes a lot of sense. In particular the TableGen-generated code that parses intrinsic names to resolve strings->ids, is pretty massive. > > Without having looked at the code in question, could we maybe just have a binary search on a table? (possibly an existing table?) > > -- Sean Silva > > >> >> For example, what happens when an "impossible" enum is used to compute the value of some other enum? > > This could be un-affected. My clang changes only impact when impossible enums are used in conditional statements and switch statements. That aside, in our specific use I’d be pretty terrified if we’re using the value of intrinsic ids to compute other intrinsic ids because I don’t believe we have any guarantee of the ordering or values of intrinsic ids. From looking at the TableGen backend it looks to me like the id is really just the index into the records list, which is constructed in order of the parsing, and those indexes are used all over the place and assumed to match. > >> At what number do you start up numbering those enums listed after an impossible enum? I see no easy, obvious, and unsurprising answers here. I don't want to have to think this much about something that should be *extremely* basic. > > The way I handled numbering in my patch was to make the impossible enums immediately follow the last possible enum, and it all worked fine. > >> >> If the target-independent optimizer is being bloated by code to handle optimizing target-specific intrinsics, we should look at factoring such code into an IR-level transform library in the target that can be inserted into the optimizer pass pipeline like anything else. But I have my doubts that this is the cause. >> >> If the target-specific intrinsics are carrying a huge amount of code in the core IR library in order to support parsing, serializing, printing, etc., we should really find a more compact way of doing this. I cannot fathom how this could require 500kb of data (or text) to handle. We're doing something deeply wrong with intrinsics, and I think it is a mistake to pursue anything until we understand that. > > I understand all your points here. Independent of whether or not this is the right approach for handling intrinsics I’m going to clean-up and propose my patches to clang to add the attribute because it does seem useful to me. > > -Chris > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu> http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141222/1e410318/attachment.html>
Circling back to Chandler on file size differences. Here are the highlights of what is different. For my analysis I built LLVM and Clang using a clang built with my patches. The same clang was used for the baseline and the stripped build. I used the following CMake command: cmake -G "Sublime Text 2 - Ninja" -DCMAKE_BUILD_TYPE=Release -DLLVM_BUILD_LLVM_DYLIB=Yes -DLLVM_DISABLE_LLVM_DYLIB_ATEXIT=On -DCMAKE_INSTALL_PREFIX=/Users/cbieneman/dev/llvm-install/ -DLLVM_TARGETS_TO_BUILD="AArch64;ARM;X86" ../llvm and built using Ninja. Created a fresh build directory and built once as a baseline from the following revisions: LLVM - ba05946 lld - 33bd1dc clang - 1589d90 (With my patches applied) I then applied my tablegen and CMake patches, made a new build directory, and built a second time. I then compared the file sizes between the two directories by diffing the output of: find . -type f -exec stat -f '%N %z' '{}' + | sort The biggest benefits are an 11% reduction in size for libLLVMCore, which is mostly due to Function.cpp.o reducing in size by 300KB (almost 39%). The biggest thing in there that would contribute to actual code size is the almost 28,000 line switch statement that provides the implementation for Function::lookupIntrinsicID. I’ve attached a CSV file with the file size highlights for anyone who is interested. -Chris> On Dec 22, 2014, at 1:39 PM, Chris Bieneman <beanz at apple.com> wrote: > > A binary search on a table would be possible, but we’d need to generate a table that was sorted. Today the order of intrinsics is really just the order that TableGen parses them in. > > -Chris > >> On Dec 22, 2014, at 1:34 PM, Sean Silva <chisophugis at gmail.com <mailto:chisophugis at gmail.com>> wrote: >> >> >> >> On Mon, Dec 22, 2014 at 12:55 PM, Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> wrote: >> >>> On Dec 22, 2014, at 12:02 PM, Chandler Carruth <chandlerc at google.com <mailto:chandlerc at google.com>> wrote: >>> >>> >>> On Fri, Dec 19, 2014 at 3:39 PM, Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> wrote: >>> More diffs to enjoy. >>> >>> I’ve updated my tablegen patches to work with the clang::impossible_enum attribute. With these changes I am seeing the same code size regressions that #ifdefs were showing — without all the ifdefs. >>> >>> That’s about a 2% decrease in our library for WebKit. >>> >>> See attached patches for llvm & clang. >>> >>> FWIW, I still strongly object to this entire approach. >>> >>> If we want to add support for this magical attribute to clang, fine, but honestly I don't think it's that relevant for the issue at hand. >>> >>> The critical question to my mind is why are intrinsics taking up so much space. I think it would be much more profitable to make intrinsics compact than to add hacks to LLVM to exclude target intrinsics entirely. Especially based on a fairly subtle and magical enum attribute. It changes too much of the meaning of enums. >> >> The vast majority of the space savings comes from reduced code size (not constant data), which was somewhat surprising to me at first, but in retrospect makes a lot of sense. In particular the TableGen-generated code that parses intrinsic names to resolve strings->ids, is pretty massive. >> >> Without having looked at the code in question, could we maybe just have a binary search on a table? (possibly an existing table?) >> >> -- Sean Silva >> >> >>> >>> For example, what happens when an "impossible" enum is used to compute the value of some other enum? >> >> This could be un-affected. My clang changes only impact when impossible enums are used in conditional statements and switch statements. That aside, in our specific use I’d be pretty terrified if we’re using the value of intrinsic ids to compute other intrinsic ids because I don’t believe we have any guarantee of the ordering or values of intrinsic ids. From looking at the TableGen backend it looks to me like the id is really just the index into the records list, which is constructed in order of the parsing, and those indexes are used all over the place and assumed to match. >> >>> At what number do you start up numbering those enums listed after an impossible enum? I see no easy, obvious, and unsurprising answers here. I don't want to have to think this much about something that should be *extremely* basic. >> >> The way I handled numbering in my patch was to make the impossible enums immediately follow the last possible enum, and it all worked fine. >> >>> >>> If the target-independent optimizer is being bloated by code to handle optimizing target-specific intrinsics, we should look at factoring such code into an IR-level transform library in the target that can be inserted into the optimizer pass pipeline like anything else. But I have my doubts that this is the cause. >>> >>> If the target-specific intrinsics are carrying a huge amount of code in the core IR library in order to support parsing, serializing, printing, etc., we should really find a more compact way of doing this. I cannot fathom how this could require 500kb of data (or text) to handle. We're doing something deeply wrong with intrinsics, and I think it is a mistake to pursue anything until we understand that. >> >> I understand all your points here. Independent of whether or not this is the right approach for handling intrinsics I’m going to clean-up and propose my patches to clang to add the attribute because it does seem useful to me. >> >> -Chris >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu> http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev> > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141222/78f6937d/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: Intrinsic Stripping.csv Type: text/csv Size: 700 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141222/78f6937d/attachment.csv> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141222/78f6937d/attachment-0001.html>