[PATCH] D16383: Change ConstantFoldInstOperands to take Instruction instead of opcode and type. NFC.

[Manuel Jacob via llvm-commits]

On 2016-01-27 01:51, Philip Reames wrote:
> On 01/25/2016 07:04 PM, Manuel Jacob via llvm-commits wrote:
>> mjacob added a comment.
>> 
>> In http://reviews.llvm.org/D16383#335626, @dberlin wrote:
>> 
>>> Hi Manuel,
>>> 
>>> Things like NewGVN don't always have instruction *'s to hand to 
>>> ConstantFoldInstOperands.
>>> 
>>> For example, we create an expression that stores operands and opcodes 
>>> and such (like GVN does!)
>>> 
>>> Before, we could pass the opcode and operands, and that worked great.
>>> 
>>> Now you've made it take the Instruction.
>> 
>> Sorry for making difficulties.
>> 
>>> I don't see how this makes anything related to opaque pointers 
>>> easier, maybe you could enlighten me?
>> 
>> The problem is getelementptr. With opaque pointers, we must have the 
>> instruction to get the source element type. Currently this is possible 
>> by getting the pointee type of the first (pointer) operand of the GEP. 
>> This won't be possibly with opaque pointers.
> Why is this not as simple as adding a type parameter to the existing 
> API?

While we could add a type parameter for the element type of the pointer 
operand, this parameter would only be meaningful for GEPs.  This raises 
the question what a user should pass if the instruction in not a GEP.  
One option would be to set this parameter to `nullptr` by default and 
require it to be passed only for GEPs.  However this adds an implicit 
requirement of having to pass an additional type if and only if the 
instruction (or constant expression) constant-folded is a GEP.  I'd 
argue that this complicates the interface.

-Manuel

>> 
>>> Additionally, i can't see how use cases like mine are supposed to 
>>> function - i have what was an instruction, but is now an operation 
>>> and some operands.
>>>   I want to see if it will fold to a constant.
>>> What API am i expected to use to do so?
>>> Note also that the InstOperands API was *explicitly* designed to not 
>>> require an instruction, to support the above use case. This was the 
>>> difference between ConstantFoldInst and ConstantFoldInstOperands.  It 
>>> happens most users can pass it instructions, but that doesn't , IMHO 
>>> make a lot of sense.
>> 
>> ConstantFoldInstOperands constant folds the given instruction **but 
>> with the specified operands**. If I understand correctly, you still 
>> have the instruction available.
>> 
>>> I want to request that you provide some way to not require the 
>>> instruction, as the current API does (and many users use!), so that 
>>> folks can use this without creating fake instructions to do so.
>> 
>> The problem with such an API is that it doesn't work with 
>> getelementptr (and not with comparison instructions, as it was not 
>> possible before this patch). I think that most use cases are covered 
>> by already existing functions.
>> 
>> 1. The user wants to fold an instruction but with other operands (e.g. 
>> because they were constant-folded by the user). 
>> ConstantFoldInstOperands should work here.
>> 2. The user wants to fold a binary operation or cast. I added 
>> functions for this in http://reviews.llvm.org/rL258389 and 
>> http://reviews.llvm.org/rL258390.
>> 
>> There is no API for the case when the user has a random instruction 
>> opcode but no instruction. I don't think that adding a function for 
>> this case which however doesn't work with getelementptr and compares 
>> is very helpful in this case.
>> 
>> 
>> http://reviews.llvm.org/D16383
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits