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

[Philip Reames via llvm-commits]

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?
>
>> 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