[Beignet] [PATCH 1/2] GBE: Fix bug with negative constant GEP index.

[Zhigang Gong]

Please ignore this version and review the v2. Thanks.

On Wed, Nov 26, 2014 at 04:51:11PM +0800, Zhigang Gong wrote:
> GEP index may be negative constant value as below:
> 
> %arrayidx = getelementptr inbounds <4 x i32> addrspace(1)* %src4, i32 %add.ptr.sum, i32 -4
> 
> The previous implementation assumes it's a unsigned value which is incorrect
> and may cause infinite loop.
> 
> Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> ---
>  backend/src/llvm/llvm_gen_backend.cpp |  5 +++--
>  backend/src/llvm/llvm_gen_backend.hpp |  2 +-
>  backend/src/llvm/llvm_passes.cpp      | 17 +++++++++--------
>  3 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/backend/src/llvm/llvm_gen_backend.cpp b/backend/src/llvm/llvm_gen_backend.cpp
> index a41c94c..3d74a0a 100644
> --- a/backend/src/llvm/llvm_gen_backend.cpp
> +++ b/backend/src/llvm/llvm_gen_backend.cpp
> @@ -1126,7 +1126,7 @@ namespace gbe
>        return pointer_reg;
>      }
>      else if (expr->getOpcode() == Instruction::GetElementPtr) {
> -      uint32_t TypeIndex;
> +      int32_t TypeIndex;
>        uint32_t constantOffset = 0;
>  
>        Value *pointer = val;
> @@ -1136,6 +1136,7 @@ namespace gbe
>          ConstantInt* ConstOP = dyn_cast<ConstantInt>(expr->getOperand(op));
>          GBE_ASSERT(ConstOP);
>          TypeIndex = ConstOP->getZExtValue();
> +        GBE_ASSERT(TypeIndex >= 0);
>          if (op == 1) {
>            if (TypeIndex != 0) {
>              Type *elementType = (cast<PointerType>(pointer->getType()))->getElementType();
> @@ -1145,7 +1146,7 @@ namespace gbe
>              offset += elementSize * TypeIndex;
>            }
>          } else {
> -          for(uint32_t ty_i=0; ty_i<TypeIndex; ty_i++)
> +          for(int32_t ty_i=0; ty_i<TypeIndex; ty_i++)
>            {
>              Type* elementType = CompTy->getTypeAtIndex(ty_i);
>              uint32_t align = getAlignmentByte(unit, elementType);
> diff --git a/backend/src/llvm/llvm_gen_backend.hpp b/backend/src/llvm/llvm_gen_backend.hpp
> index 5dac3ea..cf50edd 100644
> --- a/backend/src/llvm/llvm_gen_backend.hpp
> +++ b/backend/src/llvm/llvm_gen_backend.hpp
> @@ -66,7 +66,7 @@ namespace gbe
>    static const OCLIntrinsicMap instrinsicMap;
>  
>    /*! Pad the offset */
> -  uint32_t getPadding(uint32_t offset, uint32_t align);
> +  uint32_t getPadding(int32_t offset, int32_t align);
>  
>    /*! Get the type alignment in bytes */
>    uint32_t getAlignmentByte(const ir::Unit &unit, llvm::Type* Ty);
> diff --git a/backend/src/llvm/llvm_passes.cpp b/backend/src/llvm/llvm_passes.cpp
> index f9fda4d..9c298fa 100644
> --- a/backend/src/llvm/llvm_passes.cpp
> +++ b/backend/src/llvm/llvm_passes.cpp
> @@ -126,7 +126,7 @@ namespace gbe
>      return bKernel;
>    }
>  
> -  uint32_t getPadding(uint32_t offset, uint32_t align) {
> +  uint32_t getPadding(int32_t offset, int32_t align) {
>      return (align - (offset % align)) % align; 
>    }
>  
> @@ -279,32 +279,33 @@ namespace gbe
>  
>      for(uint32_t op=1; op<GEPInst->getNumOperands(); ++op)
>      {
> -      uint32_t TypeIndex;
> +      int32_t TypeIndex;
>        //we have a constant struct/array acces
>        if(ConstantInt* ConstOP = dyn_cast<ConstantInt>(GEPInst->getOperand(op)))
>        {
> -        uint32_t offset = 0;
> +        int32_t offset = 0;
>          TypeIndex = ConstOP->getZExtValue();
> +        int32_t step = TypeIndex > 0 ? 1 : -1;
>          if (op == 1) {
>            if (TypeIndex != 0) {
>              Type *elementType = (cast<PointerType>(parentPointer->getType()))->getElementType();
>              uint32_t elementSize = getTypeByteSize(unit, elementType);
>              uint32_t align = getAlignmentByte(unit, elementType);
>              elementSize += getPadding(elementSize, align);
> -            offset += elementSize * TypeIndex;
> +            offset += elementSize * TypeIndex * step;
>            }
>          } else {
> -          for(uint32_t ty_i=0; ty_i<TypeIndex; ty_i++)
> +          for(int32_t ty_i=0; ty_i != TypeIndex; ty_i += step)
>            {
>              Type* elementType = CompTy->getTypeAtIndex(ty_i);
>              uint32_t align = getAlignmentByte(unit, elementType);
> -            offset += getPadding(offset, align);
> -            offset += getTypeByteSize(unit, elementType);
> +            offset += getPadding(offset, align * step);
> +            offset += getTypeByteSize(unit, elementType) * step;
>            }
>  
>            //add getPaddingding for accessed type
>            const uint32_t align = getAlignmentByte(unit, CompTy->getTypeAtIndex(TypeIndex));
> -          offset += getPadding(offset, align);
> +          offset += getPadding(offset, align * step);
>          }
>  
>          constantOffset += offset;
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet