> On Dec 19, 2014, at 12:34 AM, Alex Rosenberg <alexr at leftfield.org> wrote: > > Putting aside the several minor bikesheds we will get to if we go this route, how close does this approach get to the code shrink you were originally trying to achieve?I haven't yet adjusted my tablegen changes to take advantage of the, but it is on my list for today.> > Are there any structures other than switch statements that need to go on a diet too? e.g. comparisons?My patches impact both case statements and comparisons. == compares are always false, any other compare is always true. -Chris> > Alex > >> On Dec 18, 2014, at 7:49 PM, Chris Bieneman <beanz at apple.com> wrote: >> >> Ok… I bit. >> >> Alex’s proposal here is really compelling to me because it means that the required changes to make this work would be more limited. Specifically a clang attribute could give us all the benefits of #ifdefs throughout the code without the maintenance burden. So, being the silly person I am, I wrote the patches for clang. >> >> I’ve never done any frontend hacking before, so take these with giant cellars of salt, but the concept seems sound to me. >> >> The patches do the following: >> (1) Add a new C++11-style [[impossible_enum]] attribute >> (2) Any case statement that has [[impossible_enum]] applied to it is not emitted during IRGen - the bodies are always emitted so as not to interfere with fall through, but blocks that cannot be entered are optimized away >> (3) Equality comparison against [[impossible_enum]] values are always false, all other comparisons are always true >> >> There was some discussion on IRC today whether or not this was the right way to do this, but I thought I’d send these patches out anyways so people can take a look. >> >> The attached diffs are for clang, I’ve also attached a c++ test file. >> >> -Chris >> <impossible_enum.cpp> >> <impossible_enum.diff> >> >>> On Dec 16, 2014, at 12:09 PM, Alex Rosenberg <alexr at ohmantics.com> wrote: >>> >>> Random shower thought: >>> >>> I think the markup can be minimized if it only appears once in the header where the enums are defined instead of at every place where the enums are used. Then we could value propagate that certain enum values are never possible where they're checked for. That should generally be able to strip the same set of stuff but use less markup. >>> >>> Alex >>> >>>> On Dec 10, 2014, at 3:53 PM, Chris Bieneman <beanz at apple.com> wrote: >>>> >>>> llvm-dev, >>>> >>>> In my ongoing saga to improve LLVM for embedded use, we would like to support stripping out unused intrinsics based on the LLVM targets actually being built. >>>> >>>> I’ve attached two patches. >>>> >>>> The first is a new flag for tablegen to take a list of targets. If passed tablegen will only emit intrinsics that either have empty target prefixes, or target prefixes matching one of the targets in the list. If the flag is not passed the behavior is unchanged. This patch can land today (subject to review). >>>> >>>> The second patch is a WIP, and adds support to the CMake build system for using the new tablegen flag, and for generating a new llvm/Config/llvm-targets.h header which contains defines for each target specified with LLVM_TARGETS_TO_BUILD. >>>> >>>> This new header will allow us to #ifdef code using target-specific intrinsics outside the targets, thus allowing us to strip out all the unused intrinsics. >>>> >>>> -Chris >>>> >>>> <cmake-build.diff><tablegen.diff>_______________________________________________ >>>> LLVM Developers mailing list >>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>>> 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/20141219/c0ce4d0d/attachment.html>
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. -Chris> On Dec 19, 2014, at 8:56 AM, Chris Bieneman <beanz at apple.com> wrote: > > > > > > On Dec 19, 2014, at 12:34 AM, Alex Rosenberg <alexr at leftfield.org <mailto:alexr at leftfield.org>> wrote: > >> Putting aside the several minor bikesheds we will get to if we go this route, how close does this approach get to the code shrink you were originally trying to achieve? > > I haven't yet adjusted my tablegen changes to take advantage of the, but it is on my list for today. > >> >> Are there any structures other than switch statements that need to go on a diet too? e.g. comparisons? > > My patches impact both case statements and comparisons. == compares are always false, any other compare is always true. > > -Chris > >> >> Alex >> >> On Dec 18, 2014, at 7:49 PM, Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> wrote: >> >>> Ok… I bit. >>> >>> Alex’s proposal here is really compelling to me because it means that the required changes to make this work would be more limited. Specifically a clang attribute could give us all the benefits of #ifdefs throughout the code without the maintenance burden. So, being the silly person I am, I wrote the patches for clang. >>> >>> I’ve never done any frontend hacking before, so take these with giant cellars of salt, but the concept seems sound to me. >>> >>> The patches do the following: >>> (1) Add a new C++11-style [[impossible_enum]] attribute >>> (2) Any case statement that has [[impossible_enum]] applied to it is not emitted during IRGen - the bodies are always emitted so as not to interfere with fall through, but blocks that cannot be entered are optimized away >>> (3) Equality comparison against [[impossible_enum]] values are always false, all other comparisons are always true >>> >>> There was some discussion on IRC today whether or not this was the right way to do this, but I thought I’d send these patches out anyways so people can take a look. >>> >>> The attached diffs are for clang, I’ve also attached a c++ test file. >>> >>> -Chris >>> <impossible_enum.cpp> >>> <impossible_enum.diff> >>> >>>> On Dec 16, 2014, at 12:09 PM, Alex Rosenberg <alexr at ohmantics.com <mailto:alexr at ohmantics.com>> wrote: >>>> >>>> Random shower thought: >>>> >>>> I think the markup can be minimized if it only appears once in the header where the enums are defined instead of at every place where the enums are used. Then we could value propagate that certain enum values are never possible where they're checked for. That should generally be able to strip the same set of stuff but use less markup. >>>> >>>> Alex >>>> >>>> On Dec 10, 2014, at 3:53 PM, Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> wrote: >>>> >>>>> llvm-dev, >>>>> >>>>> In my ongoing saga to improve LLVM for embedded use, we would like to support stripping out unused intrinsics based on the LLVM targets actually being built. >>>>> >>>>> I’ve attached two patches. >>>>> >>>>> The first is a new flag for tablegen to take a list of targets. If passed tablegen will only emit intrinsics that either have empty target prefixes, or target prefixes matching one of the targets in the list. If the flag is not passed the behavior is unchanged. This patch can land today (subject to review). >>>>> >>>>> The second patch is a WIP, and adds support to the CMake build system for using the new tablegen flag, and for generating a new llvm/Config/llvm-targets.h header which contains defines for each target specified with LLVM_TARGETS_TO_BUILD. >>>>> >>>>> This new header will allow us to #ifdef code using target-specific intrinsics outside the targets, thus allowing us to strip out all the unused intrinsics. >>>>> >>>>> -Chris >>>>> >>>>> <cmake-build.diff><tablegen.diff>_______________________________________________ >>>>> 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 <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/20141219/35e7d9c3/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: tablegen.diff Type: application/octet-stream Size: 13519 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141219/35e7d9c3/attachment.obj> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141219/35e7d9c3/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: impossible_enum.diff Type: application/octet-stream Size: 7137 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141219/35e7d9c3/attachment-0001.obj> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141219/35e7d9c3/attachment-0002.html>
This seems to indicate that the idea is a workable solution to your use case. We've still got some bikesheds to get through. (And I don't consider myself a good reviewer for Clang in this, so we need to identify who is.) As background to this extension idea, let me say that I prefer the notion that developers, particularly perf-aware developers like gamedevs, will want to handle PGO via markup. The idea is for them to turn PGO measurements into consciously-chosen markup, since a given test run may not be fully expressive. (e.g. They may not play the whole game through each time.) So the idea is to have the enum be marked with a weight instead of just "impossible." The impossible value would be perhaps -inf. I'd suggest that weights could be either float 0 to 1 or 0 to 100, whichever works best with the PGO infrastructure. Alex> On 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. > > -Chris > > <tablegen.diff> > <impossible_enum.diff> > >> On Dec 19, 2014, at 8:56 AM, Chris Bieneman <beanz at apple.com> wrote: >> >> >> >> >> >>> On Dec 19, 2014, at 12:34 AM, Alex Rosenberg <alexr at leftfield.org> wrote: >>> >>> Putting aside the several minor bikesheds we will get to if we go this route, how close does this approach get to the code shrink you were originally trying to achieve? >> >> I haven't yet adjusted my tablegen changes to take advantage of the, but it is on my list for today. >> >>> >>> Are there any structures other than switch statements that need to go on a diet too? e.g. comparisons? >> >> My patches impact both case statements and comparisons. == compares are always false, any other compare is always true. >> >> -Chris >> >>> >>> Alex >>> >>>> On Dec 18, 2014, at 7:49 PM, Chris Bieneman <beanz at apple.com> wrote: >>>> >>>> Ok… I bit. >>>> >>>> Alex’s proposal here is really compelling to me because it means that the required changes to make this work would be more limited. Specifically a clang attribute could give us all the benefits of #ifdefs throughout the code without the maintenance burden. So, being the silly person I am, I wrote the patches for clang. >>>> >>>> I’ve never done any frontend hacking before, so take these with giant cellars of salt, but the concept seems sound to me. >>>> >>>> The patches do the following: >>>> (1) Add a new C++11-style [[impossible_enum]] attribute >>>> (2) Any case statement that has [[impossible_enum]] applied to it is not emitted during IRGen - the bodies are always emitted so as not to interfere with fall through, but blocks that cannot be entered are optimized away >>>> (3) Equality comparison against [[impossible_enum]] values are always false, all other comparisons are always true >>>> >>>> There was some discussion on IRC today whether or not this was the right way to do this, but I thought I’d send these patches out anyways so people can take a look. >>>> >>>> The attached diffs are for clang, I’ve also attached a c++ test file. >>>> >>>> -Chris >>>> <impossible_enum.cpp> >>>> <impossible_enum.diff> >>>> >>>>> On Dec 16, 2014, at 12:09 PM, Alex Rosenberg <alexr at ohmantics.com> wrote: >>>>> >>>>> Random shower thought: >>>>> >>>>> I think the markup can be minimized if it only appears once in the header where the enums are defined instead of at every place where the enums are used. Then we could value propagate that certain enum values are never possible where they're checked for. That should generally be able to strip the same set of stuff but use less markup. >>>>> >>>>> Alex >>>>> >>>>>> On Dec 10, 2014, at 3:53 PM, Chris Bieneman <beanz at apple.com> wrote: >>>>>> >>>>>> llvm-dev, >>>>>> >>>>>> In my ongoing saga to improve LLVM for embedded use, we would like to support stripping out unused intrinsics based on the LLVM targets actually being built. >>>>>> >>>>>> I’ve attached two patches. >>>>>> >>>>>> The first is a new flag for tablegen to take a list of targets. If passed tablegen will only emit intrinsics that either have empty target prefixes, or target prefixes matching one of the targets in the list. If the flag is not passed the behavior is unchanged. This patch can land today (subject to review). >>>>>> >>>>>> The second patch is a WIP, and adds support to the CMake build system for using the new tablegen flag, and for generating a new llvm/Config/llvm-targets.h header which contains defines for each target specified with LLVM_TARGETS_TO_BUILD. >>>>>> >>>>>> This new header will allow us to #ifdef code using target-specific intrinsics outside the targets, thus allowing us to strip out all the unused intrinsics. >>>>>> >>>>>> -Chris >>>>>> >>>>>> <cmake-build.diff><tablegen.diff>_______________________________________________ >>>>>> LLVM Developers mailing list >>>>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>>>>> 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/20141220/4aedf544/attachment.html>
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. For example, what happens when an "impossible" enum is used to compute the value of some other enum? 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. 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. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141222/da07143d/attachment.html>