Hi Renato, > A ConstantExpr *is* a Value,
Indeed, this was poor wording on my part. As ConstantExpr is a Constant, its operands must be Constants. And it itself can be used as an operand to something expecting a Constant. If I wish to change it or one of its operands to an Instruction, I cannot do that (because, obviously, Instructions aren't constant). There is no easy way to go from a ConstantExpr to its Instruction corollary, and that seems an unnecessary restriction. That is why I'm adding this extra API function. The function I added does *not* bypass the hierarchy, IMHO, as every ConstantExpr has an Instruction counterpart. Where I said "Value" before, I meant "Value which is not a subclass of Constant". > Have you thought about adding constructors for the Instructions > passing ConstantExpr references? That might be used automatically by > the compiler as a casting match, which will be even neater. That seems like a heavyweight solution, and I also disagree with it because the current implementation as sent keeps all the implementation details of ConstantExpr (of which there are many) located and isolated in the same .cpp file. Adding implicit constructors which take a ConstantExpr would spread implementation details of ConstantExpr around the codebase. Also, Richard Smith mentioned at the dev conference that there may be a push to remove ConstantExpr completely, and if so my current implementation would be much easier to clean up :) Cheers, James ______________________________________ From: [email protected] [[email protected]] On Behalf Of Renato Golin [[email protected]] Sent: 16 November 2012 20:23 To: James Molloy Cc: [email protected]; [email protected] Subject: Re: [llvm-commits] [PATCH] Allow converting ConstantExprs to Instructions On 16 November 2012 18:17, James Molloy <[email protected]> wrote: > The attached patch implements a simple extra API function to retrieve an > Instruction* equivalent for a ConstantExpr. With this addition, dealing > with the constraints imposed by SPIR is actually possible without > horrible hackery... A ConstantExpr *is* a Value, so the wrapper you did bypasses the hierarchy (possibly broken), but is also disconnected from the API, which means when either side change, you might forget to change this bridge and get away with it. Having said that, I don't think the consequences of forgetting to update this code is drastic (I can't see why it'd do anything other than assert at compile time), but since there is already a connection between ConstantExpr and Value, I think it's a cheap trick. Have you thought about adding constructors for the Instructions passing ConstantExpr references? That might be used automatically by the compiler as a casting match, which will be even neater. -- cheers, --renato http://systemcall.org/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
