[llvm-commits] [llvm-gcc] Signedness patch for rs6000.h

Reid Spencer rspencer at reidspencer.com
Thu Dec 21 09:40:07 PST 2006


On Thu, 2006-12-21 at 11:49 -0400, Jim Laskey wrote:
> It's a bit of a tango, but enclosed is the patch I'll check in.  The
> i386 and ppc patches are dependent because of the change
> to LLVM_TARGET_INTRINSIC_LOWER.  Below are additional changes to
> rs6000.h (marked in red).
> 
> 
> 
> 
> Index: gcc/config/rs6000/rs6000.h
> ===================================================================
> --- gcc/config/rs6000/rs6000.h (revision 121615)
> +++ gcc/config/rs6000/rs6000.h (working copy)
> @@ -3655,7 +3655,7 @@
>        Cache = M->getOrInsertFunction(NAME, FT);
> \
>      }
> \
>      Value *Offset = OPS[OPNUM], *Ptr = OPS[OPNUM+1];
> \
> -    Ptr = CastToType(Ptr, VoidPtrTy);
> \
> +    Ptr = CastToType(Instruction::BitCast, Ptr, VoidPtrTy);
> \

Okay, looks fine.

>      if (!isa<Constant>(Offset) || !
> cast<Constant>(Offset)->isNullValue())     \
>        Ptr = new GetElementPtrInst(Ptr, Offset, "tmp", CURBB);
> \
>      OPS.erase(OPS.begin()+OPNUM);
> \
> @@ -3685,6 +3685,27 @@
>             ((TY == Type::SByteTy) ? 'b' :               \
>              ((TY == Type::FloatTy) ? 'f' : 'x'))))
>                    
> +/* LLVM_TARGET_INTRINSIC_CAST_RESULT - This macro just provides a
> frequently
> + * used sequence for use inside LLVM_TARGET_INTRINSIC_LOWER. Note
> that this
> + * macro assumes it is being invoked from inside
> LLVM_TARGET_INTRINSC_LOWER
> + * (see below) because it requires the "ResIsSigned" and
> "ExpIsSigned" macro 
> + * arguments in order to derive signedness for the cast.
> + */
> +#define LLVM_TARGET_INTRINSIC_CAST_RESULT(RESULT, RESISSIGNED,
> DESTTY,        \
> +                                          EXPISSIGNED)
> \
> +  { Instruction::CastOps opcode = CastInst::getCastOpcode(RESULT,
> \
> +        RESISSIGNED, DESTTY, EXPISSIGNED);
> \
> +    RESULT = CastInst::create(opcode, RESULT, DESTTY, "tmp", CurBB);
> \
> +  } 
> +
> +/* LLVM_INTRINSIC_OP_IS_SIGNED - This macro determines if a given
> operand
> + * to the intrinsic is signed or not. Note that this macro assumes it
> is being
> + * invoked from inside LLVM_TARGET_INTRINSIC_LOWER (see below)
> because it 
> + * requires the "exp" macro argument in order to determine signedness
> + */
> +#define LLVM_INTRINSIC_OP_IS_SIGNED(EXP, OPNUM) \
> +  !TYPE_UNSIGNED(TREE_TYPE(TREE_OPERAND(EXP, (OPNUM+1))))
> +
> /* LLVM_TARGET_INTRINSIC_LOWER - For builtins that we want to expand
> to normal
>   * LLVM code, emit the code now.  If we can handle the code, this
> macro should
>   * emit the code, return true.  Note that this would be much better
> as a
> @@ -3693,7 +3714,8 @@
>   * use methods it defines.
>   */
> #define LLVM_TARGET_INTRINSIC_LOWER(BUILTIN_CODE, DESTLOC, RESULT,
> \
> -                                    DESTTY, OPS, CURBB)
> \
> +                                    DESTTY, OPS, CURBB, EXP,
> RESISSIGNED,     \
> +                                    EXPISSIGNED)
> \
>    switch (BUILTIN_CODE) {
> \
>    default: break;
> \
>    case ALTIVEC_BUILTIN_VADDFP:
> \
> @@ -3827,34 +3849,39 @@
>        /* Map all of these to a shuffle. */
> \
>        unsigned Amt = Elt->getZExtValue() & 15;
> \
>        PackedType *v16i8 = PackedType::get(Type::SByteTy, 16);
> \
> -      OPS[0] = CastToType(OPS[0], v16i8);
> \
> -      OPS[1] = CastToType(OPS[1], v16i8);
> \
> +      Value *Op0 = OPS[0];
> \
> +      Instruction::CastOps opc = CastInst::getCastOpcode(
> \
> +        Op0, Op0->getType()->isSigned(), DESTTY, DESTTY->isSigned());
> \

This isn't right. You shouldn't add any isSigned() calls as they are
going 
away. The signedness of Op0 needs to be determined using the
LLVM_INTRINSIC_OP_IS_SIGNED macro. The signedness of DESTTY is provided
in the
EXPISSIGNED argument to the macro.

> +      OPS[0] = CastToType(opc, Op0, v16i8);
> \
> +      Value *Op1 = OPS[1];
> \
> +      opc = CastInst::getCastOpcode(
> \
> +        Op1, Op1->getType()->isSigned(), DESTTY, DESTTY->isSigned());
> \

Same comment.

> +      OPS[1] = CastToType(opc, Op1, v16i8);
> \
>        RESULT = BuildVectorShuffle(OPS[0], OPS[1],
> \
>                                    Amt, Amt+1, Amt+2, Amt+3,
> \
>                                    Amt+4, Amt+5, Amt+6, Amt+7,
> \
>                                    Amt+8, Amt+9, Amt+10, Amt+11,
> \
>                                    Amt+12, Amt+13, Amt+14, Amt+15);
> \
> -      RESULT = CastToType(RESULT, DESTTY);
> \
>        return true;
> \
>      }
> \
>      return false;
> \
>    case ALTIVEC_BUILTIN_VPKUHUM: {
> \
> -    Instruction::CastOps opc = CastInst::getCastOpcode(
> \
> -        OPS[0], OPS[0]->getType()->isSigned(), DESTTY,
> DESTTY->isSigned());   \
> +    Instruction::CastOps opc = CastInst::getCastOpcode(OPS[0],
> \
> +        LLVM_INTRINSIC_OP_IS_SIGNED(EXP,0), DESTTY, EXPISSIGNED);
> \

This one is correct.

>      OPS[0] = CastInst::create(opc, OPS[0], DESTTY, OPS[0]->getName(),
> CurBB); \
> -    opc = CastInst::getCastOpcode(
> \
> -        OPS[1], OPS[1]->getType()->isSigned(), DESTTY,
> DESTTY->isSigned());   \
> +    opc = CastInst::getCastOpcode(OPS[1],
> \
> +        LLVM_INTRINSIC_OP_IS_SIGNED(EXP,1), DESTTY, EXPISSIGNED);
> \
>      OPS[1] = CastInst::create(opc, OPS[1], DESTTY, OPS[0]->getName(),
> CurBB); \
>      RESULT = BuildVectorShuffle(OPS[0], OPS[1], 1, 3, 5, 7, 9, 11,
> 13, 15,    \
>                                  17, 19, 21, 23, 25, 27, 29, 31);
> \
>      return true;
> \
>    }
> \
>    case ALTIVEC_BUILTIN_VPKUWUM: {
> \
> -    Instruction::CastOps opc = CastInst::getCastOpcode(
> \
> -        OPS[0], OPS[0]->getType()->isSigned(), DESTTY,
> DESTTY->isSigned());   \
> +    Instruction::CastOps opc = CastInst::getCastOpcode(OPS[0],
> \
> +        LLVM_INTRINSIC_OP_IS_SIGNED(EXP,0), DESTTY, EXPISSIGNED);
> \
>      OPS[0] = CastInst::create(opc, OPS[0], DESTTY, OPS[0]->getName(),
> CurBB); \
> -    opc = CastInst::getCastOpcode(
> \
> -        OPS[1], OPS[1]->getType()->isSigned(), DESTTY,
> DESTTY->isSigned());   \
> +    opc = CastInst::getCastOpcode(OPS[1],
> \
> +        LLVM_INTRINSIC_OP_IS_SIGNED(EXP,1), DESTTY, EXPISSIGNED);
> \
>      OPS[1] = CastInst::create(opc, OPS[1], DESTTY, OPS[0]->getName(),
> CurBB); \
>      RESULT = BuildVectorShuffle(OPS[0], OPS[1], 1, 3, 5, 7, 9, 11,
> 13, 15);   \
>      return true;
> \
> @@ -3888,9 +3915,7 @@
>      Constant *C = ConstantInt::get(Type::IntTy, 0x7FFFFFFF);
> \
>      C = ConstantPacked::get(std::vector<Constant*>(4, C));
> \
>      RESULT = BinaryOperator::createAnd(OPS[0], C, "tmp", CurBB);
> \
> -    Instruction::CastOps opcode = CastInst::getCastOpcode(
> \
> -        RESULT, RESULT->getType()->isSigned(), DESTTY,
> DESTTY->isSigned());   \
> -    RESULT = CastInst::create(opcode, RESULT, DESTTY, "tmp", CurBB);
> \
> +
> LLVM_TARGET_INTRINSIC_CAST_RESULT(RESULT,RESISSIGNED,DESTTY,EXPISSIGNED); \
>      return true;
> \
>    }
> \
> 
> 
> Cheers,
> 
> 
> -- Jim
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> On 20-Dec-06, at 09:43 PM, Chris Lattner wrote:
> 
> > 
> > 
> > On Dec 20, 2006, at 4:46 PM, Reid Spencer wrote:
> > 
> > 
> > > Attached is a patch for the gcc/config/rs6000/rs6000.h file to
> > > remove
> > > isSigned() calls. This is analgous to the last patch I sent but
> > > for PPC
> > > instead of X86.  Same comments
> > > apply.
> > > 
> > > 
> > > This patch has not been compiled or tested as I don't have a PPC
> > > machine. Please review and try it.
> > 
> > 
> > This patch looks fine to me.
> > 
> > 
> > Jim, please check that it works (e.g. by making a testcase that uses
> > the modified instructions) and commit it if it looks good to you,
> > 
> > 
> > Thanks,
> > 
> > 
> > -Chris
> > 
> > 
> 




More information about the llvm-commits mailing list