[llvm-commits] [llvm] r141599 - in /llvm/trunk: docs/LangRef.html include/llvm/Target/TargetData.h lib/Target/ARM/ARMTargetMachine.cpp lib/Target/TargetData.cpp lib/Target/X86/README-SSE.txt lib/Target/X86/README.txt lib/Target/X86/X86TargetMachine.cpp lib/Transforms/Utils/Local.cpp

[Duncan Sands]

Hi Lang, here you say that the default is zero:

> The natural stack alignment is set in target data strings via the "S<size>"
> option. Size is in bits and must be a multiple of 8. The natural stack alignment
> defaults to "unspecified" (represented by a zero value),

while here

=================================
> --- llvm/trunk/docs/LangRef.html (original)
> +++ llvm/trunk/docs/LangRef.html Mon Oct 10 18:42:08 2011
> @@ -1319,6 +1319,12 @@
>         the bits with the least significance have the lowest address
>         location.</dd>
>
> +<dt><tt>S<i>size</i></tt></dt>
> +<dd>Specifies the natural alignment of the stack in bits. Alignment promotion
> +      of stack variables is limited to the natural stack alignment to avoid
> +      dynamic stack realignment. The stack alignment must be a multiple of
> +      8-bits, and currently defaults to 128 bits if unspecified.</dd>

you say that the default is 128.

> @@ -163,6 +164,11 @@
>       return !isLegalInteger(Width);
>     }
>
> +  /// Returns true if the given alignment exceeds the natural stack alignment.
> +  bool exceedsNaturalStackAlignment(unsigned Align) const {
> +    return (StackNaturalAlign != 0)&&  (Align>  StackNaturalAlign);
> +  }

Is it really a good idea to do the transform if the stack alignment is unknown?

> --- llvm/trunk/lib/Target/TargetData.cpp (original)
> +++ llvm/trunk/lib/Target/TargetData.cpp Mon Oct 10 18:42:08 2011
> @@ -218,7 +219,12 @@
>           Token = Split.second;
>         } while (!Specifier.empty() || !Token.empty());
>         break;
> -
> +    case 'S': // Stack natural alignment.
> +      StackNaturalAlign = getInt(Specifier.substr(1));
> +      StackNaturalAlign /= 8;
> +      // FIXME: Should we really be truncating these alingments and
> +      // sizes silently?
> +      break;

Why don't you keep it in bits?  Also, will the above code accept a negative
stack alignment?

Also: "these alingments" -> "alignments"

> --- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/Local.cpp Mon Oct 10 18:42:08 2011
> @@ -721,10 +721,14 @@
>   /// their preferred alignment from the beginning.
>   ///
>   static unsigned enforceKnownAlignment(Value *V, unsigned Align,
> -                                      unsigned PrefAlign) {
> +                                      unsigned PrefAlign, const TargetData *TD) {
>     V = V->stripPointerCasts();
>
>     if (AllocaInst *AI = dyn_cast<AllocaInst>(V)) {
> +    // If the preferred alignment is greater than the natural stack alignment
> +    // then don't round up. This avoids dynamic stack realignment.
> +    if (TD&&  TD->exceedsNaturalStackAlignment(PrefAlign))
> +      return Align;

Is it really a good idea to increase the alignment if there is no target data?

Ciao, Duncan.