From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61854 invoked by alias); 19 May 2015 09:39:24 -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 60947 invoked by uid 89); 19 May 2015 09:39:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-oi0-f45.google.com Received: from mail-oi0-f45.google.com (HELO mail-oi0-f45.google.com) (209.85.218.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 19 May 2015 09:39:22 +0000 Received: by oign205 with SMTP id n205so6371018oig.2 for ; Tue, 19 May 2015 02:39:20 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.202.228.71 with SMTP id b68mr21887122oih.58.1432028360502; Tue, 19 May 2015 02:39:20 -0700 (PDT) Received: by 10.76.115.167 with HTTP; Tue, 19 May 2015 02:39:20 -0700 (PDT) In-Reply-To: References: Date: Tue, 19 May 2015 09:39:00 -0000 Message-ID: Subject: Re: [RFC] COMDAT Safe Module Level Multi versioning From: Richard Biener To: Sriraman Tallam Cc: "H.J. Lu" , David Li , GCC Patches , binutils , Cary Coutant Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg01672.txt.bz2 On Tue, May 19, 2015 at 8:16 AM, Sriraman Tallam wrot= e: > We have the following problem with selectively compiling modules with > -m options and I have provided a solution to solve this. I would > like to hear what you think. > > Multi versioning at module granularity is done by compiling a subset > of modules with advanced ISA instructions, supported on later > generations of the target architecture, via -m options and > invoking the functions defined in these modules with explicit checks > for the ISA support via builtin functions, __builtin_cpu_supports. > This mechanism has the unfortunate side-effect that generated COMDAT > candidates from these modules can contain these advanced instructions > and potentially =E2=80=9Cviolate=E2=80=9D ODR assumptions. Choosing such= a COMDAT > candidate over a generic one from a different module can cause SIGILL > on platforms where the advanced ISA is not supported. > > Here is a slightly contrived example to illustrate: > > > matrixdouble.h > -------------------- > // Template (Comdat) function definition in a header: > > template > __attribute__((noinline)) > void matrixDouble (T *a) { > for (int i =3D 0 ; i < 16; ++i) //Vectorizable Loop > a[i] =3D a[i] * 2; > } > > avx.cc (Compile with -mavx -O2) > --------- > > #include "matrixdouble.h" > void getDoubleAVX(int *a) { > matrixDouble(a); // Instantiated with vectorized AVX instructions > } > > > non_avx.cc (Compile with -mno-avx -O2) > --------------- > > #include =E2=80=9Cmatrixdouble.h=E2=80=9D > void > getDouble(int *a) { > matrixDouble(a); // Instantiated with non-AVX instructions > } > > > main.cc > ----------- > > void getDoubleAVX(int *a); > void getDouble(int *a); > > int a[] =3D { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 }; > int main () { > // The AVX call is appropriately guarded. > if (__builtin_cpu_supports(=E2=80=9Cavx=E2=80=9D)) > getDoubleAVX(a); > else > getDouble(a); > return a[0]; > } > > > In the above code, function =E2=80=9CgetDoubleAVX=E2=80=9D is only called= when the > run-time CPU supports AVX instructions. This code looks clean but > suffers from the COMDAT ODR violation. Two copies of COMDAT function > =E2=80=9CmatrixDouble=E2=80=9D are generated. One copy is generated in o= bject file > =E2=80=9Cavx.o=E2=80=9D with AVX instructions and another copy exists in = =E2=80=9Cnon_avx.o=E2=80=9D > without AVX instruction. At link time, in a link order where object > file avx.o is seen ahead of non_avx.o, the COMDAT copy of function > =E2=80=9CmatrixDouble=E2=80=9D that contains AVX instructions is kept lea= ding to > SIGILL on unsupported platforms. To reproduce the SIGILL, > > > $ g++ -c -O2 -mavx avx.cc > $ g++ -c -O2 -mno-avx non_avx.cc > $ g++ main.cc avx.o non_avx.o > $ ./a.out # on a non-AVX machine > Illegal Instruction > > > To solve this, I propose introducing a new compiler option, say > -fodr-unsafe-comdats, to let the user tag objects that use specialized > options and let the linker choose the comdat candidate to be linked > wisely. The root cause of the above problem is that comdat functions > in common headers may not be properly guarded and the linker picks the > first candidate it sees. A link order where the object with the > specialized comdat functions appear first causes these comdats to be > picked leading to SIGILL on unsupported arches. With the objects > tagged, the linker can be made to pick other comdat candidates when > possible. > > More details: > > This option is user specified when using arch specific options like > -m. It is an indicator to the compiler that any comdat bodies > generated are potentially unsafe for execution. Note that the COMDAT > bodies however have to be generated as there are no guarantees that > other modules will do so. The compiler then emits a specially named > section, like =E2=80=9C.gnu.odr.unsafe=E2=80=9D, in the object file. Whe= n the linker > tries to pick a COMDAT candidate from several choices, it must avoid > COMDAT copies from objects with sections named =E2=80=9C.gnu.odr.unsafe= =E2=80=9D when > presented with a choice to pick a candidate from an object that does > not have the =E2=80=9C.gnu.odr.unsafe=E2=80=9D section. Note that it may= not be > possible to do that in which case the linker must pick the unsafe > copy, it could explicitly warn when this happens. > > Alternately, the compiler can bind locally any emitted comdat version > from a specialized module, which could also be guarded by an option. > This will solve the problem but this may not be always possible > especially when addresses of any such comdat version is taken. Hm. But which options are unsafe? Also wouldn't it be better to simply _not_ have unsafe options produce comdats but always make local clones for them (thus emit the comdat with "unsafe" flags dropped)? Richard. > > Thanks > Sri