r350643 - Limit COFF 'common' emission to <=32 alignment types.

Tue Jan 8 15:20:31 PST 2019

Yes, the MinGW toolchain can handle this by specifying the alignment of a
common symbol using the aligncomm directive. The MSVC toolchain has no such
mechanism.

This is why the check uses isKnownWindowsMSVCEnvironment.

On Tue, Jan 8, 2019 at 1:09 PM Shoaib Meenai <smeenai at fb.com> wrote:

> It checks for both OS=Win32 and Environment=MSVC, so that wouldn't cover
> other COFF environments. wbs (Martin Storsjo) mentioned on IRC that MinGW
> adds an aligncomm directive to specify alignment for common symbols, so
> perhaps that's part of it?
>
>
>
> *From: *"Keane, Erich" <erich.keane at intel.com>
> *Date: *Tuesday, January 8, 2019 at 1:04 PM
> *To: *Shoaib Meenai <smeenai at fb.com>, "cfe-commits at lists.llvm.org" <
> cfe-commits at lists.llvm.org>, David Majnemer <david.majnemer at gmail.com>
> *Subject: *RE: r350643 - Limit COFF 'common' emission to <=32 alignment
> types.
>
>
>
> Yep, exactly.  I looked, and isKnownWindowsMSVCEnvironment checks for
> OS=Win32, which I believe would be different for other architectures.
>
>
>
> *From:* Shoaib Meenai [mailto:smeenai at fb.com]
> *Sent:* Tuesday, January 8, 2019 12:41 PM
> *To:* Keane, Erich <erich.keane at intel.com>; cfe-commits at lists.llvm.org;
> David Majnemer <david.majnemer at gmail.com>
> *Subject:* Re: r350643 - Limit COFF 'common' emission to <=32 alignment
> types.
>
>
>
> Ah, looks like you were originally checking for COFF, and then David
> suggested checking for MSVC instead? I'm curious about why, although I'm
> sure the suggestion is legit :)
>
>
>
> *From: *cfe-commits <cfe-commits-bounces at lists.llvm.org> on behalf of
> Shoaib Meenai via cfe-commits <cfe-commits at lists.llvm.org>
> *Reply-To: *Shoaib Meenai <smeenai at fb.com>
> *Date: *Tuesday, January 8, 2019 at 12:39 PM
> *To: *Erich Keane <erich.keane at intel.com>, "cfe-commits at lists.llvm.org" <
> cfe-commits at lists.llvm.org>
> *Subject: *Re: r350643 - Limit COFF 'common' emission to <=32 alignment
> types.
>
>
>
> Why does this check for isKnownWindowsMSVCEnvironment specifically?
> Wouldn't any COFF target (windows-cygnus, windows-gnu, windows-itanium,
> etc.) have the same limitation, since it's an object file format issue and
> not an ABI issue?
>
>
>
> *From: *cfe-commits <cfe-commits-bounces at lists.llvm.org> on behalf of
> Erich Keane via cfe-commits <cfe-commits at lists.llvm.org>
> *Reply-To: *Erich Keane <erich.keane at intel.com>
> *Date: *Tuesday, January 8, 2019 at 10:48 AM
> *To: *"cfe-commits at lists.llvm.org" <cfe-commits at lists.llvm.org>
> *Subject: *r350643 - Limit COFF 'common' emission to <=32 alignment types.
>
>
>
> Author: erichkeane
>
> Date: Tue Jan  8 10:44:22 2019
>
> New Revision: 350643
>
>
>
> URL:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D350643-26view-3Drev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=RNVKy_b0_Wgp_PTFDpvQXETsZdWubmT5SGnGz3GigS0&s=Ph9GOtRaQERmqyeJeAJTFwV3sg3q8fE05FlJ3qwNx4I&e=
>
> Log:
>
> Limit COFF 'common' emission to <=32 alignment types.
>
>
>
> As reported in PR33035, LLVM crashes if given a common object with an
>
> alignment of greater than 32 bits. This is because the COFF file format
>
> does not support these alignments, so emitting them is broken anyway.
>
>
>
> This patch changes any global definitions greater than 32 bit alignment
>
> to no longer be in 'common'.
>
>
>
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.llvm.org_show-5Fbug.cgi-3Fid-3D33035&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=RNVKy_b0_Wgp_PTFDpvQXETsZdWubmT5SGnGz3GigS0&s=ac1NEHuvztd6jSTCsOUJajkklfeyqdzW-xqtddJ-hvM&e=
>
>
>
> Differential Revision:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D56391&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=RNVKy_b0_Wgp_PTFDpvQXETsZdWubmT5SGnGz3GigS0&s=AucP9Sp-DYHSaOP-sPfpAOrww3xwdh8FjQkHrLZhhyo&e=
>
>
>
> Change-Id: I48609289753b7f3b58c5e2bc1712756750fbd45a
>
>
>
> Added:
>
>     cfe/trunk/test/CodeGen/microsoft-no-common-align.c
>
> Modified:
>
>     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>
>
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>
> URL:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_CodeGen_CodeGenModule.cpp-3Frev-3D350643-26r1-3D350642-26r2-3D350643-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=RNVKy_b0_Wgp_PTFDpvQXETsZdWubmT5SGnGz3GigS0&s=gmTnEmW03ruG8LbJluf5Z4yQcxM64QP7Ce1VTnVqvPo&e=
>
>
> ==============================================================================
>
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
>
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Jan  8 10:44:22 2019
>
> @@ -3761,6 +3761,11 @@ static bool isVarDeclStrongDefinition(co
>
>        }
>
>      }
>
>    }
>
> +  // COFF doesn't support alignments greater than 32, so these cannot be
>
> +  // in common.
>
> +  if (Context.getTargetInfo().getTriple().isKnownWindowsMSVCEnvironment()
> &&
>
> +      Context.getTypeAlignIfKnown(D->getType()) > 32)
>
> +    return true;
>
>    return false;
>
> }
>
>
>
> Added: cfe/trunk/test/CodeGen/microsoft-no-common-align.c
>
> URL:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_CodeGen_microsoft-2Dno-2Dcommon-2Dalign.c-3Frev-3D350643-26view-3Dauto&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=RNVKy_b0_Wgp_PTFDpvQXETsZdWubmT5SGnGz3GigS0&s=hzPmmVFbvg4OTEVpnQ5pIfy295Ne0-xAsctZs00WZgY&e=
>
>
> ==============================================================================
>
> --- cfe/trunk/test/CodeGen/microsoft-no-common-align.c (added)
>
> +++ cfe/trunk/test/CodeGen/microsoft-no-common-align.c Tue Jan  8 10:44:22
> 2019
>
> @@ -0,0 +1,8 @@
>
> +// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -o - %s |
> FileCheck %s
>
> +typedef float TooLargeAlignment __attribute__((__vector_size__(64)));
>
> +typedef float NormalAlignment __attribute__((__vector_size__(4)));
>
> +
>
> +TooLargeAlignment TooBig;
>
> +// CHECK: @TooBig = dso_local global <16 x float>  zeroinitializer, align
> 64
>
> +NormalAlignment JustRight;
>
> +// CHECK: @JustRight = common dso_local global <1 x
> float>  zeroinitializer, align 4
>
>
>
>
>
> _______________________________________________
>
> cfe-commits mailing list
>
> cfe-commits at lists.llvm.org
>
>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=RNVKy_b0_Wgp_PTFDpvQXETsZdWubmT5SGnGz3GigS0&s=Myn7SZhcOe32EZiKZr4ByJAZOoFl5aIfmWV9555Vh9A&e=
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190108/842b6cb1/attachment-0001.html>