[PATCH] D56838: [CGP] Check for existing inttotpr before creating new one

Roman Tereshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 17 22:52:40 PST 2019


rtereshin added a comment.

In D56838#1362647 <https://reviews.llvm.org/D56838#1362647>, @hfinkel wrote:

> In D56838#1362440 <https://reviews.llvm.org/D56838#1362440>, @rtereshin wrote:
>
> > In D56838#1361579 <https://reviews.llvm.org/D56838#1361579>, @hfinkel wrote:
> >
> > > Can we run EarlyCSE after CGP (instead of trying to do these kinds of point fixes)?
> >
> >
> > Hi Hal,
> >
> > Thank you for looking into this!
> >
> > That's a great suggestion! Originally I just eyeballed it as too expensive compile-time wise for the little effect that is achieved here.
> >
> > This time around with you pointing it out, I performed the actual measurements for a very large suite of shaders (the downstream target in question is a GPU). I see about 1.0 (+/-0.3)% increase in overall compile time (the part of it that happens at an application run-time) on average. The generated code quality improvement that comes out of this may be important, but it doesn't trigger often enough to justify 1% compile time increase across the board.
> >
> > Thanks,
> > Roman
>
>
> Fair enough. Thanks for checking.
>
> I have a comment about the test case, but otherwise LGTM.


Fixed the test, thank you!

Roman


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56838





More information about the llvm-commits mailing list