From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24230 invoked by alias); 19 May 2015 18:16:07 -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 24152 invoked by uid 89); 19 May 2015 18:16:07 -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-f51.google.com Received: from mail-vn0-f51.google.com (HELO mail-vn0-f51.google.com) (209.85.216.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 19 May 2015 18:16:03 +0000 Received: by vnbg190 with SMTP id g190so1761711vnb.3 for ; Tue, 19 May 2015 11:16:01 -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=tL21kojh09raF1QNLkbkh3HrgHZZLSi20py8E8/wX5A=; b=mbMJUKWYivZjMkUlnRoGP5WEdOcFDa7JZfgPfLQtf6/wWU100J5QyTbM4I1W44cEkG y3i14eL8jlP0e+rREx7O+eZJXhDjIWq/MQNXtfkkkqwW72ukp1qh0e1NvsNq0fR/15b6 Ao3Y8B/IPmAviSy0KeIwup5MuyuGJHOGm4uaF6C7msBwyxglTb4zFGl/l+gGriVXjlHz OyYNnbhOhVB2QwRpkRFjxWFYFmAO9ipdijgb7+Wu0nTBAr1zL6nQ5f54KvZ1VEDCn4vu HmbKcMWYDlNrVpgdhEf3nAgDJLj7R9dQqfFfyU+i3QZmMWIiKLlBxiLJDb0+9yOZih+2 2IUQ== X-Gm-Message-State: ALoCoQklU+ZrUeBN1WYO/s6Pk+c5HbmbxVE1u3CBxmX8ogbvh5azvkHIkiBpqie5hwufbfXElllb MIME-Version: 1.0 X-Received: by 10.52.142.229 with SMTP id rz5mr29294348vdb.40.1432059361538; Tue, 19 May 2015 11:16:01 -0700 (PDT) Received: by 10.52.229.196 with HTTP; Tue, 19 May 2015 11:16:01 -0700 (PDT) In-Reply-To: <555B7143.3000502@samsung.com> References: <555B7143.3000502@samsung.com> Date: Tue, 19 May 2015 18:18:00 -0000 Message-ID: Subject: Re: [RFC] COMDAT Safe Module Level Multi versioning From: Sriraman Tallam To: Yury Gribov 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/msg01738.txt.bz2 On Tue, May 19, 2015 at 10:22 AM, Yury Gribov wrote: > On 05/19/2015 09:16 AM, Sriraman Tallam wrote: >> >> 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. > > > Can IFUNC relocations be used to properly select optimal version of code = at > runtime? Yes, we do want a solution like this but all the dispatching code for IFUNC needs to be generated at link-time. Here is an example header file with target-specific functionalities : https://bitbucket.org/eigen/eigen/src/6ed647a644b8e3924800f0916a4ce4addf9e7= 739/Eigen/Core?at=3Ddefault This header can be included in several modules and the modules may be specialized for SSE4.2, AVX, etc respectively when compiling for x86. The calls to functions defined in each module are appropriately guarded by target checks using builtins but each such module may generate specialized COMDAT candidates which needs to be carefully handled by the linker. Only the linker sees the various COMDAT choices, so we need to generate the body of the IFUNC based dispatcher at link-time. The linker needs to do quite a bit here. We need a mechanism to tell the linker what extra options were used to generate each COMDAT copy and what platform checks are needed before this copy can be executed. Thanks Sri > > -Y >