[llvm-commits] CAST Patch Update [1/4] first third of the patch.
Chris Lattner
clattner at apple.com
Sun Nov 12 21:38:22 PST 2006
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
More information about the llvm-commits
mailing list