From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117621 invoked by alias); 19 May 2015 17:08:46 -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 117508 invoked by uid 89); 19 May 2015 17:08:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=unavailable version=3.3.2 X-HELO: mail-vn0-f50.google.com Received: from mail-vn0-f50.google.com (HELO mail-vn0-f50.google.com) (209.85.216.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 19 May 2015 17:08:44 +0000 Received: by vnbf62 with SMTP id f62so1597840vnb.7 for ; Tue, 19 May 2015 10:08:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=qmxK7EMfH6KvULtzFki85fRbNwqGtPz4W3f3HF4i9Vk=; b=GFD+Q8ddeErrLaoTzKvLOPSo3pYypmz6X6cmnO/Wp3h5qKix/mtwUS2fyouWkGln53 fAuvYN8iIP2CrWhnKUoWz9zMHpFZSb4XfR9M4DWudyr7HfA8Qi8EmQjzmeOY1YVCgojX R1iM9wGq6HPZWQcJvVMP72geHIGdi/PbRby9g9Etq+HY1F79lEGQJbAsp+l0t2zEKC76 tA27ETGMI/BPqeKDc/HPubPQwtcqkgNY+49jNlrDxao0az/kp2CZZGb+nRXteNuVq0Vr 6jb4I0qY2IEwWDSRSMNFemxcNH29a+2jFnWms0PeGhDstVurejs/nbIeSd5Hwng3aWL/ Apug== X-Gm-Message-State: ALoCoQm2UlVZhDjr+Yf4Hux16ZaoPv9vTTnYFeTLoo0jfp8UmzLi33e58ya/34fOrTv3+P1M7j6r MIME-Version: 1.0 X-Received: by 10.52.89.174 with SMTP id bp14mr28667983vdb.58.1432055320965; Tue, 19 May 2015 10:08:40 -0700 (PDT) Received: by 10.52.229.196 with HTTP; Tue, 19 May 2015 10:08:40 -0700 (PDT) In-Reply-To: References: Date: Tue, 19 May 2015 17:22:00 -0000 Message-ID: Subject: Re: [RFC] COMDAT Safe Module Level Multi versioning From: Sriraman Tallam To: Richard Biener 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/msg01726.txt.bz2 On Tue, May 19, 2015 at 2:39 AM, Richard Biener wrote: > On Tue, May 19, 2015 at 8:16 AM, Sriraman Tallam wr= ote: >> 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 suc= h 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 calle= d 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 = object 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 le= ading 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. Wh= en 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 ma= y 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 simp In general, should that be any option that affects code gen and is only *applied to a subset of modules* is potentially unsafe as the comdat copies generated from those modules are not identical to the copies from other modules. Tagging such modules with -fodr-unsafe-comdats, even conservatively, is fine. In the worst case, the comdat candidate from that module is the only available candidate and the linker uses that and emits a non-fatal message that this comdat was used. Shouldn't that be alright? Thanks Sri > _not_ have unsafe options produce comdats but always make local clones > for them (thus emit the comdat with "unsafe" flags dropped)? > > Richard. > >> >> Thanks >> Sri