[llvm-commits] [llvm] r109643 - in /llvm/trunk: include/llvm/Instructions.h lib/AsmParser/LLParser.cpp lib/Transforms/InstCombine/InstCombineCalls.cpp lib/VMCore/Instructions.cpp test/Assembler/align-inst-alloca.ll test/Assembler/align-inst-load.ll test/Assembler/align-inst-store.ll test/Assembler/align-inst.ll

[Chris Lattner]

On Jul 28, 2010, at 1:12 PM, Dan Gohman wrote:

> Author: djg
> Date: Wed Jul 28 15:12:04 2010
> New Revision: 109643
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=109643&view=rev
> Log:
> Define a maximum supported alignment value for load, store, and
> alloca instructions (constrained by their internal encoding),
> and add error checking for it. Fix an instcombine bug which
> generated huge alignment values (null is infinitely aligned).
> This fixes undefined behavior noticed by John Regehr.

Cool.

> 
> +/// MaximumAlignment - This is the greatest alignment value supported by
> +/// load, store, and alloca instructions.
> +static const unsigned MaximumAlignment = 1u << 29;

How about making this an enum in the Instruction class (so it is llvm::Instruction::MaximumAlignment), instead of being llvm::MaximumAlignment?

-Chris


> +
> //===----------------------------------------------------------------------===//
> //                                AllocaInst Class
> //===----------------------------------------------------------------------===//
> 
> Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=109643&r1=109642&r2=109643&view=diff
> ==============================================================================
> --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Wed Jul 28 15:12:04 2010
> @@ -1154,6 +1154,8 @@
>   if (ParseUInt32(Alignment)) return true;
>   if (!isPowerOf2_32(Alignment))
>     return Error(AlignLoc, "alignment is not a power of two");
> +  if (Alignment > MaximumAlignment)
> +    return Error(AlignLoc, "huge alignments are not supported yet");
>   return false;
> }
> 
> @@ -1176,6 +1178,7 @@
>     if (Lex.getKind() != lltok::kw_align)
>       return Error(Lex.getLoc(), "expected metadata or 'align'");
> 
> +    LocTy AlignLoc = Lex.getLoc();
>     if (ParseOptionalAlignment(Alignment)) return true;
>   }
> 
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=109643&r1=109642&r2=109643&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Wed Jul 28 15:12:04 2010
> @@ -104,11 +104,15 @@
>   ComputeMaskedBits(V, Mask, KnownZero, KnownOne);
>   unsigned TrailZ = KnownZero.countTrailingOnes();
> 
> -  // LLVM doesn't support alignments larger than this currently.
> +  // Avoid trouble with rediculously large TrailZ values, such as
> +  // those computed from a null pointer.
>   TrailZ = std::min(TrailZ, unsigned(sizeof(unsigned) * CHAR_BIT - 1));
> 
>   unsigned Align = 1u << std::min(BitWidth - 1, TrailZ);
> 
> +  // LLVM doesn't support alignments larger than this currently.
> +  Align = std::min(Align, MaximumAlignment);
> +
>   if (PrefAlign > Align)
>     Align = EnforceKnownAlignment(V, Align, PrefAlign);
> 
> 
> Modified: llvm/trunk/lib/VMCore/Instructions.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Instructions.cpp?rev=109643&r1=109642&r2=109643&view=diff
> ==============================================================================
> --- llvm/trunk/lib/VMCore/Instructions.cpp (original)
> +++ llvm/trunk/lib/VMCore/Instructions.cpp Wed Jul 28 15:12:04 2010
> @@ -891,6 +891,8 @@
> 
> void AllocaInst::setAlignment(unsigned Align) {
>   assert((Align & (Align-1)) == 0 && "Alignment is not a power of 2!");
> +  assert(Align <= MaximumAlignment &&
> +         "Alignment is greater than MaximumAlignment!");
>   setInstructionSubclassData(Log2_32(Align) + 1);
>   assert(getAlignment() == Align && "Alignment representation error!");
> }
> @@ -1026,8 +1028,11 @@
> 
> void LoadInst::setAlignment(unsigned Align) {
>   assert((Align & (Align-1)) == 0 && "Alignment is not a power of 2!");
> +  assert(Align <= MaximumAlignment &&
> +         "Alignment is greater than MaximumAlignment!");
>   setInstructionSubclassData((getSubclassDataFromInstruction() & 1) |
>                              ((Log2_32(Align)+1)<<1));
> +  assert(getAlignment() == Align && "Alignment representation error!");
> }
> 
> //===----------------------------------------------------------------------===//
> @@ -1122,8 +1127,11 @@
> 
> void StoreInst::setAlignment(unsigned Align) {
>   assert((Align & (Align-1)) == 0 && "Alignment is not a power of 2!");
> +  assert(Align <= MaximumAlignment &&
> +         "Alignment is greater than MaximumAlignment!");
>   setInstructionSubclassData((getSubclassDataFromInstruction() & 1) |
>                              ((Log2_32(Align)+1) << 1));
> +  assert(getAlignment() == Align && "Alignment representation error!");
> }
> 
> //===----------------------------------------------------------------------===//
> 
> Added: llvm/trunk/test/Assembler/align-inst-alloca.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/align-inst-alloca.ll?rev=109643&view=auto
> ==============================================================================
> --- llvm/trunk/test/Assembler/align-inst-alloca.ll (added)
> +++ llvm/trunk/test/Assembler/align-inst-alloca.ll Wed Jul 28 15:12:04 2010
> @@ -0,0 +1,6 @@
> +; RUN: not llvm-as %s -o /dev/null 2>/dev/null
> +
> +define void @foo() {
> +  %p = alloca i1, align 1073741824
> +  ret void
> +}
> 
> Added: llvm/trunk/test/Assembler/align-inst-load.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/align-inst-load.ll?rev=109643&view=auto
> ==============================================================================
> --- llvm/trunk/test/Assembler/align-inst-load.ll (added)
> +++ llvm/trunk/test/Assembler/align-inst-load.ll Wed Jul 28 15:12:04 2010
> @@ -0,0 +1,6 @@
> +; RUN: not llvm-as %s -o /dev/null 2>/dev/null
> +
> +define void @foo() {
> +  load i1* %p, align 1073741824
> +  ret void
> +}
> 
> Added: llvm/trunk/test/Assembler/align-inst-store.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/align-inst-store.ll?rev=109643&view=auto
> ==============================================================================
> --- llvm/trunk/test/Assembler/align-inst-store.ll (added)
> +++ llvm/trunk/test/Assembler/align-inst-store.ll Wed Jul 28 15:12:04 2010
> @@ -0,0 +1,6 @@
> +; RUN: not llvm-as %s -o /dev/null 2>/dev/null
> +
> +define void @foo() {
> +  store i1 false, i1* %p, align 1073741824
> +  ret void
> +}
> 
> Added: llvm/trunk/test/Assembler/align-inst.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/align-inst.ll?rev=109643&view=auto
> ==============================================================================
> --- llvm/trunk/test/Assembler/align-inst.ll (added)
> +++ llvm/trunk/test/Assembler/align-inst.ll Wed Jul 28 15:12:04 2010
> @@ -0,0 +1,8 @@
> +; RUN: llvm-as %s -o /dev/null
> +
> +define void @foo() {
> +  %p = alloca i1, align 536870912
> +  load i1* %p, align 536870912
> +  store i1 false, i1* %p, align 536870912
> +  ret void
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits