[llvm-dev] Old SimplifyInstruction API about to fully die

[Daniel Berlin via llvm-dev]

r301673

On Fri, Apr 28, 2017 at 10:08 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

> This is about to be done, as i have converted remaining users and the
> bootstraps are just about done.
> I will follow up with the rev number.
>
> For those needing to convert out of tree uses:
>
> Trivially, you should be able to take any place that has
> SimplifyInstruction(I, ...), and do SimplifyInstruction(I, {...}).
>
> However, where possible, it is going to be better to construct a
> SimplifyQuery once and use it everywhere.
> CorrelatedValuePropagation and LoopRotation have examples of how to do
> this.
>
> I have converted everywhere where it is  easy to do so.to construct them
> once, and everywhere else now constructs them using the above.
>
> I am also introducing a helper function, named "getBestSimplifyQuery",
> that can take Pass/AnalysisManager/LoopStandardAnalysis, and will
> construct the best SimplifyQuery possible from whatever optional analysis
> are available.
>
> Thus, in a Pass runOnFunction, you could do
>
> const SimplifyQuery SQ = getBestSimplifyQuery(*this, F)
>
> and it will hand you back a structure filled in with as many of the
> analyses as are around and up to date. You can then pass that to
> SimplifyInstruction later.
>
> Using  this is strongly preferred as it will enable us to add analysis to
> SimplifyInstruction and just update those functions.
>
> I bootstrapped with every target enabled, sorry if i missed something!
>
> On Wed, Apr 26, 2017 at 4:55 PM, Philip Reames via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Agreed.  This seems like overall a huge win for readability.
>>
>> I am a bit worried about all the miscompiles you're going to expose by
>> using all that extra information, but we should definitely just fix those.
>> :)
>>
>> Philip
>>
>> On 04/26/2017 08:53 AM, Hal Finkel via llvm-dev wrote:
>>
>>
>> On 04/25/2017 11:32 PM, Daniel Berlin via llvm-dev wrote:
>>
>> For those not following along, startingin r301379, Simplify* in
>> InstructionSimplify now can just take a query struct  instead of 8000
>> optional arguments.  Nothing is really new since it used the same thing
>> under the covers.
>>
>> I'm slowly converting the old uses away (deletion of the old APIs is a
>> different question).
>> Staring at most of them, i could just directly convert them using braced
>> list initialization, and construct the objects again and again, but the
>> vast majority of uses actually:
>>
>> A. don't change the relevant query arguments over the lifetime of the pass
>> B. actually have more information sitting around than they are passing
>> along.
>>
>> For example, instcombine has a DomTree and AssumptionCache that are
>> required, but it doesn't pass them to SimplifyBinOp in a bunch of cases.
>>
>> JumpThreading has TLI but doesn't pass it to SimplifyCmpInst.
>>
>> CVP at least admits it has a problem:
>> "CorrelatedValuePropagation.cpp:  // FIXME: Provide TLI, DT, AT to
>> SimplifyInstruction.
>> CorrelatedValuePropagation.cpp:  if (Value *V = SimplifyInstruction(P,
>> DL)) {
>> "
>> (This is because it uses LVI, which requires those things, but it doesn't
>> ask for them itself)
>>
>> Assuming this is not deliberate, it would seem to me to just be easiest
>> to set up the query structure once in the pass and use it everywhere, which
>> would hopefully avoid these issues in the future, besides making most of
>> the call strings dramatically shorter :)
>>
>> Is there any real downside (compile time performance, etc) to this vs
>> what we do now (which is basically constructing the query objects again and
>> again under the covers) that anyone can think of?
>>
>> (again, assuming we are not deliberately avoiding passing info to
>> Simplify* in most of these cases)
>>
>>
>> I don't think it is deliberate; it is just that no one has done the work
>> to make it uniform. It is possible that doing this will expose some
>> compile-time issues, but if so, we should deal with them for real (not just
>> hide them by a failure to provide optional-but-available analyses).
>>
>> I'm certainly in favor of more uniformity here.
>>
>>  -Hal
>>
>>
>> I figured i'd ask before i went writing the scripts/etc to help convert a
>> bunch of this stuff :)
>>
>>
>>
>> _______________________________________________
>> LLVM Developers mailing listllvm-dev at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>> --
>> Hal Finkel
>> Lead, Compiler Technology and Programming Languages
>> Leadership Computing Facility
>> Argonne National Laboratory
>>
>>
>>
>> _______________________________________________
>> LLVM Developers mailing listllvm-dev at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170428/3cf92fda/attachment.html>