From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81683 invoked by alias); 28 Jun 2017 08:04:02 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 81651 invoked by uid 89); 28 Jun 2017 08:04:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy= X-HELO: mail-ua0-f177.google.com Received: from mail-ua0-f177.google.com (HELO mail-ua0-f177.google.com) (209.85.217.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Jun 2017 08:03:59 +0000 Received: by mail-ua0-f177.google.com with SMTP id z22so33045246uah.1 for ; Wed, 28 Jun 2017 01:03:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=jkbaxoyoFesvPAxVPQHEbNIgqLImG6KqZ0bRW+fQ+fY=; b=uPyJpRSgIvZ561gVDza6ue9a8QgMP+ZI/nV6wvkcdPI93Bd2o4+gjgXEFnxj/7zDVb 18fxINeg91ssFOR/wlx/i5lpvwA3z4BWWHwxGpBfv8fvUMsiYWxrzwwkwrerMLFNsdAJ ACy/LRZd/ejUFAdh2M4CLu+xi5mFNmn6TsyjISTlY31UPOrjvK/hX/K1Etj+VFGd5yCK TAwEo0OY1DhWgLoyDzGJusgqperqCI5GllqK9cEdqbA+sHVqxKtYygxP5SJUjIK24muJ hTSW5r1NI4K0eR1+ObPheXMjLCLowuJjtMEebft0wuRQiSKV9VMuN/Xhd8Lv7cLwmu/8 oWpw== X-Gm-Message-State: AKS2vOx4ej30DkG2B8SfYHAQ5apUGr0LCHKQAYiB/GWZkM4bUV8byUrf eS0k8tusYpOtclbgjTIqCvDgTKPTmKLqL5fkRg== X-Received: by 10.176.67.163 with SMTP id l32mr4756309ual.119.1498637037252; Wed, 28 Jun 2017 01:03:57 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.148.220 with HTTP; Wed, 28 Jun 2017 01:03:56 -0700 (PDT) In-Reply-To: References: From: Christophe Lyon Date: Wed, 28 Jun 2017 08:04:00 -0000 Message-ID: Subject: Re: Fix genmultilib reuse rule checks for large sets of option combinations To: Joseph Myers Cc: "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg02127.txt.bz2 Hi Joseph, On 8 June 2017 at 22:28, Joseph Myers wrote: > genmultilib computes combination_space, a list of all combinations of > options in MULTILIB_OPTIONS that might have multilibs built for them > (some of which may end up not having multilibs built for them, and > some of those may end up being mapped to other multilibs with > MULTILIB_REUSE). It is then used to validate the right hand part of > MULTILIB_REUSE rules, checking with expr that combination_space > matches a basic regular expression derived from that right hand part. > > There are two problems with this approach to validation: > > * It requires that right hand part to have options in the same order > as in MULTILIB_OPTIONS, in contradiction to the documentation of > MULTILIB_REUSE saying that order does not matter there. > > * combination_space can be so large that the expr call fails with an > E2BIG error. I have a local ARM configuration with 40 multilibs but > 3840 combinations of options from MULTILIB_OPTIONS (so 3839 listed > in combination_space, since it doesn't list the default multilib) > and 996 MULTILIB_REUSE rules. This generates a combination_space > string longer than the Linux kernel's MAX_ARG_STRLEN (PAGE_SIZE * > 32, the limit on the length of a single argv string), so that expr > cannot be run. > > This patch changes the validation approach to generate a much shorter > extended regular expression for any sequence of multilib options in > any order, and uses that for the validation instead. > > Tested with a built for arm-none-eabi --with-multilib-list=aprofile > (as a configuration that uses MULTILIB_REUSE). > > 2017-06-08 Joseph Myers > > * genmultilib (combination_space): Remove variable. > Validate reuse rules against regular expression for any sequence > of multilib options in any order. > > Index: gcc/genmultilib > =================================================================== > --- gcc/genmultilib (revision 249028) > +++ gcc/genmultilib (working copy) > @@ -186,8 +186,7 @@ > EOF > chmod +x tmpmultilib > > -combination_space=`initial=/ ./tmpmultilib ${options}` > -combinations="$combination_space" > +combinations=`initial=/ ./tmpmultilib ${options}` > > # If there exceptions, weed them out now > if [ -n "${exceptions}" ]; then > @@ -460,6 +459,15 @@ > echo "NULL" > echo "};" > > +# Generate a regular expression to validate option combinations. > +options_re= > +for set in ${options}; do > + for opt in `echo ${set} | sed -e 's_[/|]_ _g'`; do > + options_re="${options_re}${options_re:+|}${opt}" > + done > +done > +options_re="^/((${options_re})/)*\$" > + > # Output rules used for multilib reuse. > echo "" > echo "static const char *const multilib_reuse_raw[] = {" > @@ -473,7 +481,7 @@ > # in this variable, it means no multilib will be built for current reuse > # rule. Thus the reuse purpose specified by current rule is meaningless. > if expr "${combinations} " : ".*/${combo}/.*" > /dev/null; then > - if expr "${combination_space} " : ".*/${copts}/.*" > /dev/null; then > + if echo "/${copts}/" | grep -E "${options_re}" > /dev/null; then > combo="/${combo}/" > dirout=`./tmpmultilib3 "${combo}" "${todirnames}" "${toosdirnames}" "${enable_multilib}"` > copts="/${copts}/" > This broke my builds, where I do not use --with-multilib-list=aprofile, and uses the default. I suspect it would also fail for the aprofile multilibs, though. I think there's a problem with options that have a '+' which confuses the regexp in options_re. For instance, I get this error message: The rule mthumb=mthumb/mfpu.auto/march.armv5te+fp contains an option absent from MULTILIB_OPTIONS. A bit of manual debugging led me to: $ echo /mthumb/mfpu=auto/march=armv5te+fp/ | grep -E '^/((marm|mthumb|mfpu=auto|march=armv5te+fp|march=armv7+fp|mfloat-abi=hard)/)*$' [empty result] Replacing the '+' in armv5te+fp with either '\+' or '.' allows the pattern to match: /mthumb/mfpu=auto/march=armv5te+fp/ Is it a matter of adding sed -e 's/\+/./g' when building options_re ? Or would this break something else? Thanks, Christophe > -- > Joseph S. Myers > joseph@codesourcery.com