<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 25, 2016 at 7:04 PM, Manuel Jacob <span dir="ltr"><<a href="mailto:me@manueljacob.de" target="_blank">me@manueljacob.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mjacob added a comment.<br>
<span class=""><br>
In <a href="http://reviews.llvm.org/D16383#335626" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16383#335626</a>, @dberlin wrote:<br>
<br>
> Hi Manuel,<br>
><br>
> Things like NewGVN don't always have instruction *'s to hand to ConstantFoldInstOperands.<br>
><br>
> For example, we create an expression that stores operands and opcodes and such (like GVN does!)<br>
><br>
> Before, we could pass the opcode and operands, and that worked great.<br>
><br>
> Now you've made it take the Instruction.<br>
<br>
<br>
</span>Sorry for making difficulties.\<br></blockquote><div><br></div><div>It happens :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> I don't see how this makes anything related to opaque pointers easier, maybe you could enlighten me?<br>
<br>
<br>
</span>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.<br>
<span class=""><br></span></blockquote><div><br></div><div>But it's also not possible for comparisons, so it's just "another thing that it doesn't work on". That's fine :)</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> 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.<br>
<br>
>  I want to see if it will fold to a constant.<br>
<br>
><br>
<br>
> What API am i expected to use to do so?<br>
<br>
><br>
<br>
> 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.<br>
<br>
<br>
</span>ConstantFoldInstOperands constant folds the given instruction **but with the specified operands**. If I understand correctly, you still have the instruction available.<br>
<span class=""><br></span></blockquote><div><br></div><div>I do *not* have the instruction available.</div><div>I have it's opcode, operands, type, etc.</div><div>(For geps, i have the source element type as well ).</div><div> But i don't have the original instruction, and in fact, there isn't alwas a mapping</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> 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.<br>
<br>
<br>
</span>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.<br>
<br></blockquote><div><br></div><div>Sure, but constantfoldinstoperands handled dispatching to those functions, otherwise, you have to write and update a dispatch function constantly.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.</blockquote><div>What if i want to fold a generic operation without having to worry about whether it is specifically a binary op, cast, etc.</div><div> </div><div>You've basically pushed all that logic/work into the callers, where it will be duplicated.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2. The user wants to fold a binary operation or cast. I added functions for this in <a href="http://reviews.llvm.org/rL258389" rel="noreferrer" target="_blank">http://reviews.llvm.org/rL258389</a> and <a href="http://reviews.llvm.org/rL258390" rel="noreferrer" target="_blank">http://reviews.llvm.org/rL258390</a>.<br>
<br>
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.<br></blockquote><div><br></div><div>I'm going to pretty strongly disagree. Not everything in LLVM necessarily has an instruction associated with it yet.</div><div><br></div><div>Things like PRE, etc, often have a bunch of operands an opcode, and want to know what will happen. Before you rpatch, this was easy.</div><div><br></div><div>Now, it's literally impossible to get *any* results (instead of just "give me the results you can").</div><div><br></div><div>That seems like a pretty bad API regression.</div><div><br></div></div></div></div>