[PATCH] D61409: [SimplifyCFG] Added condition assumption for unreachable blocks

[Sanjay Patel via Phabricator via llvm-commits]

spatel added subscribers: reames, hfinkel.
spatel added a comment.

In D61409#1488242 <https://reviews.llvm.org/D61409#1488242>, @lebedev.ri wrote:

> In D61409#1488239 <https://reviews.llvm.org/D61409#1488239>, @xbolva00 wrote:
>
> > It would be good if anybody familiar with llvm patternmatch could fix m_OneUse to ignore llvm.assume.
>
>
>
>
> In D61409#1487377 <https://reviews.llvm.org/D61409#1487377>, @lebedev.ri wrote:
>
> > In D61409#1487375 <https://reviews.llvm.org/D61409#1487375>, @nikic wrote:
> >
> > > In D61409#1487372 <https://reviews.llvm.org/D61409#1487372>, @lebedev.ri wrote:
> > >
> > > > In D61409#1487366 <https://reviews.llvm.org/D61409#1487366>, @nikic wrote:
> > > >
> > > > > This is a good idea in theory, but I'm somewhat concerned that this will backfire in practice because assumes often pessimize optimization. For example LLVM has a really bad offender where it inserts assumes for alignment annotations during inlining -- thankfully there's an option to disable it, because it's a major perf regression otherwise.
> > > >
> > > >
> > > > Pessimization of the final produced code, or of the llvm itself?
> > >
> > >
> > > Of the final produced code. I think it's mainly because assumes cause one-use checks to fail.
> >
> >
> > Oh, good point. It would make sense to not count `@llvm.assume()` during use-checking,
> >  but then this isn't just about the `call void @llvm.assume()`,  but about all the instructions
> >  that would be dropped when the call is dropped during back-end lowering...
>
>
> I.e. just ignoring only the `@llvm.assume()` itself is only tip of the iceberg.
>
> In D61409#1488239 <https://reviews.llvm.org/D61409#1488239>, @xbolva00 wrote:
>
> > Then this patch will not cause any problems or regressions..
> >
> > @spatel @RKSimon
>


I agree that this is a (much) bigger than fixing the pattern matcher. I have no instinct or data to say how much inserted assumes can/do harm optimization, but this is a long-standing problem with that implementation. @hfinkel or @reames may have a better idea.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61409/new/

https://reviews.llvm.org/D61409