[PATCH][RFC] Implement target-specific __attribute__((aligned)) value

[Ulrich Weigand]

I've renamed the variables as requested below, and checked the patch in as
rev. 235397.

Thanks for the review!


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand | Phone: +49-7031/16-3727
  STSM, GNU/Linux compilers and toolchain
  IBM Deutschland Research & Development GmbH
  Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung: Dirk
Wittkopp
  Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht
Stuttgart, HRB 243294



From:	Richard Smith <richard at metafoo.co.uk>
To:	Ulrich Weigand/Germany/IBM at IBMDE
Cc:	llvm cfe <cfe-commits at cs.uiuc.edu>
Date:	21.04.2015 00:01
Subject:	Re: [PATCH][RFC] Implement target-specific
            __attribute__((aligned)) value
Sent by:	metafoo at gmail.com



This generally looks fine.

On Mon, Mar 30, 2015 at 11:20 AM, Ulrich Weigand <Ulrich.Weigand at de.ibm.com
> wrote:


  Hello,

  the GCC construct __attribute__((aligned)) is defined to set alignment
  to "the default alignment for the target architecture" according to
  the GCC documentation:

    The default alignment is sufficient for all scalar types, but may not
  be
    enough for all vector types on a target that supports vector
  operations.
    The default alignment is fixed for a particular target ABI.

I don't think this means to define a general term "default alignment";
instead, I think this is just shorthand for "default alignment for
__attribute__((aligned))". I'd prefer if you used a bit more of a specific
name for this field. Something like DefaultAlignForAttributeAligned would
be suitably clear.

  clang currently hard-codes an alignment of 16 bytes for that construct,
  which is correct on some platforms (including X86), but wrong on others
  (including SystemZ).  Since this value is ABI-relevant, it is important
  to get correct for compatibility purposes.

  The following patch adds a new TargetInfo member "DefaultAlign" that
  targets can set to the appropriate default __attribute__((aligned))
  value.

  Note that I'm deliberately *not* using the existing "SuitableAlign"
  value, which is used to set the pre-defined macro __BIGGEST_ALIGNMENT__,
  since those two values may not be the same on all platforms.  In fact,
  on X86, __attribute__((aligned)) always uses 16-byte alignment, while
  __BIGGEST_ALIGNMENT__ may be larger if AVX-2 or AVX-512 are supported.
  (This is actually not yet correctly implemented in clang either.)

  The patch provides a value for DefaultAlign only for SystemZ, and leaves
  the default for all other targets at 16, which means no visible change
  in behavior on all other targets.  (The value is still wrong for some
  other targets, but I'd prefer to leave it to the target maintainers for
  those platforms to fix and test.)

  (See attached file: clang-align-attribute)


  Mit freundlichen Gruessen / Best Regards

  Ulrich Weigand

  --
    Dr. Ulrich Weigand | Phone: +49-7031/16-3727
    STSM, GNU/Linux compilers and toolchain
    IBM Deutschland Research & Development GmbH
    Vorsitzende des Aufsichtsrats: Martina Koederitz | Geschäftsführung:
  Dirk
  Wittkopp
    Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht
  Stuttgart, HRB 243294
  _______________________________________________
  cfe-commits mailing list
  cfe-commits at cs.uiuc.edu
  http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits