[llvm-commits] CAST Patch Update [1/4] first third of the patch.

[Chris Lattner]

On Nov 12, 2006, at 8:25 PM, Reid Spencer wrote:
> I have a meta-comment on your review. The ConvertInst::getCast and
> ConstantExpr::getCast methods work.  Many of your review comments  
> claim
> they don't. I should have primed you for this but didn't have time.
> Here's why it works:
>
> 1. These two methods are temporary. They were introduced to  
> minimize the
> size of the cast patch. Its already 5 times larger than previous  
> patches
> in this series. Without them it would be much larger still.

Understood, shrinking the patch is greatly appreciated :)

> 2. While the integer types are signed these methods work correctly
> because there is only one cast opcode that is suitable for any given
> pair of first class types. Consequently all the "new CastInst(...)"
> calls can be replaced with "ConvertInst::getCast(...)" calls which
> simply determine the (only) opcode that is suitable.

This isn't clear.  Consider the SCEV code, for example.  There you  
*generalized the code* to allow casts that didn't happen before.  In  
this case, I think that signedness is starting to get confused (e.g.  
you can get sext's of unsigned types).  Once it gets confused,  
getCast stops working.

> 3. When all the instruction changes are done (SETCC, GEP, CALL remain)
> we will revisit each call site for these methods and determine how to
> replace the getCast call with the instantiation of the correct cast
> instruction (most likely via getConvert which takes an opcode as the
> first argument).

Right.

> You have pointed out many instances where the calls to getCast  
> would be
> dangerous if the integer types became signless.  However, that is not
> the case currently.  I consider every single one of them dangerous and
> they will all be removed before we make the integer types signless.

Ok.

> If there's something provably wrong with this strategy, please inform
> me. I'm trying to keep your review work minimized.

I think that SCEV is the only really unsafe one that I have seen so far.

-Chris