[llvm-commits] [llvm] r138722 - in /llvm/trunk: lib/Analysis/ConstantFolding.cpp lib/Transforms/InstCombine/InstCombineCasts.cpp test/Transforms/InstCombine/cast.ll

[Rotem, Nadav]

Hi Duncan, 

Here is another attempt. In the attached patch I had reverted r138722, and added an exception to isEliminableCastPair, which allows the bitcasting of A->B->A. 
I also added some new tests. 

Thanks,
Nadav





-----Original Message-----
From: Duncan Sands [mailto:baldrick at free.fr] 
Sent: Monday, August 29, 2011 09:41
To: Rotem, Nadav
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [llvm] r138722 - in /llvm/trunk: lib/Analysis/ConstantFolding.cpp lib/Transforms/InstCombine/InstCombineCasts.cpp test/Transforms/InstCombine/cast.ll

Hi Nadav,

> I attached a patch to fix the MMX type issue. I am still not sure about the legality of casting vectors to integers, as defined in isEliminableCastPair.

I think you should check the uses of isEliminableCastPair to see if you can see
why it doesn't want to consider vectors.  Assuming that there is no good reason,
I think you should use isEliminableCastPair in your patch, and change
isEliminableCastPair to allow vectors, but disallow mmx.

Ciao, Duncan.

PS: Don't forget to run the testsuite after making this change!

>
> Thanks,
> Nadav
>
>
>
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Duncan Sands
> Sent: Sunday, August 28, 2011 15:07
> To: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [llvm] r138722 - in /llvm/trunk: lib/Analysis/ConstantFolding.cpp lib/Transforms/InstCombine/InstCombineCasts.cpp test/Transforms/InstCombine/cast.ll
>
> Hi Nadav,
>
>> Bitcasts are transitive. Bitcast-Bitcast-X becomes Bitcast-X.
>
> sadly this is not true because you are only allowed to bitcast an mmx type
> to and from a vector type, but you can bitcast vector to and from integers
> etc.
>
>> +  // Bitcast of Bitcast can be done using a single cast.
>> +  ConstantExpr *CE = dyn_cast<ConstantExpr>(C);
>> +  if (CE&&   CE->getOpcode() == Instruction::BitCast) {
>> +    return ConstantExpr::getBitCast(CE->getOperand(0), DestTy);
>> +  }
>
> No need for curly brackets {}.
>
>> +  // Bitcasts are transitive.
>> +  if (BitCastInst* BSrc = dyn_cast<BitCastInst>(Src)) {
>> +    return CastInst::Create(Instruction::BitCast, BSrc->getOperand(0), DestTy);
>> +  }
>
> Likewise.  Also, this could be:
>     if (isa<BitCastInst>(Src))
>       return CastInst::Create(Instruction::BitCast, Src->getOperand(0), DestTy);
> Also, shouldn't you create the cast using instcombine's builder?
>
> Ciao, Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bitcast_patch.diff
Type: application/octet-stream
Size: 3950 bytes
Desc: bitcast_patch.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110829/f4d94354/attachment.obj>